740640 - B2G 3G: Support HTTP proxy

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Philipp von Weitershausen [:philikon]]

Some mobile data providers require HTTP proxies, sadly. (E.g. T-Mobile US.) We should support them.

Comment 1

[Philipp von Weitershausen [:philikon]]

Tentatively assigning to jaoo. Feel free to remove yourself again if you can't work on it.

Comment 2

[José Antonio Olivera Ortega [:jaoo]]

After some researching I have figured out how Android supports HTTP proxy for data calls. I'll try to describe it you guys at a very high level of abstraction as follow. The Android setting application allows a user to set up the APN parameters for the data call (http proxy parameters included). That application stores the APN
parameters in the telephony data base and the telephony provider notifies the system about it. The GSMDataConnectionTracker object watches for APN changes in the data base and sets up the data call. Once the data call setup is done the GsmDataConnectionTracker.onDataSetupComplete() method sets the proxy conf. via the DataConnectionAC object by using the setLinkPropertiesHttpProxySync() method. At the end the proxy settings are set with the mLinkProperties.setHttpProxy(proxy) method. After that dance a broadcast notification is sent, including the LinkProperties object. At the end the ConnectivityService receives that LinkProperties data and configures routes, dns and proxies. For proxies that configuration process means contacting every java VM in every process and setting the java properties http.{proxyHost, proxyPort}.

The thing here is how to implement that way of configuring proxy support for data calls in B2G. We cannot replicate it because there are no java VMs. The first idea I have is to handle that proxy configuration in the network manager. If the proxy settings come together with the APN setting let the network manager configures them if the data call is the active network connection. But...How could the network manager configure the proxy setting? That's the thing! My question here is, what about configuring at this point the proxy setting for gecko as described at [1]?.

Please some feedback is really appreciated!

[1] https://developer.mozilla.org/en/Mozilla_Embedding_FAQ/How_do_I...#How_do_I_set_the_network_proxy.3F

Comment 3

[Philipp von Weitershausen [:philikon]]

I see two basic options:

(a) Have the RILNetworkInterface set/unset proxy settings. This would probably keep things a bit simpler. The only thing we might want to do is have the NetworkManager notify a NetworkInterface when it becomes the active one (e.g. via an `onStatusChanged()` method call).

(b) Have the NetworkManager set the proxy information, based on information provided by the NetworkInterface. In this case we probably want to extend nsINetworkInterface to expose the proxy settings so that the NetworkManager can pick them up. This would basically tie the HTTP proxy settings to a specific network link. I don't see any other way, though.

I don't have a strong preference for either one, to be honest. I think the proxy thing is pretty specific to the RIL, so I would probably lean towards (a).

Comment 4

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Philipp von Weitershausen [:philikon] from comment #3)
> (b) Have the NetworkManager set the proxy information, based on information
> provided by the NetworkInterface. In this case we probably want to extend
> nsINetworkInterface to expose the proxy settings so that the NetworkManager
> can pick them up. This would basically tie the HTTP proxy settings to a
> specific network link. I don't see any other way, though.
> 
> I don't have a strong preference for either one, to be honest. I think the
> proxy thing is pretty specific to the RIL, so I would probably lean towards
> (a).

IMHO I would probably lean towards 'b' because it ties the HTTP proxy settings to a specific network link and allows us to make the mechanism extensible for every network interface. Let's think about the WiFi case, nowadays many WiFi networks have proxies for reaching the Internet and option 'b' will cover the problem. Am I explaining myself?

Comment 5

[José Antonio Olivera Ortega [:jaoo]]

Attachment: WIP v1

This WIP patch addresses what philikon suggested about option b in comment #3. Would it work configuring the proxy setting for gecko as described at [1]?.

[1] https://developer.mozilla.org/en/Mozilla_Embedding_FAQ/How_do_I...#How_do_I_set_the_network_proxy.3F

Comment 6

