On Wed, Jul 9, 2014 at 7:35 AM, Marion & Christophe JAILLET <christophe.jail...@wanadoo.fr> wrote: > Just a few details : > > 1) Shouldn't we use 100-continue (lowercase c) instead, to more closely > match http://www.w3.org/Protocols/rfc2616/rfc2616-sec8.html, ยง 8.2.3 ? > This would also be consistent with the use of this string in > protocol.c > > > 2) if of any use, in the fast path, strcmp could be used instead of > strcasecmp
It seems that HTTP (unquoted-)tokens are case insensitive: http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.20 : "Comparison of expectation values is case-insensitive for unquoted tokens (including the 100-continue token), and is case-sensitive for quoted-string expectation-extensions." So we can probably use lowercase 100-continue to conform the rfc "wording", but the case sentive comparison looks invalid. > > > 3) // fast path, should be /* fast path */ Thanks, will fix it. > > > 4) in protocol.c, around line 1212 there is: > > if (((expect = apr_table_get(r->headers_in, "Expect")) != NULL) > && (expect[0] != '\0')) { > /* > * The Expect header field was added to HTTP/1.1 after RFC 2068 > * as a means to signal when a 100 response is desired and, > * unfortunately, to signal a poor man's mandatory extension that > * the server must understand or return 417 Expectation Failed. > */ > if (strcasecmp(expect, "100-continue") == 0) { > r->expecting_100 = 1; > } > > this is not consistent with the code below. Should this be changed to > something like: > if (strcasecmp(expect, "100-continue") == 0 || > ap_find_token(r->pool, expect, "100-Continue")) { > r->expecting_100 = 1; > } > ? +1, that's another issue though.