1557386 - Beacon should not be restricted by CORS

Status: RESOLVED FIXED

(Core :: DOM: Security, defect, P3)

Description

[Christoph Kerschbaumer [:ckerschb]]

It seems our implementation of Beacon still applies CORS to Beacon even though the spec says otherwise [1].

We should fix that.

[0] https://searchfox.org/mozilla-central/source/dom/base/Navigator.cpp#1133
[1] https://w3c.github.io/beacon/#sec-processing-model

Comment 1

[Anne (:annevk)]

Well, we should use CORS, but it should depend on the Content-Type header. Thomas recently cleaned up that check elsewhere if I remember correctly so might be an easy fix for him.

Comment 2

[Thomas Wisniewski [:twisniewski]]

For the record, I have a work-in-progress patch that needs a web platform test, and I'll try to get that across the finish line ASAP (I'll attach the WIP patch soon at least, if I can't get to adding a WPT myself soon).

Comment 3

[Jonathan Kingston [:jkt] he/him]

I'm seeing the following CORB errors which seen related to treating these requests as CORS:

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://test.com/. (Reason: CORS request did not succeed).

Using the following code via the console on example.com

window.addEventListener("beforeunload", () => {
  navigator.sendBeacon("https://test.com", new Blob(["test"], {type: "application/x-www-form-urlencoded"}));
});

My understanding of the spec is this shouldn't be treated as CORS/CORB; this isn't the behaviour of Chrome.

Comment 4

[Anne (:annevk)]

Cross-Origin Request Blocked != CORB as proposed by Google (CORB is also not something Firefox implements). The problem is in comment 1.

Comment 5

[Jonathan Kingston [:jkt] he/him]

Haha should have checked searchfox, I've forgotten everything already :'). Thanks Anne!

:twisniewski - did you still have the WIP patch, I'm happy to review or finish it?

Comment 6

[Jonathan Kingston [:jkt] he/him]

