Fix 324 : Out of memory errors while updating repos.
Fixes#324, but in the process makes the updater take a lot longer. My benchmarks tell me that an update which used to take approx 30 seconds on my Nexus 4 now takes about 50-55 seconds. This is because it first inserts the apps into the database (in a temp table) and then subsequently copies that table to the actual table. This means there is a lot more disk access than before.
I'm open for discussion on whether this tradeoff is worth it - however I'll caution that there is always going to be a tradeoff between faster and more memory vs slower and less memory. This is the case with all software, and perhaps more so with memory constrained devices such as phones. Also, as the repo index grows (until perhaps we are able to extract the app descriptions in the future), this will become more of an issue.
I'd also like this to be CR'ed properly before merging, because it changes some important code around the repo updater. It is important because security, and it is also important because it is the main thing that F-Droid needs to do (get a list of apps to show the user).
See merge request !173
The repo xml handler now has a different mechanism for returning
data about the parsed xml file. This is done via a callback, rather
than storing the data in member variables. The tests now deal with
this correctly.
The update/delete operations of the TempAp[pk]Provider's didn't
work, so that has now been fixed.
Refactored repo update to stream apks from network -> jar file reader ->
xml parser -> database. No longer build up large lists of app metadata
to save. Saves memory, but is MUCH slower.
Does sig verification properly, but does it at the END of the process
and DOESN'T ROLL BACK on failure.
Quick and dirty benchmarks show an increase in time from ~25 seconds
to ~30 seconds on my Nexus 4 with Android 5.0. This doesn't seem so
bad to me, for the tradeoff that people on low end devices can actually
update now.
Also, as @eighthave pointed out, if we are able to stream the download
directly from the internet, then that time will drop to essentially
the time it takes to download the index.
Leave only the ones which align with the current multi-repo behaviour
that F-Droid exhibits.
The commented out tests can be uncommented in the future when working
on proper multi-repo support.
(One of) the problems with F-Droid's multiple support is that there is
a primary key on the fdroid_apk table which is a composite of:
* id
* vercode
Which means that two repos providing the same version means one will
update the other, rather than ending up with two different versions.
Instead, there should be some other way to differentiate apks from
different sources. Firstly, it should take into account the signing
cert. Secondly, it may taken into account the hash, because two people
could sign different apks with the same cert and then we are back at
square one.
The new test skeletons right now update three different repos
in different configurations. They do so such that the order of updates
changes and therefore the way in which conflicts between repos are
dealt with are tested.
They should all have the same result (though I'm not sure exactly what
that should be yet).
Now that there is only ever the index.jar, the whole flow of RepoUpdater
has changed quite a bit. This updates the logic for deciding when to store
the current repo's pubkey in the database for future reference.
This changes the flow to stop writing the unpacked index.xml and instead
stream it directly to the XML parser from the index.jar. This should speed
things up some.
refs #259https://gitlab.com/fdroid/fdroidclient/issues/259
This is also work towards running the whole thing in the background:
refs #103https://gitlab.com/fdroid/fdroidclient/issues/103
This also removes the progress stuff since it will need to change a lot to
work with the streaming mode
Before, there was an abstract RepoUpdater class with two subclasses, one
for signed and unsigned. Now there is just a single class, and it only
ever starts with the index.jar. So this removes lots of code that was
there to handle that more complicated structure. For example, there is no
longer the need to separately work on the index.xml vs index.jar.
Updated the README with details of how to run the tests. Also added a
minor fix to allow tests to run on pre-honeycomb devices. However their
behaviour may not be 100% defined - because the contentproviders are
not shuttind down correctly due to lack of an API to do so.
Fix#263 "cannot manually add repo that was swapped before"
Pretends that the swap repo never existed, by deleting it before adding
the new repo, and showing the same message that is shown when a new
repo is added. This does not change behaviour for existing non-swap
repos. They are not deleted before being added again, or else we would
lose the ability to verify the fingerprint of an existing repo is the
same as a newly added one with the same URL.
Note that this has the effect that the fingerprint/pubkey of the swap
repo is nuked when adding that repo manually.
Internationalised the string "BAD FINGERPRINT" while I was at it.
To test it out, here is some instructions to make life easier:
Firstly, go into manage repos and delete the guardian project main repo (going to pretend to use this for swapping to make life easier).
Then if you run `sqlite3 /data/data/org.fdroid.fdroid/databases/fdroid` and execute the query:
`select substr(fingerprint, 0, 10), substr(pubkey, 70, 10), address, isSwap from fdroid_repo order by fingerprint desc;`
You should see:
```
B7C2EEFD8|081ad310b3|https://guardianproject.info/fdroid/archive|0
43238D512|071310b300|https://f-droid.org/archive|0
43238D512|071310b300|https://f-droid.org/repo/|0
```
Now simulate a swap session like so:
```
adb shell am start -a android.intent.action.VIEW -d 'https://guardianproject.info/fdroid/repo?swap=1'
```
Which results in the following database:
```
B7C2EEFD8|081ad310b3|https://guardianproject.info/fdroid/archive|0
B7C2EEFD8|081ad310b3|https://guardianproject.info/fdroid/repo|1
43238D512|071310b300|https://f-droid.org/archive|0
43238D512|071310b300|https://f-droid.org/repo/|0
```
Note the last column (`isSwap`) is `1` for the newly added swap repo. Now we will add the repo (without a fingerprint) to the Manage Repo activity. If you are feeling lazy, execute:
```
adb shell am start -a android.intent.action.VIEW -d https://guardianproject.info/fdroid/repo
```
The repo will be removed, then re-added as a TOFU repo:
```
B7C2EEFD8|081ad310b3|https://guardianproject.info/fdroid/archive|0
43238D512|071310b300|https://f-droid.org/archive|0
43238D512|071310b300|https://f-droid.org/repo/|0
||https://guardianproject.info/fdroid/repo/|0
```
I noticed some bugginess with sending the same intent and it being ignored, I'll have to look at this another day (not caused by this change, it already existed in master).
See merge request !90
Pretends that the swap repo never existed, by deleting it before adding
the new repo, and showing the same message that is shown when a new
repo is added. This does not change behaviour for existing non-swap
repos. They are not deleted before being added again, or else we would
lose the ability to verify the fingerprint of an existing repo is the
same as a newly added one with the same URL.
Note that this has the effect that the fingerprint/pubkey of the swap
repo is nuked when adding that repo manually.
Internationalised the string "BAD FINGERPRINT" while I was at it.
Now the other content provider functions which can result in broken SQL
due to the number of arguments is private, and can only be accessed
from a public helper method which ensures that limit is never hit.
The update count was broken because I added the join onto the
apk table, and in the process, forced a GROUP BY on the AppProvider
queries. This group by made the COUNT(*) actually count the number
of apks for each app, not the total rows.
They were all due to the addition of "application label" to the
installed app cache. This commit adds a mock ApplicationInfo
to the mock package manager and also specifies the label while
inserting into the test content provider.
This also makes AndroidStudio integration work better, which makes
running and debugging tests much nicer than the CLI.
Also cleaned up imports in one test, and made the symlink tests not
fail on older devices below API 19.
* Android-21 introduced an API for symlinking.
* Android-19 has an API which can be used via reflection.
* Earlier versions use Runtime.exec('/system/bin/ln')
This also extends the SanitizedFile stuff so that the android < 19 can
safely use Runtime.exec() with less fear of command injection vulnerabilities.
Finally, some tests for the SanitizedFile and symlink stuff was added.
This prevents an app with "write external storage" permission from
being able to switch the legit app with a dodgey one between F-Droid
requesting an install, and the package manager actually showing the
install dialog to the user.
In order to make the file in private internal storage readable by
the package manager, its parent directories need to be world-executable,
and the file itself needs to be world-readable. It seems that the
"/data/data/org.fdroid.fdroid/cache" dir provided by the Context is
already world executable, but the "apks" subdirectory does not default
to this.
Also, to be compatible with android-8, a Runtime.getRuntime().exec()
call was added for such devices, which invokes /system/bin/chmod.
The effect of this was to require some level of file sanitization to
be made available using the Java type system to prevent command injection
attacks from weird apk names (as people are free to download metadata
from random internet people).