Bug 201127 – Tainted origin flag logic should affect no-cors redirects

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Page Loading

[Rob Buis]

See https://fetch.spec.whatwg.org/#concept-http-redirect-fetch.

[Rob Buis]

Created attachment 377224 [details]
Patch

[Rob Buis]

Created attachment 377225 [details]
Patch

[Rob Buis]

Created attachment 377230 [details]
Patch

[Rob Buis]

Created attachment 377321 [details]
Patch

[Frédéric Wang (:fredw)]

Is it ready for review? I'm a bit confused where the tainting flag comes in play in this patch.

[Rob Buis]

Created attachment 377743 [details]
Patch

[Rob Buis]

(In reply to Frédéric Wang (:fredw) from comment #5)
> Is it ready for review? I'm a bit confused where the tainting flag comes in
> play in this patch.

It was not, but is now :)

[youenn fablet]

Comment on attachment 377743 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=377743&action=review

> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:328
> +            request.setHTTPOrigin(SecurityOrigin::createUnique()->toString());

We should probably also set m_origin here.
This will ensure that the CORP check is fine.

It might be interesting to do some more refactoring.
In particular, add a routine that implements https://fetch.spec.whatwg.org/#append-a-request-origin-header.
This will make sure we add the Origin header for POST but not for GET.
We should probably add a test for GET to check that no Origin header in no-cors is set after a cross origin redirection.

doesNotNeedCORSCheck includes navigate mode for which we do not want to add a null origin.
We might want to remove doesNotNeedCORSCheck and add some early checks for navigate mode and same origin mode.
Then we could have some common code for no-cors and cors cases.
This should get us closer to the fetch spec.

> LayoutTests/imported/w3c/web-platform-tests/fetch/security/embedded-credentials.tentative.sub-expected.txt:3
>  Blocked access to external URL http://user:[e-mail]:8800/images/red.png

This test should be updated to use hosts[alt].

[Rob Buis]

Created attachment 377877 [details]
Patch

[Rob Buis]

Created attachment 385755 [details]
Patch

[Rob Buis]

Created attachment 385768 [details]
Patch

[Rob Buis]

It is possible the code in these patches is still valuable, but in the end 186030 is similar/same so let's try to fix that one instead.

*** This bug has been marked as a duplicate of bug 186030 ***