search

w3c / beacon

Beacon
https://w3c.github.io/beacon/
Other
46 stars 22 forks source link

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

Closed igrigorik closed 8 years ago

igrigorik commented 8 years ago

After a thorough scrub through Fetch with @toddreifsteck, I believe this should address #32. For motivation on this change see https://github.com/w3c/beacon/issues/32#issuecomment-228825553 - 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 commented 8 years ago

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 commented 8 years ago

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.

annevk commented 8 years ago

Which locked down MIME type?

toddreifsteck commented 8 years ago

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

toddreifsteck commented 8 years ago

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 commented 8 years ago

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

annevk commented 8 years ago

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.