[Philipp von Weitershausen [:philikon]]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #4)
> (In reply to Philipp von Weitershausen [:philikon] from comment #3)
> > (b) Have the NetworkManager set the proxy information, based on information
> > provided by the NetworkInterface. In this case we probably want to extend
> > nsINetworkInterface to expose the proxy settings so that the NetworkManager
> > can pick them up. This would basically tie the HTTP proxy settings to a
> > specific network link. I don't see any other way, though.
> > 
> > I don't have a strong preference for either one, to be honest. I think the
> > proxy thing is pretty specific to the RIL, so I would probably lean towards
> > (a).
> 
> IMHO I would probably lean towards 'b' because it ties the HTTP proxy
> settings to a specific network link

So does option (a).

> and allows us to make the mechanism extensible for every network interface.

But why do we need that?

> Let's think about the WiFi case,
> nowadays many WiFi networks have proxies for reaching the Internet and
> option 'b' will cover the problem. Am I explaining myself?

When did you ever have to set a proxy for a Wifi connection? I have never ever when using my phone.

Comment 7

[Philipp von Weitershausen [:philikon]]

Comment on attachment 642365 [details] [diff] [review]
WIP v1

Review of attachment 642365 [details] [diff] [review]:
-----------------------------------------------------------------

Provided we really do go with option (b) (I still prefer (a)), here's my feedback:

::: dom/system/gonk/NetworkManager.js
@@ +187,5 @@
>        ifname: this.active.name,
> +      oldIfname: (oldInterface && oldInterface != this.active) ? oldInterface.name : null,
> +      httpProxy: this.active.httpProxyHost ?
> +                 {host: this.active.httpProxyHost, port: this.active.httpProxyPort} :
> +                 null

Let's keep this object flat and just inline `httpProxyHost` and `httpProxyPort` as top-level attributes. You can just copy them over:

    ...
    httpProxyPort: this.active.httpProxyPort,
  };

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1699,5 @@
>    dhcp: false,
>  
> +  httpProxyHost: null,
> +
> +  httpProxyPort: 8080,

Makes this `null` too?

::: dom/system/gonk/net_worker.js
@@ +58,5 @@
>    // Bump the DNS change property.
>    let dnschange = libcutils.property_get("net.dnschange", "0");
>    libcutils.property_set("net.dnschange", (parseInt(dnschange, 10) + 1).toString());
> +
> +  if(options.httpProxy) {

Nit: space after `if`

Comment 8

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Philipp von Weitershausen [:philikon] from comment #6)
> When did you ever have to set a proxy for a Wifi connection? I have never
> ever when using my phone.

In some cases users typically access The Internet and their email while connected to the corporate WiFi and that WiFi network typically reaches the internet through a proxy (even with proxy authentication). Once I visited a company where the WiFi connection was configured in that way.

Comment 9

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 1 - v2

Added support for proxy configuration in NetworkManager.

Comment 10

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 2 - v1

Changes needed for adding proxy data to settings. It depends on bug 775814.

Comment 11

[José Antonio Olivera Ortega [:jaoo]]

Actually I've testing the patches by hardcoding all apn data in code because if I set the apn data through the setting application something is wrong. After configuring the apn data I click on OK and only ril.data.passwd setting gets updated. For that reason the RIL does not set up call properly. Taking a look at because it is so weird.

Comment 12

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Part 3 -v1

This is another unrelated patch for this bug I guess :(. I do here something that was on my TODO list, that is to retrieve APN setting through SettingsService so we do not use the pref service anymore. The reason because I am attaching this patch here is because while I have been testing the problem I see with the setting app I have decided to change current implementation to this one. I am still seeing a problem when the setting app tries to save the APN settings. The thing with that problem is that only the setting of the last field on the APN setting list gets update after clicking on the OK button.

Comment 13

[José Antonio Olivera Ortega [:jaoo]]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #12)
> Created attachment 644521 [details] [diff] [review]
> Part 3 -v1
> 
>I am still seeing a problem when the setting app
> tries to save the APN settings. The thing with that problem is that only the
> setting of the last field on the APN setting list gets update after clicking
> on the OK button.

Solved in Gaia PR. https://github.com/mozilla-b2g/gaia/pull/2690.

Comment 14

[Philipp von Weitershausen [:philikon]]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #8)
> (In reply to Philipp von Weitershausen [:philikon] from comment #6)
> > When did you ever have to set a proxy for a Wifi connection? I have never
> > ever when using my phone.
> 
> In some cases users typically access The Internet and their email while
> connected to the corporate WiFi and that WiFi network typically reaches the
> internet through a proxy (even with proxy authentication). Once I visited a
> company where the WiFi connection was configured in that way.

Does any smart phone currently on the market have the capabilities (or at least UI) to use an HTTP proxy over Wifi? Anecdotal evidence is irrelevant.

Comment 15

[Philipp von Weitershausen [:philikon]]

Comment on attachment 644380 [details] [diff] [review]
Part 1 - v2

Review of attachment 644380 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/NetworkManager.js
@@ +192,5 @@
>    },
>  
> +  setNetworkProxy: function setNetworkProxy() {
> +    try {
> +      if (!this.active.httpProxyHost || this.active.httpProxyHost == "") {

The 2nd test here is superfluous.

@@ +194,5 @@
> +  setNetworkProxy: function setNetworkProxy() {
> +    try {
> +      if (!this.active.httpProxyHost || this.active.httpProxyHost == "") {
> +        // Sets direct connection to internet.
> +        Services.prefs.setIntPref("network.proxy.type", 0);

Better:

  Services.prefs.clearUserPref("network.proxy.type");
  Services.prefs.clearUserPref("network.proxy.http");

@@ +201,5 @@
> +      }
> +
> +      debug("Going to set proxy settings for " + this.active.name + " network interface.");
> +      // Sets manual proxy configuration. 
> +      Services.prefs.setIntPref("network.proxy.type", 1);    

const the `1` to a meaningful name.

@@ +202,5 @@
> +
> +      debug("Going to set proxy settings for " + this.active.name + " network interface.");
> +      // Sets manual proxy configuration. 
> +      Services.prefs.setIntPref("network.proxy.type", 1);    
> +      // XXX: Use this proxy server for all protocols. Do we want that behaviour?

Pretty sure we don't.

@@ +210,5 @@
> +        8080 : this.active.httpProxyPort;
> +      Services.prefs.setIntPref("network.proxy.http_port", port);
> +      // TODO: Figure out how to deal with authentication support. 
> +     } catch (ex) {
> +       debug("Unable to set proxy setting for " + this.active.name + " network interface.");

Also add `ex` to the error message here.

Comment 16

[José Antonio Olivera Ortega [:jaoo]]

Attachment: WIP v3

It would be great to have a double ckeck for this patch. I mean someone else who test the patch with a carrier that needs proxy support for data calls. Telefonica's APN needs proxy support but the same APN works without proxy support as well.

Comment 17

[Philipp von Weitershausen [:philikon]]

Comment on attachment 645798 [details] [diff] [review]
WIP v3

Review of attachment 645798 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the review comments in bug 776294 addressed.

Comment 18

[José Antonio Olivera Ortega [:jaoo]]

Attachment: WIP v4

This patch include some modification because it depends on bug 776294. Do I need to request review again since it was already r+'ed?

Comment 19

[José Antonio Olivera Ortega [:jaoo]]

Gaia pull request https://github.com/mozilla-b2g/gaia/pull/2898 for including proxy related fields in the Setting App.

Comment 20

[Philipp von Weitershausen [:philikon]]

Comment on attachment 646553 [details] [diff] [review]
WIP v4

Review of attachment 646553 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/NetworkManager.js
@@ +212,5 @@
> +      Services.prefs.setBoolPref("network.proxy.share_proxy_settings", false);
> +      Services.prefs.setCharPref("network.proxy.http", this.active.httpProxyHost);
> +      let port = this.active.httpProxyPort == "" ? 8080 : this.active.httpProxyPort;
> +      Services.prefs.setIntPref("network.proxy.http_port", port);
> +      // TODO: Figure out how to deal with authentication support.

If this is an actual TODO, please file a bug and mention the bug number here. But I don't think we need proxy authentication, so I would just leave this comment out.

Comment 21

[José Antonio Olivera Ortega [:jaoo]]

Attachment: WIP v5

Comment 22

[Philipp von Weitershausen [:philikon]]

What's the status? Can this be pushed? You have L3 rights, jaoo, right?

Comment 23

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Philipp von Weitershausen [:philikon] from comment #22)
> What's the status? Can this be pushed? 

It can be pushed. Just finished a test again after a clean build and it seems everything is working.

> You have L3 rights, jaoo, right?

No, I do not. I do not have L3 rights. Some time ago you vouched for me for L1 rights and that's the level I have.

Comment 24

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #23)
Hi jaoo, 
Do you need help to push that patch? I can commit for you.
But I am not sure it's ready or not, since it's marked "WIP".

Comment 25

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #24)
> (In reply to José Antonio Olivera Ortega [:jaoo] from comment #23)
> Hi jaoo, 
> Do you need help to push that patch? I can commit for you.
> But I am not sure it's ready or not, since it's marked "WIP".

Yes, it's marked as "WIP" (sorry, my fault), but it's ready. This patch depends on bug 776294 so this one has to land after bug 776294. Bug 776294 is ready as well so with philikon's permission go ahead if you want. I cannot do it since I do not have L3 rights.

Comment 26

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3041cf7e42ed

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3041cf7e42ed