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

RESOLVED FIXED in Firefox 59

Status

()

enhancement
P2
normal
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: tt, Assigned: tt)

Tracking

unspecified
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(2 attachments)

Assignee

Description

2 years ago
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
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".
Flags: needinfo?(annevk)

Comment 2

2 years ago
I'd prefer an explicit okay from Chrome (asked for one in the issue) first.
Flags: needinfo?(annevk)
Assignee

Comment 3

2 years ago
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?
Attachment #8940573 - Flags: review?(bkelly)
Attachment #8940573 - Flags: review?(bkelly) → review+
Attachment #8940574 - Flags: review?(bkelly) → review+
Assignee

Comment 6

2 years ago
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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e3edde0c2242
https://hg.mozilla.org/mozilla-central/rev/71e0525bd738
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
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?
Assignee

Comment 9

Last year
(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
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.
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?
See comment 11.
Flags: needinfo?(james)
I'm going to begin a sync (which will upstream local patches) today. Hope that's good enough.
Flags: needinfo?(james)
You need to log in before you can comment on or make changes to this bug.