Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add range to CORS-safelisted request-headers #1310

Closed
jakearchibald opened this issue Sep 24, 2021 · 23 comments · Fixed by #1312
Closed

Add range to CORS-safelisted request-headers #1310

jakearchibald opened this issue Sep 24, 2021 · 23 comments · Fixed by #1312

Comments

@jakearchibald
Copy link
Collaborator

We allow no-cors requests to have Range headers as long as it's set by an internal API, and the request is unmodified.

It feels like CORS requests should be able to use Range in particular ways without triggering a preflight. If the format matches what https://fetch.spec.whatwg.org/#concept-request-add-range-header can create, that's ok right?

@jakearchibald
Copy link
Collaborator Author

jakearchibald commented Sep 24, 2021

I guess we should also check that the start position is less or equal to the end position

@joeyparrish
Copy link

I strongly support this. This would immediately enable background fetch of streaming media (which frequently uses range headers) without preflight.

@annevk
Copy link
Member

annevk commented Sep 27, 2021

The problem with allowing more headers is that we get closer to server limits which in turn might result in certain security issues. See the recent discussion in w3c/webappsec-cspee#22.

We still have room in that the total header value cap for CORS is a 1024, but in practice only 128 is allowed for each of the four headers, but it does make me nervous. The other question of course is whether 128 is sufficient for Range, but from what I've seen I think it is. (Fast forward two decades and folks will make fun of this comment, I'm sure.)

@jakearchibald
Copy link
Collaborator Author

Assuming you use both start and end values, that leaves 57 bytes for each length (assuming the worst case where they're the same size). That's up to 999,999,999,999,999,999,999,999,999,999,999 yottabytes. Surely Enough For Anyone™!

@joeyparrish
Copy link

That's up to 999,999,999,999,999,999,999,999,999,999,999 yottabytes. Surely Enough For Anyone™!

Challenge accepted! 😁

@bakkot
Copy link
Contributor

bakkot commented Sep 27, 2021

I tripped over this just recently in attempting to use https://github.com/phiresky/sql.js-httpvfs, which makes use of range requests - I checked that the place I was fetching (github pages) supported cross-origin GETs (it returns access-control-allow-origin: * for a non-preflight request), but it turned out not to support cross-origin GETs with range (because it doesn't respond to preflight requests at all, afaict).

It seems silly that I could request the entire resource but not a portion of it.

@mnot
Copy link
Member

mnot commented Sep 28, 2021

@jakearchibald that's allowing for only one range-spec.

@jakearchibald
Copy link
Collaborator Author

jakearchibald commented Sep 28, 2021

@mnot yep, which is all browsers ever do as far as I can tell. My intent here is to allow range headers that match ^bytes=\d+-\d*$ (where the first number is less than or equal to the second). Multiple range-specs will still require a preflight, as will values like bytes my bum.

annevk pushed a commit that referenced this issue Oct 5, 2021
The allowed format aligns with the values the browser uses when requesting media and resuming downloads.

Tests: web-platform-tests/wpt#31058.

Fixes #1310.
@anson0370
Copy link

anson0370 commented Aug 14, 2024

I tried this code in both Chrome and Safari, but it doesn't seem to work:

fetch('https://pscrb.fm/rss/p/media.transistor.fm/db73bbc2/d569819b.mp3', {
  mode: 'no-cors',
  headers: {
    Range: 'bytes=0-10240'
  }
})

When I check the Network tab in DevTools, the request headers don't show the Range header. I'm not sure if I'm misunderstanding something.

I tried removing mode: 'no-cors', and the Range header correctly appeared in the request headers. Of course, I also got a CORS error. As I understand it, this shouldn't be the correct behavior, right?

@joeyparrish
Copy link

Updates to the spec are not immediately reflected in browser implementations... but nearly 2 years seems like a reasonable amount of time for adoption. Does anyone know if this was ever implemented? Was this added to Web Platform Tests?

@anson0370
Copy link

I guess this is the test? web-platform-tests/wpt#31058

@joeyparrish
Copy link

That should be enough to at least raise a flag with UA implementors, who should be running these tests regularly.

@joeyparrish
Copy link

I'm pinging people I know in Chrome to see if I can track down the implementation status.

@joeyparrish
Copy link

Appears to be implemented in Chrome and passing WPT.

@anson0370
Copy link

Can anyone else reproduce this issue, or is it just me? Or am I using it wrong? 😂

@joeyparrish
Copy link

joeyparrish commented Aug 15, 2024

I just tried your snippet, and I think there is indeed something wrong. The Range header is not sent at all with mode: 'no-cors', but is if I omit that.

However, you should also know that your chosen URL does an HTTP 302 redirect, but browsers don't transfer Range headers when following redirects.

That said, I can reproduce your issue with other URLs that don't redirect. For example:

// Works
fetch('https://ajax.googleapis.com/ajax/libs/shaka-player/4.3.8/shaka-player.compiled.js', {
  headers: {
    Range: 'bytes=0-10240'
  }
})
// Ignores Range header
fetch('https://ajax.googleapis.com/ajax/libs/shaka-player/4.3.8/shaka-player.compiled.js', {
  mode: 'no-cors',
  headers: {
    Range: 'bytes=0-10240'
  }
})

I'm no expert, but this certainly breaks my expectations.

@joeyparrish
Copy link

I recommend you file a Chrome bug at crbug.com, and link to it here so we can follow along. I'll help you route it to the right people within Chrome.

@anson0370
Copy link

Thanks @joeyparrish !
I created an issue. https://issues.chromium.org/issues/360044701

@annevk
Copy link
Member

annevk commented Aug 16, 2024

No, you cannot use Range for no-cors. There's nothing in the specification that allows that.

@anson0370
Copy link

anson0370 commented Aug 16, 2024

Thanks @annevk ! So, CORS-safelisted request headers just mean that these headers don't need a preflight request?

In the MDN Web Docs, there's this description:

mode: no-cors

The request must be a simple request, which restricts the headers that may be set to CORS-safelisted request headers, and restricts methods to GET, HEAD, and POST.

It seems like this description misled me.

Even though there's no specification allowing this, it's still a bit strange.

If a request that doesn't require preflight is a simple request, and mode: no-cors requires the request to be a simple request, then by that logic, Range should be usable in no-cors requests, right?

Even though a no-cors request must be a simple request, it doesn't mean that a simple request can definitely be a no-cors request. But this can indeed be misleading, especially when it's not mentioned in the documentation.

@annevk
Copy link
Member

annevk commented Aug 16, 2024

Thank you and agreed. I filed mdn/content#35488.

@anson0370
Copy link

Thank you so much for your explanation!

@joeyparrish
Copy link

Filed #1767, because although this currently working is as spec'd, I believe Range should be allowed in no-cors mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

6 participants