New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 757725 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

sendBeacon with text/plain body triggers CORS

Reported by fastest...@gmail.com, Aug 22 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3188.4 Safari/537.36

Steps to reproduce the problem:
1. Goto http://output.jsbin.com/hacisa/latest/quiet
2. Click sendBeacon 'simple payload'
3. Observe console error

What is the expected behavior?
I expected the CORS requirement to be lifted since the payload is simple, as specified by the spec: https://w3c.github.io/beacon/#sec-processing-model

What went wrong?
The console says: "Access to Resource at 'https://beacon-cors.herokuapp.com/' from origin 'http://output.jsbin.com' has been blocked by CORS policy: The value of the 'Access-Control-Allow-Origin' header in the response must not be the wildcard '*' when the request's credentials mode is 'include'. Origin 'http://output.jsbin.com' is therefore not allowed access." and the request is marked "(cancelled)" in the network tab.

Did this work before? Yes 60.0.3112.78

Does this work in other browsers? Yes

Chrome version: 62.0.3188.4  Channel: dev
OS Version: 10.0
Flash Version: 

I know there's been work in  bug 490015  and more generally just with sendBeacon, but I didn't see anything relating to this.
 
I just confirmed that it doesn't work in 61.0.3163.49 (beta) as well.
Cc: tyoshino@chromium.org pbomm...@chromium.org
Labels: M-61 M-62
tyoshino@ for more insights on the bug.
Cc: yhirano@chromium.org igrigo...@chromium.org
Labels: -Pri-2 ReleaseBlock-Stable Pri-1
Owner: yhirano@chromium.org
Status: Assigned (was: Unconfirmed)
+igrigorik@

Sorry for my bad. I really should identified this mismatch between ResourceFetcher and PingLoader. I was too much biased to make everything align with the standard fetch behavior, facing a lot of bugs in sendBeacon.

sendBeacon was originally specified to always use CORS. But in June 2016, it has been changed to suppress CORS when the payload type is not a Blob.
https://github.com/w3c/beacon/pull/33
and then again changed to use CORS only when the type of Blob is none of the CORS-safelisted ones.
https://github.com/w3c/beacon/pull/34

As noted in Ilya's comment at https://github.com/w3c/beacon/issues/32#issuecomment-228825553, this was motivated by providing functionality equivalent to image pings.

Given that sendBeacon doesn't give the script back the result of fetching (so, this doesn't apply to fetch() keepalive unless we disallow reading the results of keepalive specified fetch()), it's safe to suppress CORS for these cases.

As Ilya noted in the same issue, it's very common that sendBeacon is used with redirects. This regression should definitely be fixed before it goes stable. Tentatively assigning ReleaseBlock-Stable.

Comment 4 by gov...@chromium.org, Aug 23 2017


URGENT - PTAL.
M61 Stable promotion is coming soon and your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP. 

Know that this issue shouldn't block the release?  Remove the ReleaseBlock-Stable label or move to M62. Thank you!

Note: We will only have 2 beta releases before Stable promotion. Plan is to cut M61 Stable RC on 08/31/17.

Further investigation revealed that:
- the bug caused the console error message saying that the beacon is blocked
- however, due to another long-lived issue, even with that console message emitted, Chrome proceeds with following the cross-origin redirect

So, the regression doesn't really have any effect except for the confusing console message.

Considering this, I think we can remove ReleaseBlock-Stable.
Labels: -ReleaseBlock-Stable
fastest963@:

Could you please help us double check that even with the console message the server is receiving the beacon?

We checked that locally.
yhirano@ is reverting the patch that enabled CORS at:
https://chromium-review.googlesource.com/c/chromium/src/+/627968
Cc: -yhirano@chromium.org
Status: Started (was: Assigned)
Filed https://github.com/w3c/beacon/issues/53 for clarifying the spec.
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dacc670f59624580b516fb299150c4ea8a7b7f72

commit dacc670f59624580b516fb299150c4ea8a7b7f72
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Wed Aug 23 11:47:23 2017

Revert "Specify correct request mode for SendBeacon"

This reverts commit 0fbc79aab000df51cb43513c03d9c34700edffc8.

Reason for revert: The SendBeacon spec requires us to use the
"no-cors" mode if the request headers are CORS-safelisted. As we
currently block requests with non CORS-safelisted headers, all beacon
requests we make should use the "no-cors" mode.

