noopener window.open() shouldn't inherit principal
Categories
(Core :: DOM: Navigation, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox107 | --- | fixed |
People
(Reporter: smaug, Assigned: nika)
References
(Regressed 1 open bug)
Details
Attachments
(7 files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
Per specs (HTML + URL) noopener window open uses opaque origin.
https://searchfox.org/mozilla-central/rev/df23c6e58c6be1eb8399e475878f73d4867bef87/toolkit/components/windowwatcher/nsWindowWatcher.cpp#1248 should probably use LOAD_FLAGS_DISALLOW_INHERIT_PRINCIPAL when noopener is passed to window.open.
Reporter | ||
Comment 1•4 years ago
|
||
This is low priority, since it is very hard to detect this.
Reporter | ||
Updated•4 years ago
|
Updated•4 years ago
|
Comment 3•3 years ago
|
||
Changing severity to S4 because N/A results in autonag emails.
Assignee | ||
Comment 4•2 years ago
|
||
Previously we would pull this information frequently from the subject
principal, which is unreliable. With this new approach, we more explicitly pass
the principals around as-needed into where they're going to be used.
Some assertions about the subject principal were introduced to ensure that
assumptions made about chrome windows and the system principal are not
incorrect.
Updated•2 years ago
|
Assignee | ||
Comment 5•2 years ago
|
||
When opening a window with noopener
, we will no longer inherit the
subject principal into the newly created window's initial about:blank
document, instead creating a new null principal.
This patch also makes the system/expanded principal -> null principal
translation happen earlier (it previously happened in
SetInitialPrincipalToSubject), so that it can be followed more easily
when reading the code.
Finally, the load started by nsWindowWatcher in new windows is updated
to specify LOAD_FLAGS_DISALLOW_INHERIT_PRINCIPAL
when noopener is
specified such that the explicit about:blank load also cannot inherit
the subject principal.
This change does make it so that the global is not re-used between the
initial and loaded about:blank document, however this shouldn't be
visible due to noopener
being specified, preventing any references to
the initial document from existing.
Noopener loads with javascript:
URIs will be rejected early during the
load due to the mismatch between the triggering principal and the
initial about:blank document's principal.
Depends on D154563
Assignee | ||
Comment 6•2 years ago
|
||
The newBC
and newDocShell
variables were potentially confusing, as they
could also be existing windows selected by named targeting or the window
provider, so they have been renamed. Some other variables were also renamed for
consistency and clarity.
Depends on D154564
Assignee | ||
Comment 7•2 years ago
|
||
Previously we would copy session storage even if we were not opening a new
window, meaning that a targeted load could re-trigger a copy. This was not
specified in the standard so is being changed to only copy when a new window is
created. In addition, the copy was moved before navigaton starts, again for
more consistency with ordering for the standard, such that things like
javascript:
URI loads will oberve the up-to-date session storage.
Depends on D154565
Assignee | ||
Comment 8•2 years ago
|
||
Depends on D154566
Comment 11•2 years ago
|
||
Backed out for bc failure on browser_cross_process_csp_inheritance.js
Backout link: https://hg.mozilla.org/integration/autoland/rev/4e648a00f8a1282918758dcd928ef8bdd808cdac
Log link: https://treeherder.mozilla.org/logviewer?job_id=388090744&repo=autoland&lineNumber=20104
Updated•2 years ago
|
Assignee | ||
Comment 13•2 years ago
|
||
If we don't preserve the precursor principal in this case, we'll end up
doing an unnecessary process switch in some cases, which lead to a
test failure.
This patch also cleans up some logic around the first party origin
attribute with null principals, as the logic was only used in one place
and generally added some unnecessary complexity to NullPrincipal
itself.
Depends on D154567
Comment 14•2 years ago
|
||
Comment 15•2 years ago
|
||
Backed out for failures on test_alwaysOnTop_windows.html
- backout: https://hg.mozilla.org/integration/autoland/rev/6425a97c5857cc708bb41423cd780506d15161ad
- push: https://treeherder.mozilla.org/jobs?repo=autoland&revision=4d404c79681a03b30932648a722853a2e6d0f22c&group_state=expanded
- push where failures appeared : https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=UQr7Ma3-SbC8kTvGkBApog.0&revision=8e57fab1c5e5e4b2658b909ada847d0bd89a9e86&searchStr=mochitest-chrome&group_state=expanded
- failure log: https://treeherder.mozilla.org/logviewer?job_id=388641264&repo=autoland&lineNumber=129500
[task 2022-08-27T00:12:57.135Z] 00:12:57 INFO - TEST-START | toolkit/components/windowwatcher/test/test_alwaysOnTop_windows.html
[task 2022-08-27T00:17:57.204Z] 00:17:57 INFO - TEST-INFO | started process screenshot
[task 2022-08-27T00:17:57.272Z] 00:17:57 INFO - TEST-INFO | screenshot: exit 0
[task 2022-08-27T00:17:57.279Z] 00:17:57 INFO - Buffered messages logged at 00:12:58
[task 2022-08-27T00:17:57.279Z] 00:17:57 INFO - add_task | Entering
[task 2022-08-27T00:17:57.280Z] 00:17:57 INFO - Buffered messages finished
[task 2022-08-27T00:17:57.280Z] 00:17:57 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/windowwatcher/test/test_alwaysOnTop_windows.html | Test timed out. -
[task 2022-08-27T00:17:58.213Z] 00:17:58 INFO - Not taking screenshot here: see the one that was previously logged
[task 2022-08-27T00:17:58.217Z] 00:17:58 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/windowwatcher/test/test_alwaysOnTop_windows.html | [SimpleTest.finish()] No checks actually run. (You need to call ok(), is(), or similar functions at least once. Make sure you use SimpleTest.waitForExplicitFinish() if you need it.)
[task 2022-08-27T00:17:58.217Z] 00:17:58 INFO - SimpleTest.ok@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:414:16
[task 2022-08-27T00:17:58.217Z] 00:17:58 INFO - afterCleanup@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1413:18
[task 2022-08-27T00:17:58.218Z] 00:17:58 INFO - executeCleanupFunction@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1489:7
[task 2022-08-27T00:17:58.218Z] 00:17:58 INFO - SimpleTest.finish@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1509:3
[task 2022-08-27T00:17:58.218Z] 00:17:58 INFO - killTest@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:195:22
[task 2022-08-27T00:17:58.233Z] 00:17:58 INFO - GECKO(864) | MEMORY STAT | vsize 2104390MB | vsizeMaxContiguous 65864728MB | residentFast 291MB | heapAllocated 156MB
[task 2022-08-27T00:17:58.239Z] 00:17:58 INFO - TEST-OK | toolkit/components/windowwatcher/test/test_alwaysOnTop_windows.html | took 301104ms
Updated•2 years ago
|
Comment 17•2 years ago
|
||
There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:nika, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 18•2 years ago
|
||
Depends on D155277
Reporter | ||
Updated•2 years ago
|
Comment 19•2 years ago
|
||
Comment 20•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/62cc9e155d56
https://hg.mozilla.org/mozilla-central/rev/221e28fc994d
https://hg.mozilla.org/mozilla-central/rev/2b181e351637
https://hg.mozilla.org/mozilla-central/rev/f2b6ca0547cb
https://hg.mozilla.org/mozilla-central/rev/adc9146d8329
https://hg.mozilla.org/mozilla-central/rev/484fd2c68e87
https://hg.mozilla.org/mozilla-central/rev/4ec21c88e8c9
Comment 21•2 years ago
|
||
Bug 1786376 was assigned to the wrong field. I shouldn't be in the Regressed By field. It's reversed. Then the regression keyword can be removed from this bug.
Updated•2 years ago
|
Description
•