Open Bug 942652 Opened 6 years ago Updated 3 months ago

Proxy support for background services

Categories

(Firefox for Android :: Android Sync, defect, P5)

All
Android
defect

Tracking

()

People

(Reporter: rnewman, Unassigned, Mentored)

References

(Blocks 4 open bugs)

Details

(Whiteboard: [tor-mobile])

We need to persist and obey configured proxy data.
Blocks: 942653
Hey rnewman, any idea on how one could go about this? I'm currently working with tor and trying to create a new secure browser for android using fennec(https://github.com/amoghbl1/orfox) and solving this would be really useful for it!
Sorry for the delay getting back to you; last week was unexpectedly busy.

In short: there are several different network stacks in use in the browser as a whole.

* Gecko itself uses Necko, just like desktop. We won't concern ourselves with that here.

* Other parts (e.g., favicon loading) use Android's android.net.http.AndroidHttpClient, which is a pre-configured wrapper around the Android version of Apache HttpClient. This might or might not respect system proxy settings. This is covered in Bug 942653.

* Android Background Services (in the android-sync repo on GitHub) uses a self-contained version of Apache HttpClient (httpclientandroidlib), because targeted versions of Android (a) have a buggy snapshot of HttpClient, and (b) deprecated HttpClient.

There are two ways to approach this bug.


The most straightforward "patch on top" approach is to address each client individually: you'd fix Bug 942653, and then you'd fix this bug, by adding code in the appropriate spot to configure proxy usage in the HttpClient layer.

This might become complicated if you're needing to respect non-system proxy settings -- i.e., those specified in Firefox, or (worse) in Gecko itself. That's because Android Background Services is developed separately, and is barely coupled to Firefox at all. You won't be able to rely on Fennec code, nor on side-effects from Fennec code having run earlier in the same process.


The second approach is to investigate whether SDK 9+ are still problematic from an HTTP library standpoint, and rework android-sync's self-contained HTTP layer accordingly. If you can switch to using android.net.http or HttpURLConnection, without introducing bugs, then you'll be able to reuse implementation code between Fennec core and ABS.


So step one is to figure out exactly what you want to do with proxying (Gecko-defined? Fennec-defined? System-defined?).

Step two is to do research to decide which strategy to take.

Step three is to do it.

Make sense?
A tiny bit more backstory on this one:

> * Android Background Services (in the android-sync repo on GitHub) uses a
> self-contained version of Apache HttpClient (httpclientandroidlib), because
> targeted versions of Android (a) have a buggy snapshot of HttpClient, and
> (b) deprecated HttpClient.

See <https://code.google.com/p/httpclientandroidlib/>.

At the time Android Sync was built, it had to support Android 5+. We now support 9+. I haven't done recent research to determine if things have improved.
So as per the conversation that took place on IRC, I am going to be working on making an api call from LoadFaviconTask.java to get the proxy preferences from gecko and apply it on the httpClient object. This should solve this bug specifically but the other two related bugs are going to need a little more work. What do you think about this approach rnewman?
That would be a different bug, not this one. Favicon fetching isn't part of background services. See one of the linked bugs. 

But see the conversation snorp and I had about ten days ago: yes, fetching proxy settings from Gecko is part of the solution, but not the way you described. Define a proxy manager in Java, have it initialized by a message from Gecko, persist its state (works across Gecko lifetimes, necessary for solving this bug and pre-Gecko favicon fetches), and have a Gecko-side pref observer to handle changes to proxy settings. Fetches can then very quickly ask the proxy manager to give them a connection object that's correctly configured for proxy use. No immediate contact with Gecko required. 

There will be stumbling blocks around PAC file handling and other things. We'll cross those bridges as we come to them.

Makes sense?
OK, we have a PrefsHelper defined on the side of Java to handle preferences, why don't we extend this to save the proxy settings, I think that it's persist by default (need to check that), but this could be used to check settings and we could have gecko call and update these.
Let's go with a standalone class. It'll need to do much more work than I'd want to cram into PrefsHelper, it makes sense from a design perspective, and it'll make access from background services easier when we come to tackle this bug. 

Remember that the primary mode of operation doesn't involve Gecko at all: we would very infrequently get a change notification from Gecko. I don't mind that living somewhere else. 

See how BrowserLocaleManager is integrated with the system. ProxyManager is a much simpler sibling of that class.
Blocks: 1107569
I'm marking this mentored because we're willing to help with it, but be aware that this is a very complicated ticket.
Mentor: rnewman, nalexander
Blocks: 1209496
Duplicate of this bug: 1362931
Blocks: 1357994
Whiteboard: [tor-mobile]
Priority: -- → P1
Priority: P1 → P2
P3 unless the tor mobile devs want to bump it higher.
Priority: P2 → P3
FWIW: For Tor this is actually much higher priority. Much higher than fixing fingerprinting or cross-site tracking bugs for mobile anyway. Or to put it differently: This is a blocker for Tor Browser on Android for which we at least have to write a patch if it is still an issue.
Please let the triage flag be set during Android Sync triage.
Component: Core → Android Sync
Priority: P3 → --
Product: Android Background Services → Firefox for Android
Mentor: gkruglov
Priority: -- → P3
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.
Priority: P3 → P5
You need to log in before you can comment on or make changes to this bug.