Bug 89185 – AssociatedURLLoader should allow trusted clients to read all headers, not just exposed ones

• Component: New Bugs
• Status: RESOLVED FIXED

[Ami Fischman]

Today AssociatedURLLoader::ClientAdapter::didReceiveResponse() copies the response for the benefit of its client, stripping out blocked headers.  This makes it impossible for any code, trusted or not, to inspect such headers.
An example problem this raises: chromium's media loading code specifies Range: in its request header, so it requests [origin, accept-encoding, referer, range] in the CORS pre-flight request (Access-Control-Request-Headers).  When it receives a 206 response, it wants to use the Content-Range, Content-Length, and Accept-Ranges headers, but it can't do so because these are not "simple headers" (in CORS-speak) and are not explicitly included in Access-Control-Expose-Headers because the server doesn't necessarily know the client requires them, so AssociatedURLLoader::ClientAdapter::didReceiveResponse() strips them out of the response copy handed to BufferedResourceLoader.

Instead, there should be an API for trusted code to side-step this header-stripping.

[Bill Budge]

Created attachment 147805 [details]
Proposed patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Bill Budge]

Created attachment 147806 [details]
Proposed patch

[Bill Budge]

Ignore the first patch - it doesn't build.

[Bill Budge]

Adam, this new WebURLLoaderOption field completely turns off response header filtering. Therefore this would expose "set-cookie" headers to the client. I'm assuming this is OK since it's trusted.

[Adam Barth]

Comment on attachment 147806 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147806&action=review

Interesting approach.  I was expecting you to add another function for grabbing the headers, but I see why you've taken this approach.

> Source/WebKit/chromium/public/WebURLLoaderOptions.h:57
> +    bool allowResponseHeaders; // If policy is to use access control, whether to allow non-simple response headers.

I wonder if we should name this something like exposeAllResponseHeaders to mimic the name of Access-Control-Expose-Headers ?

[Bill Budge]

Created attachment 147859 [details]
Proposed Patch

'exposeAllResponseHeaders' is a much better name. Thanks.

[WebKit Review Bot]

Comment on attachment 147859 [details]
Proposed Patch

Clearing flags on attachment: 147859

Committed r120490: <http://trac.webkit.org/changeset/120490>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.