Original change's description:
> Specify correct request mode for SendBeacon
> 
> Bug:  695939 
> Change-Id: I88421009947673d4508a088f23f2b4e5a0926d67
> Reviewed-on: https://chromium-review.googlesource.com/554423
> Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
> Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#483384}

TBR=tyoshino@chromium.org,yhirano@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  695939 ,  757725 
Change-Id: Icd6f11209bf20eae650ecd7a2dd74b853f87b1a7
Reviewed-on: https://chromium-review.googlesource.com/627968
Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496659}
[modify] https://crrev.com/dacc670f59624580b516fb299150c4ea8a7b7f72/third_party/WebKit/LayoutTests/http/tests/sendbeacon/beacon-cross-origin-redirect-expected.txt
[modify] https://crrev.com/dacc670f59624580b516fb299150c4ea8a7b7f72/third_party/WebKit/LayoutTests/http/tests/sendbeacon/resources/save-beacon.php
[modify] https://crrev.com/dacc670f59624580b516fb299150c4ea8a7b7f72/third_party/WebKit/Source/core/loader/PingLoader.cpp

yhirano@:

Correct, it appears to still work on my local machine despite the console error. Apologies for not verifying that earlier.
fastest963@: Thank you for checking! It's totally our bad that we made it emit the confusing console. Your report was really helpful.
Status: Fixed (was: Started)
Project Member

Comment 15 by bugdroid1@chromium.org, Sep 1 2017

Labels: merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b60787e90053820ff2caf85c8d1d0389a10a0df9

commit b60787e90053820ff2caf85c8d1d0389a10a0df9
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Fri Sep 01 18:04:16 2017

Revert "Specify correct request mode for SendBeacon"

This reverts commit 0fbc79aab000df51cb43513c03d9c34700edffc8.

Reason for revert: The SendBeacon spec requires us to use the
"no-cors" mode if the request headers are CORS-safelisted. As we
currently block requests with non CORS-safelisted headers, all beacon
requests we make should use the "no-cors" mode.

Original change's description:
> Specify correct request mode for SendBeacon
>
> Bug:  695939 
> Change-Id: I88421009947673d4508a088f23f2b4e5a0926d67
> Reviewed-on: https://chromium-review.googlesource.com/554423
> Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
> Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#483384}

TBR=tyoshino@chromium.org, yhirano@chromium.org


(cherry picked from commit dacc670f59624580b516fb299150c4ea8a7b7f72)

Bug:  695939 ,  757725 
Change-Id: Icd6f11209bf20eae650ecd7a2dd74b853f87b1a7
Reviewed-on: https://chromium-review.googlesource.com/627968
Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#496659}
Reviewed-on: https://chromium-review.googlesource.com/647728
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#1069}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/b60787e90053820ff2caf85c8d1d0389a10a0df9/third_party/WebKit/LayoutTests/http/tests/sendbeacon/beacon-cross-origin-redirect-expected.txt
[modify] https://crrev.com/b60787e90053820ff2caf85c8d1d0389a10a0df9/third_party/WebKit/LayoutTests/http/tests/sendbeacon/resources/save-beacon.php
[modify] https://crrev.com/b60787e90053820ff2caf85c8d1d0389a10a0df9/third_party/WebKit/Source/core/loader/PingLoader.cpp

I see similar behavior when navigating through theverge.com, Scrolling was janky as well.

Please find the screenshot for reference.
theverge.png
1011 KB View Download
this was on latest Chrome beta i.e., 61.0.3163.71.
It will be fixed on the next beta.
Project Member

Comment 20 by bugdroid1@chromium.org, Sep 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1beaf02c35103ed00799e0b649150d3d23d1cba4

commit 1beaf02c35103ed00799e0b649150d3d23d1cba4
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Thu Sep 07 11:36:47 2017

Add comments for CORS mode used in SendBeacon

Bug:  757725 
Change-Id: Iecdb1cca44695aa6a4141874a76251d4d2fe7d2f
Reviewed-on: https://chromium-review.googlesource.com/654503
Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500271}
[modify] https://crrev.com/1beaf02c35103ed00799e0b649150d3d23d1cba4/third_party/WebKit/Source/core/loader/PingLoader.cpp

Sign in to add a comment