Closed Bug 1270529 Opened 4 years ago Closed 4 years ago

Crash in java.lang.IllegalArgumentException: host=localhost, port=-1 at java.net.InetSocketAddress.<init>(InetSocketAddress.java)

Categories

(Firefox for Android :: General, defect)

Unspecified
Android
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Firefox 49
Tracking Status
firefox46 --- affected
firefox47 --- fixed
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: kbrosnan, Assigned: sebastian)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-1be175a6-1d75-4256-9dcf-232e72160503.
=============================================================

java.lang.IllegalArgumentException: host=localhost, port=-1
	at java.net.InetSocketAddress.<init>(InetSocketAddress.java:94)
	at java.net.InetSocketAddress.createUnresolved(InetSocketAddress.java:122)
	at org.mozilla.gecko.util.ProxySelector.lookupProxy(ProxySelector.java:89)
	at org.mozilla.gecko.GeckoAppShell.getProxyForURI(GeckoAppShell.java:2586)
	at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
	at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:443)

Crashing when we get a negative number for the proxy values? Seems like something simple to guard. Sebastian you are the only other person besides mfinkle who has touched the file.
Flags: needinfo?(s.kaspari)
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
I'll have a look.
Flags: needinfo?(s.kaspari)
We already return null if the host is empty and all callers seem to handle null values.

Review commit: https://reviewboard.mozilla.org/r/51401/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51401/
Attachment #8750413 - Flags: review?(ahunt)
Comment on attachment 8750413 [details]
MozReview Request: Bug 1270529 - lookupProxy(): Return null if port is -1. r?ahunt

https://reviewboard.mozilla.org/r/51401/#review48463

LGTM!

::: mobile/android/base/java/org/mozilla/gecko/util/ProxySelector.java:101
(Diff revision 1)
>       * Returns the proxy identified by the {@code hostKey} system property, or
>       * null.
>       */
> +    @Nullable
>      private Proxy lookupProxy(String hostKey, String portKey, Proxy.Type type, int defaultPort) {
>          String host = System.getProperty(hostKey);

We could make this final while we're touching this code? On the other hand maybe not if we want to avoid too much churn (e.g. if this is being uplifted).

::: mobile/android/base/java/org/mozilla/gecko/util/ProxySelector.java:106
(Diff revision 1)
>          String host = System.getProperty(hostKey);
>          if (TextUtils.isEmpty(host)) {
>              return null;
>          }
>  
>          int port = getSystemPropertyInt(portKey, defaultPort);

Same here - do want to make this final?
Attachment #8750413 - Flags: review?(ahunt) → review+
I'll land the patch with final added. :)
Comment on attachment 8750413 [details]
MozReview Request: Bug 1270529 - lookupProxy(): Return null if port is -1. r?ahunt

Approval Request Comment

[Feature/regressing bug #]: Crash, see comment 0.

[User impact if declined]: App can crash if proxy with invalid port is returned by system.

[Describe test coverage new/current, TreeHerder]: -

[Risks and why]: Low. Added additional check for port.

[String/UUID change made/needed]: -
Attachment #8750413 - Flags: approval-mozilla-beta?
Attachment #8750413 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/7cd8232b7292
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Comment on attachment 8750413 [details]
MozReview Request: Bug 1270529 - lookupProxy(): Return null if port is -1. r?ahunt

Fairly high volume crash on release. Let's uplift this fix to land in beta 5 or possibly beta 6.
Attachment #8750413 - Flags: approval-mozilla-beta?
Attachment #8750413 - Flags: approval-mozilla-beta+
Attachment #8750413 - Flags: approval-mozilla-aurora?
Attachment #8750413 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.