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

Set request's cors mode based on payload's object type #33

Merged
merged 1 commit into from
Jun 29, 2016

Conversation

igrigorik
Copy link
Member

After a thorough scrub through Fetch with @toddreifsteck, I believe this should address #32. For motivation on this change see #32 (comment) - in particular, the comment about image beacons and chained redirects.

@toddreifsteck @annevk ptal and review. Any other edge cases we need to cover here?

@annevk
Copy link
Member

annevk commented Jun 29, 2016

This seems broken. We can't just allow ArrayBuffer go across origins (with or without MIME type). It seems fetch() might have the same issue. Sigh.

@toddreifsteck
Copy link
Member

toddreifsteck commented Jun 29, 2016

ArrayBuffer is protected when it goes across origins by its inability to alter Headers and lack of a Content-Type. Merging this PR as it represents our best understanding at this point in time. @annevk , we are wide open to understanding in more detail the best way to achieve this if fetch is updated or if there is an alternative method. Microsoft Edge reviewed XHR today to double-check and it appears to handle ArrayBuffer in a similiar way to fetch's wording.

@toddreifsteck toddreifsteck merged commit 62f4664 into gh-pages Jun 29, 2016
@annevk
Copy link
Member

annevk commented Jun 30, 2016

Which locked down MIME type?

@toddreifsteck
Copy link
Member

Sorry.. I've edited my comment above to ensure it is accurate.

@toddreifsteck
Copy link
Member

With regard to ArrayBuffer and XHR... As long as the XHR is a simple request, no browser triggers a preflight for it. sendBeacon is currently designed to only create simple requests and thus can use 'no-cors' for ArrayBuffer.

@annevk
Copy link
Member

annevk commented Jul 1, 2016

I think when we added ArrayBuffer to XHR we added a leak of sorts. Too late now I suppose.

@annevk
Copy link
Member

annevk commented Jul 1, 2016

But the setup you have here is even more dangerous. Whenever we add a new type to BodyInit it'll just go straight through. You should not use a blocklist, but a safelist, when you want to do this securely.

igrigorik added a commit that referenced this pull request Jul 15, 2016
igrigorik added a commit that referenced this pull request Jul 19, 2016
igrigorik added a commit that referenced this pull request Jul 19, 2016
igrigorik added a commit that referenced this pull request Jul 26, 2016
igrigorik added a commit that referenced this pull request Jul 26, 2016
@igrigorik igrigorik deleted the nocors branch October 25, 2016 17:38
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

Successfully merging this pull request may close these issues.

3 participants