Interestingly I am getting a real CORB warning when the request is text (I'm assuming this could be discarded as beacon is never readable):

Cross-Origin Read Blocking (CORB) blocked cross-origin response https://www.test.com/ with MIME type text/html. See https://www.chromestatus.com/feature/5629709824032768 for more details.

With:

window.addEventListener("beforeunload", () => {
  navigator.sendBeacon("https://test.com", "test");
});

Comment 7

[Thomas Wisniewski [:twisniewski]]

Attachment: beacon.patch (do not land)

Sure, though I don't know if it applies cleanly or is correct anymore, this was the patch.

Comment 8

[Jonathan Kingston [:jkt] he/him]

Attachment: Bug 1557386 - Pass correct CORS flags for Beacon requests r?ckerschb

Comment 9

[Jonathan Kingston [:jkt] he/him]

So I tested the patch which applied manually fairly cleanly (Thomas really should get the mercurial credit here imo).

There is already WPT tests under: testing/web-platform/tests/beacon/beacon-cors.sub.window.js

These tests currently pass, which means that the beacon requests have always been sent correctly just the response returns into a console error. A mochitest perhaps could be written for checking the console message but as I understand it wpt couldn't cover that. (currently looking into if this is a simple thing to test for)

Comment 10

[Jonathan Kingston [:jkt] he/him]

Attachment: Bug 1557386 - Testing CORS content and errors for beacon. r=ckerschb

Depends on D75757

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/545905e08c82
Pass correct CORS flags for Beacon requests r=ckerschb

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68aec891cd2e
Testing CORS content and errors for beacon. r=ckerschb

Comment 13

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset 68aec891cd2e (bug 1557386) for test_beaconFrame.html failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=Lu_w-jO7S1StKRZ9-uA1Ww-0&fromchange=545905e08c82a598ab07d58882f3d33c47276633&tochange=5424bea51e22f7d52e81869e6d124227b11e743d&searchStr=linux%2C18.04%2Cx64%2Cdebug%2Cmochitests%2Ctest-linux1804-64%2Fdebug-mochitest-plain-e10s-8%2Cm%288%29

Backout link: https://hg.mozilla.org/integration/autoland/rev/5424bea51e22f7d52e81869e6d124227b11e743d

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=303247600&repo=autoland&lineNumber=5396

[task 2020-05-21T17:48:58.863Z] 17:48:58     INFO - TEST-START | dom/tests/mochitest/beacon/test_beaconFrame.html
[task 2020-05-21T17:48:59.206Z] 17:48:59     INFO -  *** error running SJS at /builds/worker/workspace/build/tests/mochitest/tests/dom/tests/mochitest/beacon/beacon-handler.sjs: [Exception... "Component is not available"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"  location: "JS frame :: /builds/worker/workspace/build/tests/bin/components/httpd.js :: getHeaderValues :: line 5189"  data: no] on line 2325
[task 2020-05-21T17:48:59.223Z] 17:48:59     INFO - TEST-INFO | started process screentopng
[task 2020-05-21T17:48:59.655Z] 17:48:59     INFO - TEST-INFO | screentopng: exit 0
[task 2020-05-21T17:48:59.655Z] 17:48:59     INFO - Buffered messages logged at 17:48:59
[task 2020-05-21T17:48:59.655Z] 17:48:59     INFO - TEST-PASS | dom/tests/mochitest/beacon/test_beaconFrame.html | Beacon was not sent 
[task 2020-05-21T17:48:59.657Z] 17:48:59     INFO - Buffered messages finished
[task 2020-05-21T17:48:59.659Z] 17:48:59     INFO - TEST-UNEXPECTED-FAIL | dom/tests/mochitest/beacon/test_beaconFrame.html | uncaught exception - SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data at beaconSent/<@http://mochi.test:8888/tests/dom/tests/mochitest/beacon/test_beaconFrame.html:63:27
[task 2020-05-21T17:48:59.660Z] 17:48:59     INFO - getBeaconServerStatus/request.onload@http://mochi.test:8888/tests/dom/tests/mochitest/beacon/test_beaconFrame.html:30:21
[task 2020-05-21T17:48:59.660Z] 17:48:59     INFO - EventHandlerNonNull*getBeaconServerStatus@http://mochi.test:8888/tests/dom/tests/mochitest/beacon/test_beaconFrame.html:28:5
[task 2020-05-21T17:48:59.661Z] 17:48:59     INFO - beaconSent@http://mochi.test:8888/tests/dom/tests/mochitest/beacon/test_beaconFrame.html:61:26
[task 2020-05-21T17:48:59.661Z] 17:48:59     INFO - sendBeacon@http://mochi.test:8888/tests/dom/tests/mochitest/beacon/beacon-frame.html:16:19
[task 2020-05-21T17:48:59.662Z] 17:48:59     INFO - setTimeout handler*SimpleTest_setTimeoutShim@http://mochi.test:8888/tests/SimpleTest/SimpleTest.js:788:41
[task 2020-05-21T17:48:59.662Z] 17:48:59     INFO - @http://mochi.test:8888/tests/dom/tests/mochitest/beacon/beacon-frame.html:19:56
[task 2020-05-21T17:48:59.662Z] 17:48:59     INFO - EventListener.handleEvent*@http://mochi.test:8888/tests/dom/tests/mochitest/beacon/beacon-frame.html:19:8
[task 2020-05-21T17:48:59.662Z] 17:48:59     INFO - 
[task 2020-05-21T17:48:59.663Z] 17:48:59     INFO -     simpletestOnerror@SimpleTest/SimpleTest.js:1950:18
[task 2020-05-21T17:48:59.663Z] 17:48:59     INFO -     OnErrorEventHandlerNonNull*@SimpleTest/SimpleTest.js:1925:1
[task 2020-05-21T17:48:59.664Z] 17:48:59     INFO - GECKO(3580) | JavaScript error: http://mochi.test:8888/tests/dom/tests/mochitest/beacon/test_beaconFrame.html, line 63: SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data
[task 2020-05-21T17:48:59.664Z] 17:48:59     INFO - GECKO(3580) | MEMORY STAT | vsize 2553MB | residentFast 145MB | heapAllocated 12MB
[task 2020-05-21T17:48:59.664Z] 17:48:59     INFO - TEST-OK | dom/tests/mochitest/beacon/test_beaconFrame.html | took 631ms

Comment 14

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/545905e08c82

Comment 15

[Jonathan Kingston [:jkt] he/him]

Removing ni, flagged the changed test for checkin.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/625f46790646
Testing CORS content and errors for beacon. r=ckerschb

Comment 17

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/625f46790646