Closed
Bug 1270529
Opened 8 years ago
Closed 8 years ago
Crash in java.lang.IllegalArgumentException: host=localhost, port=-1 at java.net.InetSocketAddress.<init>(InetSocketAddress.java)
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox46 affected, firefox47 fixed, firefox48 fixed, firefox49 fixed)
RESOLVED
FIXED
Firefox 49
People
(Reporter: kbrosnan, Assigned: sebastian)
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
58 bytes,
text/x-review-board-request
|
ahunt
:
review+
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
|
Details |
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 | ||
Updated•8 years ago
|
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•8 years ago
|
||
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 3•8 years ago
|
||
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+
Assignee | ||
Comment 4•8 years ago
|
||
I'll land the patch with final added. :)
Assignee | ||
Comment 5•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/7cd8232b72924c0770b65e1ea6d8c9d977710713 Bug 1270529 - lookupProxy(): Return null if port is -1. r=ahunt
Assignee | ||
Comment 6•8 years ago
|
||
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?
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7cd8232b7292
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Comment 8•8 years ago
|
||
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+
Comment 9•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/898a985fd269 https://hg.mozilla.org/releases/mozilla-beta/rev/d1d5be37e3d0
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•