This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 27150 - CORS preflight flag handling in the HTTP fetch algorithm doesn't look right
Summary: CORS preflight flag handling in the HTTP fetch algorithm doesn't look right
Status: RESOLVED FIXED
Alias: None
Product: WHATWG
Classification: Unclassified
Component: Fetch (show other bugs)
Version: unspecified
Hardware: PC Linux
: P2 normal
Target Milestone: Unsorted
Assignee: Anne
QA Contact: sideshowbarker+fetchspec
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-24 04:15 UTC by Takeshi Yoshino
Modified: 2014-10-29 05:32 UTC (History)
1 user (show)

See Also:


Attachments

Description Takeshi Yoshino 2014-10-24 04:15:35 UTC
In the substep 1 of the step 3 (handling CORS preflight flag),

It looks that this step is saying that if all of the first 4 lines at the top (including "the CORS preflight flag is set") are met, we must run the CORS preflight fetch.

But looking at the last 2 substeps,

----

- either request's method is a not simple method or request's mode is CORS-with-forced-preflight, and

- there is at least one header name in request's header list for which there is no header name cache match using request and for which the header is not a simple header

----

it looks wrong that these two conditions (bullet points) are concatenated with AND.

I guess CORS-with-forced-preflight check, method check, and header check should be concatenated with OR.

Suggested rewrite:

If the CORS preflight flag is set and
- request's mode is CORS-with-forced-preflight, or
- there is no method cache match for request's method using request and request's method is not a simple method, or
- there is at least one header name in request's header list for which there is no header name cache match using request and which is not a simple header
Comment 1 Anne 2014-10-24 15:21:45 UTC
That proposal does not work. Even if mode is CORS-with-forced-preflight, if there's a cache match there's no need to do a preflight.

My proposal:

If the /CORS preflight flag/ is set and one of these conditions is true:

* There is no method cache match for request's method using request and either request's method is a not simple method or request's mode is CORS-with-forced-preflight.

* There is at least one header name in request's header list for which there is no header name cache match using request and for which the header is not a simple header.

Then preflight...
Comment 2 Takeshi Yoshino 2014-10-24 16:42:22 UTC
(In reply to Anne from comment #1)
> That proposal does not work. Even if mode is CORS-with-forced-preflight, if
> there's a cache match there's no need to do a preflight.

I see!

> My proposal:
> 
> If the /CORS preflight flag/ is set and one of these conditions is true:
> 
> * There is no method cache match for request's method using request and
> either request's method is a not simple method or request's mode is
> CORS-with-forced-preflight.
> 
> * There is at least one header name in request's header list for which there
> is no header name cache match using request and for which the header is not
> a simple header.

(The same about the current text, but) I couldn't parse the second "for which". So, I rewrote it to "and which is not a simple header".

> 
> Then preflight...

OK. But I wonder if it works correctly for the following case,
- method cache match (therefore, the first bullet point is false)
- there're a simple headers with no header cache match
This happens when e.g. a server builds the Access-Control-Allow-Headers header by scanning the items listed in received Access-Control-Request-Headers and returning only allowed ones of them.


--- Request 1 ---

GET / HTTP/1.1

HTTP/1.1 200 OK
Access-Control-Allow-Methods: GET, POST, HEAD
Access-Control-Allow-Headers: 
...

--- Request 2 ---

POST / HTTP/1.1
Content-Type: text/plain


Here, the two bullet points are both false even if CORS-with-forced-preflight is set.


Memo:
Force preflight was introduced on this revision.
http://dev.w3.org/cvsweb/2006/waf/access-control/Overview.src.html#rev1.196
The thread led to the change:
http://lists.w3.org/Archives/Public/public-webapps/2008JulSep/0069.html
Comment 3 Anne 2014-10-27 17:10:27 UTC
If there is a method cache match we already know the server is okay with CORS and we do not have to check again. In that case we only have to check for headers and such.

The forced-preflight bit is about minimizing exposure of a server that might be unaware of CORS.
Comment 4 Takeshi Yoshino 2014-10-27 17:14:11 UTC
(In reply to Anne from comment #3)
> If there is a method cache match we already know the server is okay with
> CORS and we do not have to check again. In that case we only have to check
> for headers and such.
> 
> The forced-preflight bit is about minimizing exposure of a server that might
> be unaware of CORS.

OK. Then, the fix looks good.
Comment 6 Takeshi Yoshino 2014-10-29 05:32:40 UTC
Thanks. The commit looks good.