Closed
Bug 1422902
Opened 6 years ago
Closed 6 years ago
Crash found by partner Unable to start activity ComponentInfo java.lang.IllegalArgumentException: Must pass a non-empty manifest URL
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox-esr52 unaffected, firefox57 disabled, firefox58 fixed, firefox59 fixed)
RESOLVED
FIXED
Firefox 59
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox57 | --- | disabled |
firefox58 | --- | fixed |
firefox59 | --- | fixed |
People
(Reporter: mkaply, Assigned: droeh)
Details
Attachments
(2 files)
123.62 KB,
text/plain
|
Details | |
4.09 KB,
patch
|
snorp
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Partner was doing fuzz testing using drozer and got the attached crash.
Reporter | ||
Comment 1•6 years ago
|
||
Partner is saying this happens with adb shell am start -n org.mozilla.firefox/org.mozilla.gecko.webapps.WebAppActivity but I can't recreate.
Comment 2•6 years ago
|
||
I think WebAppActivity isn't exported, so this only works with the individually numbered copies we use for separating different web apps in the task switcher (compare the component name in the logs).
Reporter | ||
Comment 3•6 years ago
|
||
(In reply to Jan Henning [:JanH] from comment #2) > I think WebAppActivity isn't exported, so this only works with the > individually numbered copies we use for separating different web apps in the > task switcher (compare the component name in the logs). Can you give more detail on exactly how you think I could recreate the crash to try to fix?
Comment 4•6 years ago
|
||
> adb shell am start -a android.intent.action.MAIN -n org.mozilla.fennec/org.mozilla.gecko.webapps.WebApps\$WebApp0
should hopefully work
Reporter | ||
Comment 5•6 years ago
|
||
I assume you meant org.mozilla.firefox I get Error: Activity class {org.mozilla.firefox/org.mozilla.gecko.webapps.WebApps} does not exist. Nothing I do gets the $ and info after it passed along.
Comment 6•6 years ago
|
||
(In reply to Mike Kaply [:mkaply] from comment #5) > I assume you meant org.mozilla.firefox I was testing with Nightly, so yes, that needs changing accordingly. > Error: Activity class > {org.mozilla.firefox/org.mozilla.gecko.webapps.WebApps} does not exist. > > Nothing I do gets the $ and info after it passed along. You need to escape the "$" so it doesn't get gobbled up by the shell (initially I had the same problem as you're having now). On Windows, prepending a "\" (look more closely at the line I posted above) does the trick and I assume Linux/Mac aren't any different there.
Reporter | ||
Comment 7•6 years ago
|
||
In case someone elase runs across this, on Mac you have to escape and quote db shell am start -a android.intent.action.MAIN -n 'org.mozilla.firefox/org.mozilla.gecko.webapps.WebApps\$WebApp0' and I recreated the crash.
Reporter | ||
Comment 8•6 years ago
|
||
So we're crashing here with the ADB command: https://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/webapps/WebAppManifest.java#44 Questions is what should we be doing here.
Flags: needinfo?(snorp)
This is kind of a contrived case, since we should always have the intent extras, but I guess we shouldn't crash. Dylan, can you bail out here similarly to how we've done in other cases?
Flags: needinfo?(snorp) → needinfo?(droeh)
Assignee | ||
Comment 10•6 years ago
|
||
This handles the exception and null-checks getIntent().getData(), so it should fail a bit more gracefully. I also updated WebAppManifest.fromFile() to either throw an exception or return a valid manifest, so that we don't have to both catch exceptions and null-check the result.
Attachment #8935903 -
Flags: review?(snorp) → review+
Comment 11•6 years ago
|
||
Pushed by droeh@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f5461454d0b9 Handle exceptions from WebAppManifest code rather than crashing. r=snorp
Comment 12•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f5461454d0b9
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Reporter | ||
Comment 13•6 years ago
|
||
Dylan: Can you request beta uplift for this? Thanks.
Assignee | ||
Comment 14•6 years ago
|
||
Comment on attachment 8935903 [details] [diff] [review] Handle exceptions from WebAppManifest Approval Request Comment [Feature/Bug causing the regression]:1360068 [User impact if declined]:Potential crash if a malformed intent is sent to launch a PWA [Is this code covered by automated tests?]:No [Has the fix been verified in Nightly?]:Yes [Needs manual test from QE? If yes, steps to reproduce]:No [List of other uplifts needed for the feature/fix]:None [Is the change risky?]:No [Why is the change risky/not risky?]:Simply handles a previously unhandled exception. [String changes made/needed]:None
Attachment #8935903 -
Flags: approval-mozilla-beta?
Comment 15•6 years ago
|
||
Comment on attachment 8935903 [details] [diff] [review] Handle exceptions from WebAppManifest Avoid potential crash. Beta58+.
Attachment #8935903 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•6 years ago
|
status-firefox58:
--- → affected
Updated•6 years ago
|
status-firefox57:
--- → disabled
status-firefox-esr52:
--- → unaffected
Comment 16•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/ccbe2255e35a
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
•