1427978 - Update the WPT test we expected failure after rejecting the CORS synthesized response for the same-origin request

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, enhancement, P2)

Description

[Tom Tung [:tt, :ttung]]

Per bug 1419684 comment #8 and the summary, we should modify the WPT test in [1] since we decided to reject this kind of behavior. Meanwhile, we should signal spec for that as well.

For updating the test, I think the simplest way is to expect these synthesized responses being rejected in expected failure test.

Ben, please correct me if I misunderstand anything. Thanks!

[1] https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1222008&attachment=8933214

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

I think that looks right.  We should make the cases in:

  fetch-response-taint.https.html and
  worker-interception.https.html

Expect that the network request will fail if a CORS response is synthesized on a same-origin mode request.

Anne, are you ok with us upstreaming these test changes now?  This is for spec issue:

https://github.com/whatwg/fetch/issues/629

And I believe the spec change needed is simply an extra case in step 3.2.3:

https://fetch.spec.whatwg.org/#http-fetch

The way we have implemented it would correspond to a bullet like:

 * request's mode is "same-origin" and response's type is "cors".

Comment 2

[Anne (:annevk)]

I'd prefer an explicit okay from Chrome (asked for one in the issue) first.

Comment 3

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1427978 - P1: Update the wpt tests to expect to reject the cors synthesized response to a same origin request. r=bkelly

Hi Ben,

I update the WPT tests to ensure rejecting the cors synthesized response to a same-origin mode request. Would you mind reviewing it? Thanks!

BTW, after this patch r+, should I update this patch to Github WPT repo manually as well or just land it to mozilla-inbound and let it be uplifted to Github WPT repo would be fine?

Comment 4

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1427978 - P2: Remove the ini file to expect the wpt tests succeed by default. r=bkelly

Patch for removing ini file added in bug 1222008.

Comment 5

[Tom Tung [:tt, :ttung]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6cc0849e2ca892dbc87ec9831764c20b2eea3cc6

Comment 6

[Tom Tung [:tt, :ttung]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e3edde0c2242804300ebd96b0a0f648ace682688
Bug 1427978 - P1: Update the wpt tests to expect to reject the cors synthesized response to a same origin request. r=bkelly

https://hg.mozilla.org/integration/mozilla-inbound/rev/71e0525bd738a350582079354a5f7f96d0b5e555
Bug 1427978 - P2: Remove the ini file to expect the wpt tests succeed by default. r=bkelly

Comment 7

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/e3edde0c2242
https://hg.mozilla.org/mozilla-central/rev/71e0525bd738

Comment 8

[Matt Falkenhagen]

Just curious, around when would the WPT test change in comment 6 make its way upstream to WPT tests? Is there a periodic sync or would it be manually upstreamed?

Comment 9

[Tom Tung [:tt, :ttung]]

(In reply to Matt Falkenhagen from comment #8)
> Just curious, around when would the WPT test change in comment 6 make its
> way upstream to WPT tests? Is there a periodic sync or would it be manually
> upstreamed?

Our changes for WPT tests are regularly synchronized to the upstream [1] and :jgraham takes responsibility for that. But, I'm not sure about when will the change in comment 6 be merged to the upstream.

[1] https://github.com/w3c/web-platform-tests

Comment 10

[James Graham [:jgraham]]

I am going to do a sync at the end of this week. Then, depending on some ops stuff, we are going to turn on the new sync code  next week or so. After that things will usually be automatically upstreamed as soon as they reach central.

Comment 11

[Matt Falkenhagen]

Thanks James. It looks like the test change is not yet in WPT. Chromium has a patch for this behavior change and would like to reuse the tests. It feels cleaner to wait for your sync, but we might want to upstream it ourselves if it looks like it'll take longer. If we do the latter, would you have a preference about whether we directly commit a patch to WPT or just commit a patch to Chromium and let our auto-syncer upstream it?

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

See comment 11.

Comment 13

[James Graham [:jgraham]]

I'm going to begin a sync (which will upstream local patches) today. Hope that's good enough.

Comment 14

[James Graham [:jgraham]]

https://github.com/w3c/web-platform-tests/commit/b366abd65496bfa924c0dac4c5279c05a7c27d0d