27150 2014-10-24 04:15:35 +0000 CORS preflight flag handling in the HTTP fetch algorithm doesn't look right 2014-10-29 05:32:40 +0000 1 1 1 Unclassified WHATWG Fetch unspecified PC Linux RESOLVED FIXED P2 normal Unsorted 1 tyoshino annevk mike sideshowbarker+fetchspec oldest_to_newest 113684 0 tyoshino 2014-10-24 04:15:35 +0000 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 113704 1 annevk 2014-10-24 15:21:45 +0000 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... 113713 2 tyoshino 2014-10-24 16:42:22 +0000 (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 113785 3 annevk 2014-10-27 17:10:27 +0000 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. 113786 4 tyoshino 2014-10-27 17:14:11 +0000 (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. 113832 5 annevk 2014-10-28 14:02:13 +0000 Thanks. https://github.com/whatwg/fetch/commit/5f6b29fc82dff98aed4fc5ad71d2bcf4713e32f1 114202 6 tyoshino 2014-10-29 05:32:40 +0000 Thanks. The commit looks good.