Fix PingLoader to omit credentials for cross-origin violation reports

Fix PingLoader to omit credentials for cross-origin violation reports

My patch http://crrev.com/b3b697fc8bcc938e8b9ab32a34fc5933494faaa1 broke
the PingLoader code path for CSP violation reporting by always passing
AllowStoredCredentials by mistake.

The test report-cross-origin-no-cookies.html (now named .php) introduced
by http://crrev.com/a161a28377d8d71b63a02066574fb47f03dd4b3b included a
sync XHR to set a cookie for the remote host, but it has been not
working because:
- its withCredentials is not set to true
- testRunner is not configured to accept third party cookies
- setCookies.cgi emits wildcard Access-Control-Allow-Origin which is
  invalid for credentialled CORS requests
- it has non-CORS-safelisted header SET-COOKIE

So, the test didn't catch this breakage.

This CL fixes it by using a no-cors fetch() to
/security/resources/set-cookie.php which takes arguments via the query
part of a URL.

BUG=646780
R=mkwst@chromium.org

Committed: https://crrev.com/19cf9d97e48c9fe6974d61437549202745b20077
Cr-Commit-Position: refs/heads/master@{#422787}

[tyoshino (SeeGerritForStatus), 2016-09-14 10:42:34]

Description was changed from

==========
Fix PingLoader to omit credentials for cross-origin violation reports

My patch http://crrev.com/b3b697fc8bcc938e8b9ab32a34fc5933494faaa1 broke
the PingLoader code path for CSP violation reporting by always passing
AllowStoredCredentials by mistake.

The test report-cross-origin-no-cookies.html (now named .php) introduced
by http://crrev.com/a161a28377d8d71b63a02066574fb47f03dd4b3b included a
sync XHR to set a cookie for the remote host, but it has been not
working because:
- its withCredentials is not set to true
- testRunner is not configured to accept third party cookies
- setCookies.cgi emits wildcard Access-Control-Allow-Origin which is
  invalid for credentialled CORS requests
- it has non-CORS-safelisted header SET-COOKIE

So, the test didn't catch this breakage.

This CL fixes it by using a no-cors fetch() to
/security/resources/set-cookie.php which takes arguments via the query
part of a URL.

BUG=none
==========

to

==========
Fix PingLoader to omit credentials for cross-origin violation reports

My patch http://crrev.com/b3b697fc8bcc938e8b9ab32a34fc5933494faaa1 broke
the PingLoader code path for CSP violation reporting by always passing
AllowStoredCredentials by mistake.

The test report-cross-origin-no-cookies.html (now named .php) introduced
by http://crrev.com/a161a28377d8d71b63a02066574fb47f03dd4b3b included a
sync XHR to set a cookie for the remote host, but it has been not
working because:
- its withCredentials is not set to true
- testRunner is not configured to accept third party cookies
- setCookies.cgi emits wildcard Access-Control-Allow-Origin which is
  invalid for credentialled CORS requests
- it has non-CORS-safelisted header SET-COOKIE

So, the test didn't catch this breakage.

This CL fixes it by using a no-cors fetch() to
/security/resources/set-cookie.php which takes arguments via the query
part of a URL.

BUG=none
==========

[tyoshino (SeeGerritForStatus), 2016-09-14 10:42:34]

tyoshino@chromium.org changed reviewers:
+ mkwst@chromium.org

[tyoshino (SeeGerritForStatus), 2016-09-14 10:48:07]

Description was changed from

==========
Fix PingLoader to omit credentials for cross-origin violation reports

My patch http://crrev.com/b3b697fc8bcc938e8b9ab32a34fc5933494faaa1 broke
the PingLoader code path for CSP violation reporting by always passing
AllowStoredCredentials by mistake.

The test report-cross-origin-no-cookies.html (now named .php) introduced
by http://crrev.com/a161a28377d8d71b63a02066574fb47f03dd4b3b included a
sync XHR to set a cookie for the remote host, but it has been not
working because:
- its withCredentials is not set to true
- testRunner is not configured to accept third party cookies
- setCookies.cgi emits wildcard Access-Control-Allow-Origin which is
  invalid for credentialled CORS requests
- it has non-CORS-safelisted header SET-COOKIE

So, the test didn't catch this breakage.

This CL fixes it by using a no-cors fetch() to
/security/resources/set-cookie.php which takes arguments via the query
part of a URL.

BUG=none
==========

to

==========
Fix PingLoader to omit credentials for cross-origin violation reports

My patch http://crrev.com/b3b697fc8bcc938e8b9ab32a34fc5933494faaa1 broke
the PingLoader code path for CSP violation reporting by always passing
AllowStoredCredentials by mistake.

The test report-cross-origin-no-cookies.html (now named .php) introduced
by http://crrev.com/a161a28377d8d71b63a02066574fb47f03dd4b3b included a
sync XHR to set a cookie for the remote host, but it has been not
working because:
- its withCredentials is not set to true
- testRunner is not configured to accept third party cookies
- setCookies.cgi emits wildcard Access-Control-Allow-Origin which is
  invalid for credentialled CORS requests
- it has non-CORS-safelisted header SET-COOKIE

So, the test didn't catch this breakage.

This CL fixes it by using a no-cors fetch() to
/security/resources/set-cookie.php which takes arguments via the query
part of a URL.

BUG=646780
==========

[tyoshino (SeeGerritForStatus), 2016-09-14 10:48:15]

Sorry, Mike. I made a mistake in
http://crrev.com/b3b697fc8bcc938e8b9ab32a34fc5933494faaa1

Please take a look.

[Mike West, 2016-09-14 12:54:06]

LGTM % test nit.

https://codereview.chromium.org/2345463002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/report-cross-origin-no-cookies.php
(right):

https://codereview.chromium.org/2345463002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/report-cross-origin-no-cookies.php:22:
});
In hindsight, this is a terrible test. :)

