Closed Bug 1649690 Opened 4 years ago Closed 4 years ago

Improve verification for moz:firefoxOptions' Android entries

Categories

(Testing :: geckodriver, defect, P2)

All
Android
defect

Tracking

(firefox80 fixed)

RESOLVED FIXED
mozilla80
Tracking Status
firefox80 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

Details

Attachments

(1 file, 1 obsolete file)

For running tests with geckodriver on Android some Firefox options are supported:

https://developer.mozilla.org/en-US/docs/Web/WebDriver/Capabilities/firefoxOptions#android

As I noticed at least androidPackage can be used to escape the expected location of the yaml file:

"androidPackage": "../foo"

which results in:

/mnt/sdcard/../foo/-geckodriver-profile
1593550317589 geckodriver::android DEBUG Pushing GeckoView configuration file to /data/local/tmp/../foo/-geckoview-config.yaml

Nick, would it be ok to restrict the androidPackage name to starting with org.mozilla.? Or do we have apps, which are ourside of our domain?

Flags: needinfo?(nalexander)

Also what's the best way to retrieve all the available activities for a package? The following command seems to only return the default activity:

cmd package resolve-activity --brief org.mozilla.geckoview_example | tail -n 1 | cut -d '/' -f 2

James, maybe you can help (in case Nick isn't around these days)? Please see my last two comments. Thanks.

Flags: needinfo?(snorp)

I just found the following:

➜ adb shell pm dump org.mozilla.firefox | grep ' filter' | cut -d ' ' -f 12 | sort | uniq
org.mozilla.firefox/.App
org.mozilla.firefox/com.google.android.gms.gcm.GcmReceiver
org.mozilla.firefox/com.leanplum.LeanplumPushInstanceIDService
org.mozilla.firefox/com.leanplum.LeanplumPushListenerService
org.mozilla.firefox/com.leanplum.LeanplumPushReceiver
org.mozilla.firefox/org.mozilla.gecko.PackageReplacedReceiver
org.mozilla.firefox/org.mozilla.gecko.customtabs.GeckoCustomTabsService
org.mozilla.firefox/org.mozilla.gecko.distribution.ReferrerReceiver
org.mozilla.firefox/org.mozilla.gecko.fxa.activities.FxAccountConfirmAccountActivityWeb
org.mozilla.firefox/org.mozilla.gecko.fxa.activities.FxAccountFinishMigratingActivityWeb
org.mozilla.firefox/org.mozilla.gecko.fxa.activities.FxAccountGetStartedActivityWeb
org.mozilla.firefox/org.mozilla.gecko.fxa.activities.FxAccountSignUpActivityWeb
org.mozilla.firefox/org.mozilla.gecko.fxa.activities.FxAccountStatusActivity
org.mozilla.firefox/org.mozilla.gecko.fxa.activities.FxAccountUpdateCredentialsActivityWeb
org.mozilla.firefox/org.mozilla.gecko.fxa.authenticator.FxAccountAuthenticatorService
org.mozilla.firefox/org.mozilla.gecko.fxa.receivers.FxAccountUpgradeReceiver
org.mozilla.firefox/org.mozilla.gecko.fxa.sync.FxAccountSyncService
org.mozilla.firefox/org.mozilla.gecko.gcm.GcmInstanceIDListenerService
org.mozilla.firefox/org.mozilla.gecko.gcm.GcmMessageListenerService
org.mozilla.firefox/org.mozilla.gecko.notifications.NotificationReceiver
org.mozilla.firefox/org.mozilla.gecko.notifications.WhatsNewReceiver
org.mozilla.firefox/org.mozilla.gecko.overlays.ui.ShareDialog
org.mozilla.firefox/org.mozilla.gecko.restrictions.RestrictionProvider
org.mozilla.firefox/org.mozilla.gecko.search.SearchWidgetConfigurationActivity
org.mozilla.firefox/org.mozilla.gecko.search.SearchWidgetProvider
org.mozilla.firefox/org.mozilla.mozstumbler.service.mainthread.SafeReceiver
org.mozilla.firefox/org.mozilla.mozstumbler.service.mainthread.SystemReceiver

➜ adb shell pm dump org.mozilla.geckoview_example | grep ' filter' | cut -d ' ' -f 12 | sort | uniq
org.mozilla.geckoview_example/.GeckoViewActivity

Would that match the list of possible intents to run? Maybe some of these cannot be used?

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #1)

Nick, would it be ok to restrict the androidPackage name to starting with org.mozilla.? Or do we have apps, which are ourside of our domain?

I don't see why we'd restrict quite so much -- we want other folks to use this, right?

But you're absolutely correct that we should verify the package exists on the device, or similar to avoid unsafe path computations. I'm not particularly concerned about this particular escape because the permissions of the device are likely to stop most mayhem. But it's worth addressing in any situation.

Flags: needinfo?(nalexander)

Ok, I will use a generic regex that is equal to what the following page tells about the application id:
https://developer.android.com/studio/build/application-id

Nick, could you also help regarding the other question in finding all the intents of an application? Maybe it's not necessary to do and the command will simply fail. Also I don't think intent names are easy to check with a regex.

Flags: needinfo?(nalexander)
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Attachment #9161329 - Attachment is obsolete: true

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #6)

Ok, I will use a generic regex that is equal to what the following page tells about the application id:
https://developer.android.com/studio/build/application-id

Fair enough.

Nick, could you also help regarding the other question in finding all the intents of an application? Maybe it's not necessary to do and the command will simply fail. Also I don't think intent names are easy to check with a regex.

I am not aware of a meaningful way to do this. You can list them by inspecting the manifest, but many of them won't actually do anything useful. Best to just pass through the user's Intent specification; if it's wrong, the error message is pretty clear.

Flags: needinfo?(nalexander)
Group: mozilla-employee-confidential
Flags: needinfo?(snorp)
Summary: geckodriver has to verify moz:firefoxOptions Android entries → Improve verification for moz:firefoxOptions' Android entries
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/190ba19940e5 [geckodriver] Validate android specific capabilities. r=webdriver-reviewers,nalexander,jgraham
Blocks: 1584911
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: