Re: svn commit: r1608762 - in /httpd/httpd/branches/2.4.x: ./ CHANGES modules/proxy/proxy_util.c
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.
Re: svn commit: r1608762 - in /httpd/httpd/branches/2.4.x: ./ CHANGES modules/proxy/proxy_util.c
On Wed, Jul 9, 2014 at 12:56 PM, Yann Ylavic ylavic@gmail.com wrote: On Wed, Jul 9, 2014 at 7:35 AM, Marion Christophe JAILLET 3) // fast path, should be /* fast path */ Thanks, will fix it. Done in r1609100 (2.4.x) and r1609101 (trunk).
Re: svn commit: r1608762 - in /httpd/httpd/branches/2.4.x: ./ CHANGES modules/proxy/proxy_util.c
On 9 Jul 2014, at 11:56, Yann Ylavic ylavic@gmail.com wrote: 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. strcmp would only be used on the fast path - the slow path could cover the case insensitive case. Regards, Graham --
Re: svn commit: r1608762 - in /httpd/httpd/branches/2.4.x: ./ CHANGES modules/proxy/proxy_util.c
On Wed, Jul 9, 2014 at 2:12 PM, Graham Leggett minf...@sharp.fm wrote: On 9 Jul 2014, at 11:56, Yann Ylavic ylavic@gmail.com wrote: 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. strcmp would only be used on the fast path - the slow path could cover the case insensitive case. That would be a (little) faster path, but probably hit less often... Regards, Yann.
Re: svn commit: r1608762 - in /httpd/httpd/branches/2.4.x: ./ CHANGES modules/proxy/proxy_util.c
Le 08/07/2014 15:16, j...@apache.org a écrit :
Author: jim
Date: Tue Jul 8 13:16:27 2014
New Revision: 1608762
URL: http://svn.apache.org/r1608762
Log:
Merge r1588519 from trunk:
mod_proxy: When ping/pong is configured for a worker, don't send or forward
100 Continue (interim) response to the client if it does not
expect one.
Submitted by: ylavic
Reviewed/backported by: jim
Modified:
httpd/httpd/branches/2.4.x/ (props changed)
httpd/httpd/branches/2.4.x/CHANGES
httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c
Propchange: httpd/httpd/branches/2.4.x/
--
Merged /httpd/httpd/trunk:r1588519
Modified: httpd/httpd/branches/2.4.x/CHANGES
URL:
http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1608762r1=1608761r2=1608762view=diff
==
--- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Tue Jul 8 13:16:27 2014
@@ -2,6 +2,10 @@
Changes with Apache 2.4.10
+ *) mod_proxy: When ping/pong is configured for a worker, don't send or
+ forward 100 Continue (interim) response to the client if it does
+ not expect one. [Yann Ylavic]
+
*) mod_proxy_fcgi: Fix occasional high CPU when handling request bodies.
[Jeff Trawick]
Modified: httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c?rev=1608762r1=1608761r2=1608762view=diff
==
--- httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c Tue Jul 8 13:16:27
2014
@@ -3307,8 +3307,22 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbr
* to backend
*/
if (do_100_continue) {
-apr_table_mergen(r-headers_in, Expect, 100-Continue);
-r-expecting_100 = 1;
+const char *val;
+
+if (!r-expecting_100) {
+/* Don't forward any 100 Continue response if the client is
+ * not expecting it.
+ */
+apr_table_setn(r-subprocess_env, proxy-interim-response,
+ Suppress);
+}
+
+/* Add the Expect header if not already there. */
+if (((val = apr_table_get(r-headers_in, Expect)) == NULL)
+|| (strcasecmp(val, 100-Continue) != 0 // fast path
+ !ap_find_token(r-pool, val, 100-Continue))) {
+apr_table_mergen(r-headers_in, Expect, 100-Continue);
+}
}
/* X-Forwarded-*: handling
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
3) // fast path, should be /* fast path */
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;
}
?
Best regards,
CJ
---
Ce courrier électronique ne contient aucun virus ou logiciel malveillant parce
que la protection avast! Antivirus est active.
http://www.avast.com
