Closed Bug 1831583 Opened 1 year ago Closed 7 months ago

WPT failures for dynamic import of script from data: URL in worker modules

Categories

(Core :: DOM: Workers, defect, P2)

defect

Tracking

()

RESOLVED FIXED
120 Branch
Tracking Status
firefox120 --- fixed

People

(Reporter: jonco, Assigned: aiunusov)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

The following WPTs fail for worker modules:

/workers/modules/dedicated-worker-import-data-url-cross-origin.html
/workers/modules/shared-worker-import-data-url-cross-origin.html

The message given is:

  FAIL dynamic import script from data: URL should be blocked. - assert_array_equals: expected property 0 to be "ERROR" but got "export-block-cross-origin.js" (expected array ["ERROR"] got ["export-block-cross-origin.js"])

Chrome passes these tests and Safari has a different set of failures.

Assignee: nobody → aiunusov
Severity: -- → S3
Priority: -- → P3
Attachment #9339264 - Attachment description: WIP: Bug 1831583 - FIX WPT failures for dynamic import of script from data: URL in worker modules, r=smaug → WIP: Bug 1831583 - FIX WPT failures for dynamic import, r=smaug

These WPTs are on the interop2023 dashboard, so I'll add this as a blocker.

Blocks: interop-2023

Hi, Artur
Can you land the fix for the secFlags first without changing the principal?

secFlags = aIsTopLevel ? nsILoadInfo::SEC_REQUIRE_SAME_ORIGIN_DATA_IS_BLOCKED
	                               : nsILoadInfo::SEC_REQUIRE_CORS_INHERITS_SEC_CONTEXT;

As currently we are still discussing the correct principal for worker's static import.
https://github.com/whatwg/html/issues/9571

Please apply my patch for revising the WPT in https://phabricator.services.mozilla.com/D186470
so if one test got timeout the rest of the tests can still be run.

So the secFlags will fix the [dynamic import script from data: URL should be blocked.]
but will cause new failures in
https://searchfox.org/mozilla-central/rev/d7a8eadc28298c31381119cbf25c8ba14b8712b3/testing/web-platform/tests/workers/modules/dedicated-worker-import-data-url-cross-origin.html#27

/workers/modules/dedicated-worker-import-data-url.any.html
/workers/modules/dedicated-worker-import-data-url.any.worker.html
Static import (redirect)