To keep the flake down, you'll probably need to pull up the injection of
`go-to-echo-report.js` to after the image loads. Perhaps as an `onload` handler
of `img`?

[tyoshino (SeeGerritForStatus), 2016-09-14 14:41:23]

Thanks. I forgot to the upload updated expectation file. PTAL.

https://codereview.chromium.org/2345463002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/report-cross-origin-no-cookies.php
(right):

https://codereview.chromium.org/2345463002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/report-cross-origin-no-cookies.php:22:
});
On 2016/09/14 12:54:05, Mike West wrote:
> In hindsight, this is a terrible test. :)
> 
> To keep the flake down, you'll probably need to pull up the injection of
> `go-to-echo-report.js` to after the image loads. Perhaps as an `onload`
handler
> of `img`?

Ah, yeah, tried to inject it in onerror, but at that point, it looks
window.onload is already invoked? I extracted the window.location update code
from it and put it in onerror.

[tyoshino (SeeGerritForStatus), 2016-09-16 07:33:27]

Hi Mike, could you take a quick look again? Thanks!

[tyoshino (SeeGerritForStatus), 2016-09-16 07:33:45]

The CQ bit was checked by tyoshino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 07:34:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 07:37:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-16 07:37:54]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[tyoshino (SeeGerritForStatus), 2016-09-16 07:40:14]

The CQ bit was checked by tyoshino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 07:40:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 07:43:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-16 07:43:03]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[tyoshino (SeeGerritForStatus), 2016-10-04 13:21:52]

Description was changed from

==========
Fix PingLoader to omit credentials for cross-origin violation reports

My patch http://crrev.com/b3b697fc8bcc938e8b9ab32a34fc5933494faaa1 broke
the PingLoader code path for CSP violation reporting by always passing
AllowStoredCredentials by mistake.

The test report-cross-origin-no-cookies.html (now named .php) introduced
by http://crrev.com/a161a28377d8d71b63a02066574fb47f03dd4b3b included a
sync XHR to set a cookie for the remote host, but it has been not
working because:
- its withCredentials is not set to true
- testRunner is not configured to accept third party cookies
- setCookies.cgi emits wildcard Access-Control-Allow-Origin which is
  invalid for credentialled CORS requests
- it has non-CORS-safelisted header SET-COOKIE

So, the test didn't catch this breakage.

This CL fixes it by using a no-cors fetch() to
/security/resources/set-cookie.php which takes arguments via the query
part of a URL.

BUG=646780
==========

to

==========
Fix PingLoader to omit credentials for cross-origin violation reports

My patch http://crrev.com/b3b697fc8bcc938e8b9ab32a34fc5933494faaa1 broke
the PingLoader code path for CSP violation reporting by always passing
AllowStoredCredentials by mistake.

The test report-cross-origin-no-cookies.html (now named .php) introduced
by http://crrev.com/a161a28377d8d71b63a02066574fb47f03dd4b3b included a
sync XHR to set a cookie for the remote host, but it has been not
working because:
- its withCredentials is not set to true
- testRunner is not configured to accept third party cookies
- setCookies.cgi emits wildcard Access-Control-Allow-Origin which is
  invalid for credentialled CORS requests
- it has non-CORS-safelisted header SET-COOKIE

So, the test didn't catch this breakage.

This CL fixes it by using a no-cors fetch() to
/security/resources/set-cookie.php which takes arguments via the query
part of a URL.

BUG=646780
R=mkwst@chromium.org
==========

[tyoshino (SeeGerritForStatus), 2016-10-04 13:21:57]

The CQ bit was checked by tyoshino@chromium.org

[tyoshino (SeeGerritForStatus), 2016-10-04 13:21:58]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2345463002/#ps80001
(title: "Rebase")

[commit-bot: I haz the power, 2016-10-04 13:22:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-04 14:36:50]

Description was changed from

==========
Fix PingLoader to omit credentials for cross-origin violation reports

My patch http://crrev.com/b3b697fc8bcc938e8b9ab32a34fc5933494faaa1 broke
the PingLoader code path for CSP violation reporting by always passing
AllowStoredCredentials by mistake.

The test report-cross-origin-no-cookies.html (now named .php) introduced
by http://crrev.com/a161a28377d8d71b63a02066574fb47f03dd4b3b included a
sync XHR to set a cookie for the remote host, but it has been not
working because:
- its withCredentials is not set to true
- testRunner is not configured to accept third party cookies
- setCookies.cgi emits wildcard Access-Control-Allow-Origin which is
  invalid for credentialled CORS requests
- it has non-CORS-safelisted header SET-COOKIE

So, the test didn't catch this breakage.

This CL fixes it by using a no-cors fetch() to
/security/resources/set-cookie.php which takes arguments via the query
part of a URL.

BUG=646780
R=mkwst@chromium.org
==========

to

==========
Fix PingLoader to omit credentials for cross-origin violation reports

My patch http://crrev.com/b3b697fc8bcc938e8b9ab32a34fc5933494faaa1 broke
the PingLoader code path for CSP violation reporting by always passing
AllowStoredCredentials by mistake.

The test report-cross-origin-no-cookies.html (now named .php) introduced
by http://crrev.com/a161a28377d8d71b63a02066574fb47f03dd4b3b included a
sync XHR to set a cookie for the remote host, but it has been not
working because:
- its withCredentials is not set to true
- testRunner is not configured to accept third party cookies
- setCookies.cgi emits wildcard Access-Control-Allow-Origin which is
  invalid for credentialled CORS requests
- it has non-CORS-safelisted header SET-COOKIE

So, the test didn't catch this breakage.

This CL fixes it by using a no-cors fetch() to
/security/resources/set-cookie.php which takes arguments via the query
part of a URL.

BUG=646780
R=mkwst@chromium.org
==========

[commit-bot: I haz the power, 2016-10-04 14:36:52]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-10-04 14:39:59]

Description was changed from

==========
Fix PingLoader to omit credentials for cross-origin violation reports

My patch http://crrev.com/b3b697fc8bcc938e8b9ab32a34fc5933494faaa1 broke
the PingLoader code path for CSP violation reporting by always passing
AllowStoredCredentials by mistake.

The test report-cross-origin-no-cookies.html (now named .php) introduced
by http://crrev.com/a161a28377d8d71b63a02066574fb47f03dd4b3b included a
sync XHR to set a cookie for the remote host, but it has been not
working because:
- its withCredentials is not set to true
- testRunner is not configured to accept third party cookies
- setCookies.cgi emits wildcard Access-Control-Allow-Origin which is
  invalid for credentialled CORS requests
- it has non-CORS-safelisted header SET-COOKIE

So, the test didn't catch this breakage.

This CL fixes it by using a no-cors fetch() to
/security/resources/set-cookie.php which takes arguments via the query
part of a URL.

BUG=646780
R=mkwst@chromium.org
==========

to

==========
Fix PingLoader to omit credentials for cross-origin violation reports

My patch http://crrev.com/b3b697fc8bcc938e8b9ab32a34fc5933494faaa1 broke
the PingLoader code path for CSP violation reporting by always passing
AllowStoredCredentials by mistake.

The test report-cross-origin-no-cookies.html (now named .php) introduced
by http://crrev.com/a161a28377d8d71b63a02066574fb47f03dd4b3b included a
sync XHR to set a cookie for the remote host, but it has been not
working because:
- its withCredentials is not set to true
- testRunner is not configured to accept third party cookies
- setCookies.cgi emits wildcard Access-Control-Allow-Origin which is
  invalid for credentialled CORS requests
- it has non-CORS-safelisted header SET-COOKIE

So, the test didn't catch this breakage.

This CL fixes it by using a no-cors fetch() to
/security/resources/set-cookie.php which takes arguments via the query
part of a URL.

BUG=646780
R=mkwst@chromium.org

Committed: https://crrev.com/19cf9d97e48c9fe6974d61437549202745b20077
Cr-Commit-Position: refs/heads/master@{#422787}
==========

[commit-bot: I haz the power, 2016-10-04 14:40:00]

Patchset 5 (id:??) landed as
https://crrev.com/19cf9d97e48c9fe6974d61437549202745b20077
Cr-Commit-Position: refs/heads/master@{#422787}