Summary: | REGRESSION (r89086): All worker xhr requests trigger preflight requests. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Levin <levin> | ||||||||
Component: | XML | Assignee: | David Levin <levin> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ap, dslomov, japhet, per-erik.brodin | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
David Levin
2011-08-16 15:30:25 PDT
I guess we can add all headers in XMLHttpRequest::m_forbiddenRequestHeaders to CORS simple request whitelist now, since they can only be set by the engine, not by JS code. Created attachment 104136 [details]
Patch
Comment on attachment 104136 [details]
Patch
Yeah, I agree with Alexey.
I'll add another patch with tests and add the other headers after I land this one. Comment on attachment 104136 [details] Patch Committed as http://trac.webkit.org/changeset/93188 Keeping bug open to work on the bigger fix. (In reply to comment #1) > I guess we can add all headers in XMLHttpRequest::m_forbiddenRequestHeaders to CORS simple request whitelist now, since they can only be set by the engine, not by JS code. Another option would be to fix https://bugs.webkit.org/show_bug.cgi?id=63460 Now Referer and Origin will be incorrectly included in Access-Control-Request-Headers whenever there is a preflight. Created attachment 104552 [details]
Patch
Comment on attachment 104552 [details] Patch > Despite what I said in comment 1, I'm not sure if it's the best approach to move > headers that are special cased in XMLHttpRequest to some shared location. It's > ThreadableLoader client (in this case, XHR) who really knows if JavaScript code > called one of those methods that added non-engine headers. Many other clients > simply don't provide any way to specify custom headers, so doing checks on these > is pointless. > What do you think? It seemed reasonable to move these checks over here as you suggested previously. I suspect the added cost is negligible and this doesn't seem like a performance sensitive path. However, I don't really care to have all of this moved over and mostly did it due to the suggestion :). So I'll withdraw this patch and do one with only the test code which should help us catch any future regressions. Created attachment 104709 [details]
Test for preflight for simple request from web workers.
Comment on attachment 104709 [details] Test for preflight for simple request from web workers. View in context: https://bugs.webkit.org/attachment.cgi?id=104709&action=review > LayoutTests/http/tests/xmlhttprequest/workers/resources/access-control-basic-get-fail-non-simple-test.js:19 > +worker.onmessage = function(evt) Why eve instead of event? Comment on attachment 104709 [details]
Test for preflight for simple request from web workers.
I'll change the test to event and submit it.
Committed (tests) as http://trac.webkit.org/changeset/93536 |