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)

Firefox 57
defect
Not set
normal

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)

Attached file #24.txt
Partner was doing fuzz testing using drozer and got the attached crash.
Partner is saying this happens with

adb shell am start -n org.mozilla.firefox/org.mozilla.gecko.webapps.WebAppActivity

but I can't recreate.
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).
(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?
> adb shell am start -a android.intent.action.MAIN -n org.mozilla.fennec/org.mozilla.gecko.webapps.WebApps\$WebApp0

should hopefully work
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.
(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.
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.
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)
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.
Assignee: nobody → droeh
Flags: needinfo?(droeh)
Attachment #8935903 - Flags: review?(snorp)
Attachment #8935903 - Flags: review?(snorp) → review+
Pushed by droeh@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5461454d0b9
Handle exceptions from WebAppManifest code rather than crashing. r=snorp
https://hg.mozilla.org/mozilla-central/rev/f5461454d0b9
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Dylan: Can you request beta uplift for this? Thanks.
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 on attachment 8935903 [details] [diff] [review]
Handle exceptions from WebAppManifest

Avoid potential crash. Beta58+.
Attachment #8935903 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: