[Webkit-unassigned] [Bug 63460] CORS should only deal with request headers set by script authors

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=63460





--- Comment #5 from Per-Erik Brodin <per-erik.brodin at ericsson.com>  2011-06-28 14:17:03 PST ---
(In reply to comment #3)

I appreciate the quick review.

> The implementation cannot work on Mac, because header status will be lost after a round-trip through NSURLRequest (or any other HTTP library request object). WebKit needs this round-trip because it has delegate methods where it informs client applications about requests that are about to be made.
>

I realize that the header status might get lost when platforms deal with the request. However, the header status is only meant to be used by the CORS implementation and that happens before the status is lost. I was not able to build on Mac with the patch applied because of the optional arguments but once that was fixed I achieved the desired behavior on Mac as well.

> ResourceRequest is ah HTTP level notion, and it shouldn't have any knowledge of Web platform concepts.
> 

Could you recommend a different approach to fixing this bug? The included test demonstrates a deviation in the implementation from the current CORS spec (Origin being sent in Access-Control-Request-Headers header). To enable CORS in EventSource this patch was going to prevent a preflight from the two headers added by the implementation (thus not in the list of simple headers) but I guess I could solve that with a flag in ThreadableLoaderOptions instead.

> Secondly, when we talk about author vs. implementation, where do headers added by client applications fit? It kind of sounds like it's "implementation", but we can't know if those applications use untrusted content to decide which headers to add.
> 

Can clients invoke CORS? I would say that client added headers are implementation headers and that it is up to clients to safely add headers.

> > LayoutTests/ChangeLog:6
> > +        CORS should only deal with request headers set by script authors.
> > +        https://bugs.webkit.org/show_bug.cgi?id=63460
> 
> That's an interesting idea, although it seems somewhat questionable to me. The design principle use to be to avoid hitting servers with anything they couldn't be hit with through form submission, and the new rule deviates from that pretty far.
> 

I think the reasoning is that headers added by the implementation are safe even for cross origin requests. With this clarification the CORS specification does not have to be updated for every new specification that wants to add a header without causing a preflight request to be made. Also, there is only one list of simple headers so if Last-Event-ID was to be added back then you could use this header in XHR without preflight as well (not sure if this is a problem though, just an interesting consequence).

> > Source/WebCore/platform/network/ResourceRequestBase.h:47
> > +    enum ResourceRequestHeaderStatus {
> 
> Naming nit: I would have liked something like "origination" or "source" more than "status".

Yes, you are right, "source" is much better than "status". Anything related to "origin" should probably be avoided in this context though.

(In reply to comment #4)
> One more question: what about Content-Type headers that are not set by authors? Currently XMLHttpRequest defaults to application/xml, which triggers preflight. Should the same logic be applied there?

The Content-Type headers set by the XHR implementation would fall under the category "implicitly set by authors", see the second paragraph in http://lists.w3.org/Archives/Public/public-webapps/2011AprJun/1222.html

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.