[webkit-reviews] review requested: [Bug 50773] CORS origin header not set on GET when content type request header is set : [Attachment 81216] Proposed fix and regression test

[bugzilla-daemon at webkit.org]

Martin Galpin <martin at 66laps.com> has asked  for review:
Bug 50773: CORS origin header not set on GET when content type request header
is set
https://bugs.webkit.org/show_bug.cgi?id=50773

Attachment 81216: Proposed fix and regression test
https://bugs.webkit.org/attachment.cgi?id=81216&action=review

------- Additional Comments from Martin Galpin <martin at 66laps.com>
I would suggest the problem is this:

Interest starts when a request reaches DocumentThreadableLoader
(see Source/WebCore/loaders/DocumentThreadableLoader.cpp).
 
After creating a new instance of ResourceRequest (for CORS) [line 87], 
it's checked if the request is "simple" or not (e.g. requires preflight).

If the request is simple (or preflight is forced), the following sequence 
takes place:

* DocumentThreadableLoader::makeSimpleCrossOriginAccessRequest is called
  * The request origin is set [line 115]
* Request is submitted via DocumentThreadableLoader::loadRequest [line 311]

However, if the request is *not* simple, the following happens:

* DocumentThreadableLoader::makeCrossOriginAccessRequestWithPreflight is called

  * A new OPTIONS request is created (and request origin set [line 125])
* Request is submitted via DocumentThreadableLoader::loadRequest [line 149]
* If the preflight succeeds, DocumentThreadableLoader::preflightSuccess() is
  called and the original request submitted [line 302]
...
(see Source/WebCore/loaders/FrameLoader.cpp)
* FrameLoader::addHTTPOriginIfNeeded [line 2680] is called for the CORS request

  and returns without setting the origin because it is not included by default
  on GET requests (for privacy concerns, see line 2690)
* Request later fails WebCore::passesAccessControlCheck at line 111
  (see Source/WebCore/loaders/CrossOriginAccessControl.cpp)

So the problem, therefore, is that whilst the origin header is explictly set
for a simple cross-origin GET request in (makeSimpleCrossOriginAccessRequest),
it is not explicitly after a preflight (and fails the implicit check in
FrameLoader).

I attach a simple patch that fixes this by explictly setting the origin
when the cross-origin preflight is completed successfully. Also
included is a regression test for the issue.