The failed reason is the spec issue listed above, (the the principal used for worker's static import)

Doing so can help us better understand how many WPT tests are affected by the spec issue.

Thanks

Flags: needinfo?(aiunusov)

Thanks for the information! Doing the fix right now

Flags: needinfo?(aiunusov)

looks not so good at treeherder:
https://treeherder.mozilla.org/jobs?repo=try&revision=a842450bddfede8b0bf5e134fdb8b1606538dd33
(investigating, which tests are really broken after this change)

(In reply to Artur Iunusov from comment #7)

looks not so good at treeherder:
https://treeherder.mozilla.org/jobs?repo=try&revision=a842450bddfede8b0bf5e134fdb8b1606538dd33
(investigating, which tests are really broken after this change)

That's expected per my comment 5,
you should add ini files to mark those expected failures.

Hi, Artur
Can you land the fix for secFlags ?
This is blocking WPT interop-2023 and this is one of the goals of the Platform 2023 H2 goals.
If you don't have time then I will take this bug.

Flags: needinfo?(aiunusov)

I will do this today.

Just stumbled upon some not related crashes on try server

Flags: needinfo?(aiunusov)

https://treeherder.mozilla.org/push-health/push?repo=try&revision=80b23a141906d967211f3c7d5700f24263b3117f
so far, it's 73 errors

running failed tests locally, to double check and collect the broken tests, other than in /workers/* folder

(In reply to Artur Iunusov from comment #11)

https://treeherder.mozilla.org/push-health/push?repo=try&revision=80b23a141906d967211f3c7d5700f24263b3117f
so far, it's 73 errors

running failed tests locally, to double check and collect the broken tests, other than in /workers/* folder

Hi Artur
The mochitest on windows seems broken
https://treeherder.mozilla.org/jobs?revision=80b23a141906d967211f3c7d5700f24263b3117f&repo=try&selectedTaskRun=NALeGYvbSKepH7ak-rxauQ.0
Although on linux64 and macosx they look fine,
Can you check that as well?

looking

For the Linux, /event-timing/duration-with-target-low.html is broken as well (just double checked)

Hi Artur
Now browser vendors are discussing which WPT are affected by the spec issue about the correct principal to use when doing static import on workers (https://github.com/whatwg/html/issues/9571) in https://github.com/web-platform-tests/interop/issues/406

I'll prepare patches to update the ini files in WPT, it should be
/workers/modules/dedicated-worker-import-data-url.any.html, Static import (redirect)
/workers/modules/dedicated-worker-import-data-url.any.worker.html, Static import (redirect)

In the [static import script from data: URL should be allowed.] case,
The worker will load the data URI, which imports "/resources/static-import-data-url-block-cross-origin.js".

According to the current spec, the script "static-import-data-url-block-cross-origin.js"
is in the same origin of the document, therefore it should be loaded.

But with Gecko's implementation, the script "static-import-data-url-block-cross-origin.js"
is cross-origin with the main worker script (which is loaded by data URI),
so the script is blocked.

Mark this test case as failed according to the spec issue
https://github.com/whatwg/html/issues/9571

In this test, The worker will load a data URI, which import another test
script statically, and the imported script will import 'export-on-load-script.js',
which will be served with the CORS header 'Access-Control-Allow-Origin: *' in
/workers/modules/resources/export-on-load-script.js.headers

In the test case [Static import (redirect).], 'export-on-load-script.js' will be
redirected, and the redirected script doesn't have the CORS header.

According the current spec, the redirected 'export-on-load-script.js' is
in the same-origin with the document, therefore it should be loaded.

But with Gecko's implementation, the redirected script "export-on-load-script.js"
is cross-origin with the main worker script (which is loaded by data URI),
so the script is blocked.

(The first test case, which tests static import without redirect, the
script 'export-on-load-script.js' could be loaded by Gecko because the
script has the CORS header.)

Mark this test case as failed according to the spec issue
https://github.com/whatwg/html/issues/9571

Attachment #9339264 - Attachment description: WIP: Bug 1831583 - FIX WPT failures for dynamic import, r=smaug → WIP: Bug 1831583 - FIX WPT failures for dynamic import,
Attachment #9339264 - Attachment description: WIP: Bug 1831583 - FIX WPT failures for dynamic import, → Bug 1831583 - FIX WPT failures for dynamic import, r=smaug

I will take a look other failed tests next week. Since I am at PTO

So, we cannot land SEC Flag change it due to the broken functionality/tests:
/event-timing/duration-with-target-low.html
and other tests: https://treeherder.mozilla.org/push-health/push?repo=try&revision=80b23a141906d967211f3c7d5700f24263b3117f&testGroup=pr&selectedTest=dompushtesttesthaspermissionshtml&selectedTaskId=

I am looking at the tests

Priority: P3 → P2

Hi Artur
I suspect those failures are intermittent,
as per your comments, every time the failures from try are different.

I have also run a try run this week,
https://treeherder.mozilla.org/jobs?repo=try&revision=1560ce45ed93ed5fb07d38533ab6fc2364e43cad

In my test run it doesn't have the failures you said.

(my parent commit is from this Monday, 18.09.2023,
but your parent commit is from 05.09.2023, https://hg.mozilla.org/try/rev/f44a8ec033b8c6cc0cbef5480d7f73d85612c475)

Anyway, you still need to start requesting a review for your patch
https://phabricator.services.mozilla.com/D181065
Because that's a real bug in our implementation.

If the fix causes an actual failure, the following fix can be in a separate patch or a new bug.

Thanks

Hi Will,
Can you check this bug with worker team?
This bug has been pending for a while but it's blocking interop 2023.

Thanks

Flags: needinfo?(wmedina)
Attachment #9339264 - Attachment description: Bug 1831583 - FIX WPT failures for dynamic import, r=smaug → Bug 1831583: Fix secFlags when fetching a module script, r=smaug

I updated the patch
Before landing, I guess we still need to push it to tryserver

Flags: needinfo?(allstars.chh)

(In reply to Artur Iunusov from comment #22)

I updated the patch
Before landing, I guess we still need to push it to tryserver

Yes, please.
Please also rebase my patches on top of yours
https://phabricator.services.mozilla.com/D187899
https://phabricator.services.mozilla.com/D187900
https://phabricator.services.mozilla.com/D187901
so those tests will be excluded from your try run.

Flags: needinfo?(allstars.chh)
Attachment #9339264 - Attachment description: Bug 1831583: Fix secFlags when fetching a module script, r=smaug → Bug 1831583: Fix secFlags when fetching a module script, r=asuth

(In reply to Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi] from comment #21)

Hi Will,
Can you check this bug with worker team?
This bug has been pending for a while but it's blocking interop 2023.

Thanks

Flags: needinfo?(wmedina)

It looks like we can try to land the patch stack (tryserver looks more less fine)
https://treeherder.mozilla.org/jobs?repo=try&revision=da32e0d92e455532b9d94a5fdd820e516a2714ef

So with the security flag change, how do we compare with other browser?

Flags: needinfo?(allstars.chh)

I've updated the metadata in interop-2023-modules to exclude those failed tests related to the spec issue https://github.com/whatwg/html/issues/9571

We have achieved 100% pass, whereas the other two browsers (Chrome & Webkit) are 99% pass.
https://wpt.fyi/results/?label=master&label=experimental&product=chrome&product=firefox&product=safari&aligned&view=interop&q=label%3Ainterop-2023-modules

Thanks for your review and support on this.

(The secFlag fix will fix the failed tests for dynamic import, https://phabricator.services.mozilla.com/D187899,
meanwhile, the files also contain one test related to the spec issue, https://phabricator.services.mozilla.com/D187900, so the test files dedicated-worker-import-data-url-cross-origin.html, shared-worker-import-data-url-cross-origin.html are excluded from interop-2023-modules)

Flags: needinfo?(allstars.chh)
Pushed by allstars.chh@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/9358b0b1c3b2
Fix secFlags when fetching a module script, r=dom-worker-reviewers,smaug
https://hg.mozilla.org/integration/autoland/rev/55a4cd2facce
Part 2: Mark [dynamic import script from data: URL should be blocked.] as PASS in [shared|dedicated]-worker-import-data-url-cross-origin.html. r=dom-worker-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/6e401be8c586
Part 3: Mark [static import script from data: URL should be allowed.] failed as expected in [shared|dedicated]-worker-import-data-url-cross-origin.html.ini. r=dom-worker-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/460e36a5e5aa
Part 4: Mark [Static import (redirect).] failed as expected in dedicated-worker-import-data-url.any.js and shared-worker-import-data-url.window.js. r=dom-worker-reviewers,asuth
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: