1858377 - Proxy Use KO On Android 12 13 14 After Latest Google Update

Status: VERIFIED FIXED

(Fenix :: General, defect, P1)

Description

[romain.madala]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/118.0

Steps to reproduce:

Proceeded to the update of latest Google app.
Setup a PROXY IP/3128 on my WIFI connection on my S21 latest Android 13.
Note that we've seen it too on latest android 12 and on beta Android 14

Actual results:

Firefox tries to reach PROXY on TCP 80 instead of 3128 TCP
i.e. logs from my proxy with used PORT at the end of line (Note I opened 80 too on squid so that I can show it to you)
192.168.90.49 [20231011.073333] "CONNECT push.prod.netflix.com:443 HTTP/1.1" 200 - 4693 900533 TCP_TUNNEL:HIER_DIRECT "okhttp/4.7.2" 3128
192.168.90.49 [20231011.082432] "CONNECT safebrowsing.googleapis.com:443 HTTP/1.1" 200 - 12595 147 TCP_TUNNEL:HIER_DIRECT "Mozilla/5.0 (Android 13; Mobile; rv:109.0) Gecko/118.0 Firefox/118.0" 80

Expected results:

Firefox should have tried to connect PROXY on 3128 as configured in settings.

NB : might have something to do with this
https://github.com/mozilla/gecko-dev/blob/master/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java#L1305

given

https://cs.android.com/android/_/android/platform/libcore/+/220684b95b96946c6a24b0fe4bb528b401666781

Comment 1

[Renaud Lehoux]

I confirm this issue, and participated in the initial investigation.

Our understanding is that this is due to the fact that since Android upgraded to Java 17, calling toString() on an InetSocketAddress object typically returns a string with the following format: "foo/<unresolved>:3128", instead of the expected "foo:3128", and this why Firefox sends a request on the default port 80 instead of 3128.

To be more precise, we think that the issue here is with the following line:

return "PROXY " + proxy.address().toString();

Where proxy is one of the items returned by ProxySelector. proxy.address() is a SocketAddress object, actually a InetSocketAddress.
As documented here, calling toString() on an InetSocketAddress is expected to return a string with the following format: "foo/<unresolved>:80" when the InetSocketAddress object is unresolved, which is typically the case in Android (see here and here)

The change of the syntax in the return value of InetSocketAddress.toString() was introduced with this commit in Android:
https://cs.android.com/android/_/android/platform/libcore/+/220684b95b96946c6a24b0fe4bb528b401666781

A fix would be to have something like

return "PROXY " + proxy.address().getHostString() + ":" + proxy.address().getPort();

Comment 3

[jackyzy823]

I made a modified geckoview example apk (Note: the link will be expired in 1w or 20 downloads) with following changes:

diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
index e55e7ba8d3e67..0a16d7c00669f 100644
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
@@ -56,6 +56,7 @@ import android.webkit.MimeTypeMap;
 import androidx.annotation.Nullable;
 import androidx.collection.SimpleArrayMap;
 import androidx.core.content.res.ResourcesCompat;
+import java.net.InetSocketAddress;
 import java.net.Proxy;
 import java.nio.ByteBuffer;
 import java.util.List;
@@ -1300,11 +1301,15 @@ public class GeckoAppShell {
       return "DIRECT";
     }

+    final InetSocketAddress proxyAddress = (InetSocketAddress) proxy.address();
+    final String proxyHost = proxyAddress.getHostString();
+    final int proxyPort = proxyAddress.getPort();
+
     switch (proxy.type()) {
       case HTTP:
-        return "PROXY " + proxy.address().toString();
+        return "PROXY " + String.format("%s:%d", proxyHost, proxyPort);
       case SOCKS:
-        return "SOCKS " + proxy.address().toString();
+        return "SOCKS " + String.format("%s:%d", proxyHost, proxyPort);
     }

     return "DIRECT";

I'm not sure whether geckoview_exmaple apk will also trigger this bug. but you can install the offical built one first and see if the bug exists and then uninstall it (because mine is self signed) and then install the modified one.

If the offical one cannot trigger this bug ,then i will build a modified fenix packaging modified geckoview.

---

Besides, there's a concern about getHostString , it is introduced in API 19 , which is higher than the min sdk (API 16) requirement of Geckoview. We need a condition to protect this.

could be:

diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
index e55e7ba8d3e67..df8491b6f01c5 100644
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
@@ -56,6 +56,7 @@ import android.webkit.MimeTypeMap;
 import androidx.annotation.Nullable;
 import androidx.collection.SimpleArrayMap;
 import androidx.core.content.res.ResourcesCompat;
+import java.net.InetSocketAddress;
 import java.net.Proxy;
 import java.nio.ByteBuffer;
 import java.util.List;
@@ -1300,11 +1301,21 @@ public class GeckoAppShell {
       return "DIRECT";
     }

+    final String proxyString;
+    if ( Build.VERSION.SDK_INT >= 19) {
+      final InetSocketAddress proxyAddress = (InetSocketAddress) proxy.address();
+      final String proxyHost = proxyAddress.getHostString();
+      final int proxyPort = proxyAddress.getPort();
+      proxyString = String.format("%s:%d", proxyHost, proxyPort);
+    } else {
+      proxyString = proxy.address().toString();
+    }
+
     switch (proxy.type()) {
       case HTTP:
-        return "PROXY " + proxy.address().toString();
+        return "PROXY " + proxyString;
       case SOCKS:
-        return "SOCKS " + proxy.address().toString();
+        return "SOCKS " + proxyString;
     }

     return "DIRECT";

Comment 4

[reconman]

@jackyzy823 Official one has the bug, your modified one works. I was the reporter of the "duplicate" bug that was created first.

Comment 5

[jackyzy823]

Attachment: Bug 1858377 - Use correct proxy string format for GeckoView. r=#geckoview-reviewers

Comment 6

[jackyzy823]

Thanks for your test.

I created a patch. Could you also help testing the build of the patch. Thanks.

Comment 7

[reconman]

Latest patch also works.

Comment 8

[reconman]

I would classify this bug as S2 since it actually made me switch to Brave.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aeeba1a3e215
Use correct proxy string format for GeckoView. r=geckoview-reviewers,m_kato

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/aeeba1a3e215

Comment 13

[Makoto Kato [:m_kato]]

Comment on attachment 9358494 [details]
Bug 1858377 - Use correct proxy string format for GeckoView. r=#geckoview-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: User cannot use Proxy server on Android
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: 1. Set up proxy server

2. Set proxy settings to Android's network in advanced network settings.
3. Launch Firefox then access any web site

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Change proxy string text to correct format.
• String changes made/needed: N/A
• Is Android affected?: Yes

Comment 14

[Dianna Smith [:diannaS]]

Comment on attachment 9358494 [details]
Bug 1858377 - Use correct proxy string format for GeckoView. r=#geckoview-reviewers

Approved for 120.0b3

Comment 15

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/585039d1ede6

Comment 16

[Delia Pop (Android QA)]

Hello, in order to verify the issue we would need the proxy server address and port.
:jackyzy823, could you share what proxy server should we use or other useful details to help us test this? Thanks!

Comment 17

[jackyzy823]

Hello , Delia Pop .

I'm not quite sure because i don't have a device which could reproduce this bug . I'm also not sure about what device and which android version will trigger this bug. An affected user helps to validate the patch.

If you need to test , you could setup a squid proxy ( Note you should use non-default-80 port ) mentioned in bug 1860389. and use this proxy via Android Settings -> WiFi -> Edit Current using SSID -> Advanced Options -> Proxy -> Manual -> fill up the IP/host and port.

You could also use PCAPDroid mentioned in bug 1853574 to observe the traffic to the correct port.

Comment 18

[Delia Pop (Android QA)]

Attachment: 1698328094601.JPEG

Thank you!
I was able to verify with the PCAPDroid app that the desired port selected is displayed, and not the default port.
Tested with Samsung Galaxy A53 5G (Android 13) on latest Nightly 121.0a1 from 10/26.
This will also be verified on Beta 120.0b3.

Comment 19

[Delia Pop (Android QA)]

Verified fixed on Beta 120.0b3 as well with Samsung Galaxy A53 5G (Android 13).

Comment 20

[Donal Meehan [:dmeehan]]

Comment on attachment 9358494 [details]
Bug 1858377 - Use correct proxy string format for GeckoView. r=#geckoview-reviewers

Approved for 119.0.1 and Fenix/Focus 119.1.0

Comment 21

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/24e5dfee2f73

Comment 22

[Delia Pop (Android QA)]

Attachment: image_2023_11_07T14_39_20_681Z.png

Verified fixed on Fenix 119.1.0 as well with Samsung Galaxy A53 5G (Android 13).