[webkit-reviews] review granted: [Bug 61007] Allow custom schemes to use XMLHttpRequest : [Attachment 93852] Patch to DocumentThreadableLoader to allow custom schemes to use XMLHttpRequest

[bugzilla-daemon at webkit.org]

Joe Andrieu <joe at andrieu.net> has granted  review:
Bug 61007: Allow custom schemes to use XMLHttpRequest
https://bugs.webkit.org/show_bug.cgi?id=61007

Attachment 93852: Patch to DocumentThreadableLoader to allow custom schemes to
use XMLHttpRequest
https://bugs.webkit.org/attachment.cgi?id=93852&action=review

------- Additional Comments from Joe Andrieu <joe at andrieu.net>
FWIW, I'm using Chromium Embedded Framework, with WebKit v534.28.

Currently, URLs with custom schemes don't work with XMLHttpRequest, even with
CORS supported by the scheme handler.

DocumentThreadableLoader::makeSimpleCrossOriginAccessRequest in
documentthreadableloader.cpp fails because "Cross origin requests are only
defined for http" (comment in the code) This is enforced by testing
protocolInHTTPFamily().

makeSimpleCrossOriginAccessRequest is called because in the
DocumentThreadableLoader constructor, cross origin requests are handled as
simple, unless
1. forcePreFlight is set as an option
2. isSimpleCrossOrigianAccessRequest fails

Unfortunately, because isSimpleCrossOriginAccessRequest only uses the method
and the header fields as input, it can't tell if the URL has a custom scheme. 
The result is that custom schemes get treated along with
"SimpleCrossOriginAccessRequests", and subsequently fail the
protocolInHTTPFamily() test in makeSimpleCrossOriginAccessRequest.

I'd like to suggest that custom schemes are /not/ "simple". In fact, if CORS is
to be used with custom schemes, custom schemes must not be treated as simple in
order for the preflight be triggered (although I guess I'm not sure if
preflight is actually required).

Considerations:
1. The easiest solution is to just add a test for protocolInHTTPFamily before
calling makeSimpleCrossOriginAccessRequest. 

2. Alternatively, one could change the signature of
isSimpleCrossOriginAccessRequest to pass the request and let the method check
if it is a custom scheme as part of its determination.	This is probably a
cleaner solution from a code maintenance standpoint, but requires changing more
code now.

3. Because of potential security risks from scheme handlers that had counted on
webkit to prevent any access via AJAX, perhaps we need a more robust solution.
One suggestion: a registration method so that schemes can pro-actively elect to
have CORS mediate requests.

I've attached a patch for option #1.