1952605 - CORS: Safelist Last-Event-ID

Status: ASSIGNED

(Core :: DOM: Core & HTML, defect)

Description

[Espen Hovlandsdal]

Steps to reproduce:

Run a fetch request against a different origin, with a (safe) Last-Event-ID header, eg:

fetch('https://www.mozilla.org/', {
  headers: {'Last-Event-ID': 'abc123'}
})

Actual results:

Fetch request triggered a preflight request with access-control-request-headers: last-event-id.

This differs from the EventSource API, which does not trigger a preflight request for the same header.

Expected results:

The Last-Event-ID header should be safe, as long as it does not include any CORS-unsafe request header bytes, or is more than 128 characters long.

See https://github.com/whatwg/fetch/pull/1788 for the proposal and discussion around changing the specification to allow it.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Core & HTML' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Espen Hovlandsdal]

Attachment: Bug 1952605 - Add last-event-id to CORS safelisted headers. r=annevk,ckerschb

Background: https://github.com/whatwg/fetch/issues/568
WPT changes: https://github.com/web-platform-tests/wpt/pull/49257

Comment 3

[Espen Hovlandsdal]

Patch available at: https://phabricator.services.mozilla.com/D240787

Comment 4

[Espen Hovlandsdal]

Attachment: Bug 1952605 - Ensure EventSource performs preflight on unsafe Last-Event-ID . r=valentin,freddyb

Comment 5

[Frederik Braun [:freddy]]

Did you not have to modify test or test annotation files for wpt that previously did not pass?
It's unclear to me if you have privileges to run this patch set through our test suite online, so I've uploaded to go through our try server. https://treeherder.mozilla.org/#/jobs?repo=try&revision=39c194bc60dbb971bfbbf2af3c3f97e9aa8000b9

Of course, you can run ./mach test ... locally too. Tests that fail/pass on your local OS will most likely do the same on all other systems.

Comment 6

[Frederik Braun [:freddy]]

Looks like this passed fine (except for some intermittent test failures that are unrelated). Are you still pursuing this, Espen? Please see my questions above.

Comment 7

[Espen Hovlandsdal]

Thanks for following up, Frederik.
I ran the tests locally against the modified WPT tests.

This has stalled a bit because I wanted to make sure that I could land the necessary changes in all browsers, but I discovered a complicating factor when attempting to get this implemented in Chrome and WebKit: the spec says to set cache-control, but that is not a safelisted CORS header. This isn't a problem when implementing this in Gecko, since we are explicitly checking and telling it which headers are unsafe - but for Chrome/WebKit these checks are done at a lower level. By changing the CORS policy on EventSource from "never" to "consider", it now starts sending a preflight request for any EventSource connection due to the cache-control header being set.

Hoping that I'll get some guidance over on the fetch spec change - if you have any thoughts I'd love to hear them.