Closed Bug 1504968 Opened 6 years ago Closed 5 years ago

Fix missing triggeringPrincipal from android code

Categories

(Firefox for Android Graveyard :: General, enhancement, P2)

enhancement

Tracking

(firefox75 fixed)

RESOLVED FIXED
Firefox 75
Tracking Status
firefox75 --- fixed

People

(Reporter: jkt, Assigned: ckerschb)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

In Bug 1490257 we made a carve out for the asserts for android. This is mainly due to browser.js in android browsers not passing triggering principals in most places. This bug is so we follow up on that and require this everywhere.
Depends on: 1513241

Some notes: It seems we don't have the right principal on all of android, within Bug 1513241 I encountered some of those problems, see also [1].

Within this Bug we should update loadURIOptions.webidl and change the principal setting to:

required Principal? triggeringPrincipal;

If we do that currently, we face problems like [1], in detail:

0 loadURI/<() ["chrome://global/content/elements/browser-custom-element.js":809:6]
1 _wrapURIChangeCall() ["chrome://global/content/elements/browser-custom-element.js":749:8]
2 loadURI() ["chrome://global/content/elements/browser-custom-element.js":808:4]
3 create() ["chrome://browser/content/browser.js":3855:8]
4 Tab() ["chrome://browser/content/browser.js":3642:2]
5 addTab() ["chrome://browser/content/browser.js":1216:17]
6 onEvent() ["chrome://browser/content/browser.js":1965:10]
Handled load error: TypeError: Missing required 'triggeringPrincipal' member of LoadURIOptions

It seems that |this.browser.contentPrincipal| within mobile/android/chrome/content/browser.js is null. So probably we should start digging there.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=61ab3e53d35dba0a5db9c20394a7000cd301bef7

Could it be that this just works now in Fenix? I set security.strict_security_checks.enabled to true and don't see such an error in the remote debugging browser console. Furthermore I can switch to different geckoview.xhtml tabs and see a non-null, seemingly correct this.browser.contentPrincipal. I suppose this is due to GeckoView? Since Android Nightly switched to Fenix by now, this could maybe be fixed and ride the trains?

Flags: needinfo?(ckerschb)

(In reply to Johannes Pfrang [:johnp] from comment #2)

Could it be that this just works now in Fenix? I set security.strict_security_checks.enabled to true and don't see such an error in the remote debugging browser console. Furthermore I can switch to different geckoview.xhtml tabs and see a non-null, seemingly correct this.browser.contentPrincipal. I suppose this is due to GeckoView? Since Android Nightly switched to Fenix by this could maybe be fixed and ride the trains?

Yes, that's entirely possible that that just works using Fenix. I would be happy to remove "security.strict_security_checks.enabled". If you want me to, I'll take on that bug. Would be really happy if that is really true and we have the right triggeringPrincipal on Android now.

Let me know!

Flags: needinfo?(ckerschb) → needinfo?(johnp)

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #3)

(In reply to Johannes Pfrang [:johnp] from comment #2)

Could it be that this just works now in Fenix? I set security.strict_security_checks.enabled to true and don't see such an error in the remote debugging browser console. Furthermore I can switch to different geckoview.xhtml tabs and see a non-null, seemingly correct this.browser.contentPrincipal. I suppose this is due to GeckoView? Since Android Nightly switched to Fenix by this could maybe be fixed and ride the trains?

Yes, that's entirely possible that that just works using Fenix. I would be happy to remove "security.strict_security_checks.enabled". If you want me to, I'll take on that bug. Would be really happy if that is really true and we have the right triggeringPrincipal on Android now.

Let me know!

I'll just try that - if we could enforce strict top-level principal checks, that would make me just feel so much better!

Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Flags: needinfo?(johnp)
Pushed by shindli@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/739832ead584 Remove pref security.strict_security_checks.enabled and enforce strict top-level principal checking on Android. r=snorp,baku
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 75
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: