findbugs tells us:
Incorrect lazy initialization and update of static field org.fdroid.fdroid.
FDroidCertPins.PINLIST in org.fdroid.fdroid.FDroidCertPins.getPinList().
This method contains an unsynchronized lazy initialization of a static
field. After the field is set, the object stored into that location is
further updated or accessed. The setting of the field is visible to other
threads as soon as it is set. If the futher accesses in the method that set
the field serve to initialize the object, then you have a very serious
multithreading bug, unless something else prevents any other thread from
accessing the stored object until it is fully initialized.
Even if you feel confident that the method is never called by multiple
threads, it might be better to not set the static field until the value you
are setting it to is fully populated/initialized.
The third parameter in the MemorizingTrustManager constructor was not good
apparently. Here's the email from Ge0rg, the MemorizingTrustManager author:
As you added MTM into the f-droid client, I'm writing to inform you that
the MTM constructor API was incorrect, and has been changed in current
git master:
When using the three-parameter constructor, the second parameter, a
trustmanager, was only used until the user stored a certificate into
MTM, and was overwritten after that.
Please use the new MTM constructor, and pass it the pinMgr as the only
trust manager parameter.
This should help is there is ever multithreaded access to this variable.
This is an unlikely scenario, but the fix is easy.
findbugs reported this issue like this:
Incorrect lazy initialization and update of static field org.fdroid.fdroid.
FDroidApp.selectedApps in org.fdroid.fdroid.views.fragments.
SelectLocalAppsFragment.onActivityCreated(Bundle)
This method contains an unsynchronized lazy initialization of a static
field. After the field is set, the object stored into that location is
further updated or accessed. The setting of the field is visible to other
threads as soon as it is set. If the futher accesses in the method that set
the field serve to initialize the object, then you have a very serious
multithreading bug, unless something else prevents any other thread from
accessing the stored object until it is fully initialized.
findbugs found this problem and reported it like this:
Nullcheck of org.fdroid.fdroid.data.App.installedApk at line 191 of value
previously dereferenced in org.fdroid.fdroid.localrepo.LocalRepoManager.copyApksToRepo(List)
A value is checked here to see whether it is null, but this value can't be
null because it was previously dereferenced and if it were null a null
pointer exception would have occurred at the earlier dereference.
Essentially, this code and the previous dereference disagree as to whether
this value is allowed to be null. Either the check is redundant or the
previous dereference is erroneous.
default_repo_count is not used at all, and the numbering scheme is just a
vestige of that. This switches all the variables to have clear names of
what they are representing.
On Android 4.x, the category menu is showing up as pure black, and looks
very much like an app list item. I've personally witnessed many new users
struggle to find an app because the category is on "What's New" by default
and the app they are looking for is not new. Some even had troubles
remembering about the category menu after I told them. This small change
makes the category dropdown look the same on all Android versions, and
makes it a lot more apparent on newer Android releases.
This is a temporary usability fix until we can do something much better
than the category dropdown menu.
onNewIntent() is called because ManageReposActivity is set to "singleTask"
launchMode, but it is only called if ManageReposActivity is already
running. onResume() is always called, and called after onNewIntent() if it
is called, so use onNewIntent() only to set the current Intent, then parse
the Intent only in onResume().
Here is how to reproduce the original bug:
1. Close F-Droid properly and start it again.
2. Click on https://guardianproject.info/fdroid/repo in a browser (and tell
it to open with F-Droid)
3. Hit cancel on the add repo dialog
4. Leave F-Droid open and switch back to the browser
5. Open that link again.
This should result in two dialogs on top of one another. Happened from both
Firefox, Chrome, and Android browsers.
When someone clicks on a URL that F-Droid can accept, i.e. a repo URL, then
Android puts up a chooser where the user can select which app to VIEW the
URL with. That was showing up with the title "Repositories", which is the
title used for that Activity when viewing it. This keeps the Activity
title the same while changing the title in the chooser.
Instead of just sticking whatever URL is in the clipboard into the "Add
Repo" dialog, this attempts to sanitize the URL in case it has some garbage
or came from a QR Code, and therefore was all uppercase (that makes for
smaller QR Codes). It also checks if there is a fingerprint in the query
string of the URL, and sticks that into the fingerprint box.
fixes#50https://gitlab.com/fdroid/fdroidclient/issues/50
The swap stuff will also need to handle incoming Intents that represent
new repos, so the parsing logic is now its own class NewRepoConfig, which
is something like the Repo class, but using getters instead of properties.
Since the new repo data does not change once FDroid receives it, the only
way to set the values of a NewRepoConfig is via the constructor.
This is based on some incomplete work from @pserwylo:
71cb12ef5c (diff-6)
and
71cb12ef5c (diff-7)
AppListFragmentPageAdapter is a subclass of FragmentPagerAdapter, so it
should include the same spelling to make that clear and easy to trace,
grep for, etc.
Now that the Fragment is embedded in the Activity, and the menu has been
moved to the Activity in ActionBar style, most of the utility functions and
the Dialogs can be based out of the Activity, which is how they are
designed to work. This makes things work a lot easier.
fixes#3https://gitlab.com/fdroid/fdroidclient/issues/3
This allows the main menu to act like a proper ActionBar using appcompat.
It also allows for making the search happen live on the ListView, rather
than having to launch a separate Activity to show the results.
I went through all of the source code replacing anything that is now
possible using appcompat-v7. appcompat-v7 is the official way to handle
backwards compatibility, and it is supported by Google and others. Using it
as much as possible should make the code more maintainable and readable by
others since they'll be used to seeing the appcompat-v7 patterns from other
projects.
fixes#51https://gitlab.com/fdroid/fdroidclient/issues/51fixes#42https://gitlab.com/fdroid/fdroidclient/issues/42
Previously, it was using the native android.widget.SearchView.
Now it uses the widget from appcompat. For good measure, I also
made it so that the search button is always in the action bar,
rather than being hidden behind a menu sometimes.
Do so by:
- Making RepoDetailsFragment/RepoListFragment retain their instances
- Move the management of the update repo dialog to UpdateReceiver
- Let the fragments tell the UpdateReceiver when to show/hide
This was a bit more complex than all the other views, because it supports
rotation, and different views for when it is rotated. The end result is
that the way in which the views were constructed needed to be completely
redone.
In the process, I also moved the layout of the app summary to a Relative
Layout. This adds more flexibility, and is also the suggested layout
for complex views (as apposed to nested linear layouts). I believe this
is due to the performance of relative vs linear layotus.
It was aprticularly hard to figure out what was going on
when rotating an Activity which had a list fragment
that had another fragment as a header. I don't think fragments
were designed to work like this, but I believe it is all working
as expected now.
Conflicts:
src/org/fdroid/fdroid/Preferences.java
NOTE: I don't know how android will go with adding a new property
to a string-array resource, but not having it translated everywhere.
Will it struggle because the EN version has three values for "theme",
but other translations only have two?
The only remaining activity is the AppDetails acvitity, which will require
a little more than just making it extend ActionBarActivity. Currently,
it extends ListActivity. To support appcompat-v7, it really should have
two fragments - the details one and the list one. Then, when the orientation
is changed, it should load a different layout with the fragments side by side.
Although Google is encouraging people to make old devices run apps
with the action bar (via appcompat-v7), they haven't provided a way
for people to create preference/setting screens with an action bar.
There are plenty of issues in the Android issue tracker relating
to this, but it doesn't yet seem to be on the radar of the Android
devs.
Until there is a native implementation of PreferenceFragment in
the appcompat-v7 support library, this submodule provides is a 3rd
party solution. It is actually a fork of the first repo in github,
though that was a bit of an upload and dump, without accepting MR's.
This fork includes gradle support.
Tor Hidden Services are on domain names that always end in .onion, so there
is a URL pattern matcher that chooses which Downloader subclass to use
based on testing for .onion. This is a quick, dumb implementation. It
does make any attempt to see if Tor is running or even installed. That
can come once NetCipher is easy to handle in the context of FDroid.
refs #2367https://dev.guardianproject.info/issues/2367
This will ultimately be used to create the right Downloader subclass
instance based on the URL of the file to download (i.e. rfcomm://, .onion
address, ssh://, new socket protocols, etc).
Also delete unused constructors, they can trivially be readded if they are
ever used, and they are currently just clutter.
If a new "wifi connected" event comes in while a previous one is still
being processed, then cancel the current one as soon as possible. This
prevents the events from being processed in an interleaved manner, causing
chaos and crashes. Hopefully this will fix the jmdns crashes, since that
is triggered by onPostExecute() via FDroidApp.restartLocalRepoService().
java.lang.IllegalStateException: A service information can only be registered with a single instamce of JmDNS.
at javax.jmdns.impl.JmDNSImpl.registerService(JmDNSImpl.java:1005)
at org.fdroid.fdroid.localrepo.LocalRepoService$5.run(LocalRepoService.java:239)
at java.lang.Thread.run(Thread.java:856)
There is only ever a single service to advertise via mDNS, so when a new
registration is requested, remove any existing ones. This should eliminate
these stacktraces:
java.lang.IllegalStateException: A service information can only be registered with a single instamce of JmDNS.
at javax.jmdns.impl.JmDNSImpl.registerService(JmDNSImpl.java:1005)
at org.fdroid.fdroid.localrepo.LocalRepoService$5.run(LocalRepoService.java:239)
at java.lang.Thread.run(Thread.java:856)
WifiStateChangeService handles updating lots of IP-related things, then
things that depend on it listen to the broadcast from that Service. The
most straightforward way to update HTTPS or HTTP throughout the app is to
trigger this Service. It runs its stuff in an AsyncTask so it is all low
priority.
We only ever want a single LocalRepoService. Use the values returned by
the standard methods for start/stop and bind/unbind as the test for whether
the Service is indeed running.
getApplicationContext() returns the Context of the application, which is
guaranteed to have the same life as the app itself. Other Contexts, like
an Activity, might go away during runtime.
As far as I can tell, the 'url' metadata in index.xml is not used at all by
the client. In order to keep it up-to-date in the local repo, it would
have to regenerate index.xml and index.jar each time the IP address
changed. That would mean a decent amount of work happening in the
background, all the update an unused field in index.xml.
There is no longer a reason to expose writeIndexXML() since FDroid should
always generate a signed repo. So make writeIndexXML() be called as part
of writeIndexJar().
Since the HTTPS certificate includes the current IP address in it, it needs
to be regenerated each time that the IP address changes. It also can take
a long time to run, especially on the first time, since it had to do things
like create a key pair and make the certificate. Therefore it should be in
a Service/AsyncTask.
Allow the local repo to use HTTPS:// instead of HTTP://. This is currently
default off since handling the self-signed certificate is not currently
graceful. In the future, the SPKI that AndroidPinning uses should be
included in the repo meta data, then when someone marks a repo as trusted,
that local repo's SPKI should be added to the list of trusted keys in
AndroidPinning.
fixes#2960https://dev.guardianproject.info/issues/2960
This makes it so the local repo is always signed by a locally generated and
stored key. That key will become the unique ID that represents a given
local repo. It should seamlessly upgrade any existing unsigned local repo
next time that the user makes any changes to their local repo.
fixes#3380https://dev.guardianproject.info/issues/3380
