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

Does any of this require CORS-with-forced-preflight? #10

Closed
annevk opened this issue Dec 10, 2014 · 11 comments
Closed

Does any of this require CORS-with-forced-preflight? #10

annevk opened this issue Dec 10, 2014 · 11 comments

Comments

@annevk
Copy link

annevk commented Dec 10, 2014

Since this allows inspection of how fast a body is transmitted to the server, should this require a preflight just like we do in XMLHttpRequest for progress events?

@yutakahirano
Copy link
Owner

I overlooked this ticked, sorry.
You're right. I'll fix that.

@yutakahirano
Copy link
Owner

  • A BodyStreamInit is given and mode is same_origin => Set mode to same_origin
  • A BodyStreamInit is given and mode is CORS => Set mode to CORS-with-forced-preflight
  • A BodyStreamInit is given and mode is CORS-with-forced-preflight => Set mode to CORS-with-forced-preflight

What should we do when mode is no CORS? Failing the operation seems reasonable to me.

@annevk
Copy link
Author

annevk commented Jan 7, 2015

Failing sounds fine, yes. There's no need to set mode btw if we don't want to change it, I would only create branches for the cases where we anticipate a change.

yutakahirano added a commit that referenced this issue Jan 7, 2015
yutakahirano added a commit that referenced this issue Jan 7, 2015
yutakahirano added a commit that referenced this issue Jan 7, 2015
@yutakahirano
Copy link
Owner

@yutakahirano
Copy link
Owner

@annevk ping

@annevk
Copy link
Author

annevk commented Jan 15, 2015

Sorry. Request class modifications look okay. Independent security review is also required but I guess that'll happen as part of landing the code in browsers.

@yutakahirano
Copy link
Owner

Thanks, merged.

@yutakahirano
Copy link
Owner

Reopen this issue because WritableStream is dropped from the draft on b32685d. I will fix it again in the future.

@domenic
Copy link
Contributor

domenic commented Aug 2, 2016

It sounds like this means uploading a request body would require a forced preflight?

@annevk
Copy link
Author

annevk commented Aug 4, 2016

That is the question. There's two parts to this. 1) Due to us using streams, you can discover much more about the server than if you transmit a single ArrayBuffer object. 2) Thus far browsers have never used chunked encoding for uploads. Using that could be a vulnerability of sorts by itself.

Curious to see what @dveditz, @mikewest et al think. Obviously not doing a preflight would be much more desirable. (On the other hand, if we get origin-wide preflights it might not be that big of a deal.)

@mikewest
Copy link

mikewest commented Aug 4, 2016

Erring on the side of caution seems reasonable; I haven't looked at the spec, but based on the comment above (#10 (comment)), it seems like the right balance.

(I agree, by the way, that giving servers the ability to opt-into certain kinds of preflightless requests on an origin-wide basis makes it less painful to require them for new features. That tips the balance further towards conservationism...)

annevk pushed a commit to whatwg/fetch that referenced this issue Jan 17, 2017
Basic tests: web-platform-tests/wpt#4362. More tests are expected to be written as part of the implementation effort.

Fixes #88, fixes yutakahirano/fetch-with-streams#10, fixes yutakahirano/fetch-with-streams#57, and fixes yutakahirano/fetch-with-streams#66.
@annevk annevk closed this as completed Jan 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants