Here is a way to reproduce the issues (trunk and 2.4.x) discussed : $ cat httpd.conf [...] <VirtualHost 127.0.0.1:8080> ServerName localhost:8080 ProxyPass / http://localhost:80/ ping=10 ProxyPassReverse / http://localhost:80/ </VirtualHost>
For this first request, the client does not expect a 100-continue but gets one : $ nc localhost 8080 <<EOS POST / HTTP/1.1 Host: localhost:8080 Content-Type: text/plain Content-Length: 10 123456789 EOS HTTP/1.1 100 Continue HTTP/1.1 404 Not Found Server: Apache Content-Length: 257 Content-Type: text/html; charset=iso-8859-1 [...] <address>Apache Server at localhost Port 80</address> </body></html> For this second request, the backend (httpd-2.2.16) does not like the double "Expect: 100-continue" : $ nc localhost 8080 <<EOS POST / HTTP/1.1 Host: localhost:8080 Content-Type: text/plain Content-Length: 10 Expect: 100-continue 123456789 EOS HTTP/1.1 100 Continue HTTP/1.1 417 Expectation Failed Server: Apache Content-Length: 437 Content-Type: text/html; charset=iso-8859-1 [...] <p>The expectation given in the Expect request-header field could not be met by this server. The client sent<pre> Expect: 100-continue, 100-Continue </pre> </p><p>Only the 100-continue expectation is supported.</p> <hr> <address>Apache Server at localhost Port 80</address> </body></html> With the patch proposed, it works as expected, regards. On Thu, Oct 10, 2013 at 1:10 AM, Yann Ylavic <ylavic....@gmail.com> wrote: > > > > On Tue, Oct 8, 2013 at 9:01 PM, Jim Jagielski <j...@jagunet.com> wrote: > >> >> On Oct 8, 2013, at 1:25 PM, Yann Ylavic <ylavic....@gmail.com> wrote: >> >> > Helo, >> > >> > in the case where a ping is configured in a worker to check backend's >> connection (re)usability, ap_proxy_create_hdrbrgd will force >> r->expecting_100 (r1516930). >> > >> > As I understand it, r->expecting_100 relates to the client's >> connection, and is used by ap_http_filter to deal with client's >> 100-continue expectation, or by ap_send_interim_response to check whether >> the client expects one (or do nothing). >> > >> > Hence why is ap_proxy_create_hdrbrgd setting r->expecting_100 for the >> purpose of the backend connection? >> > >> >> because we are forcing the 100-continue on that request. >> See ap_proxy_http_process_response() >> >> > For what I understand from this ap_proxy_http_process_response() code : > > if (interim_response) { > /* RFC2616 tells us to forward this. > * > * OTOH, an interim response here may mean the backend > * is playing sillybuggers. The Client didn't ask for > * it within the defined HTTP/1.1 mechanisms, and if > * it's an extension, it may also be unsupported by us. > * > * There's also the possibility that changing existing > * behaviour here might break something. > * > * So let's make it configurable. > * > * We need to set "r->expecting_100 = 1" otherwise origin > * server behaviour will apply. > */ > const char *policy = apr_table_get(r->subprocess_env, > "proxy-interim-response"); > [...] > if (!policy > || (!strcasecmp(policy, "RFC") && ((r->expecting_100 = > 1)))) { > ap_send_interim_response(r, 1); > } > [...or else discard that response...] > } > > ap_proxy_http_process_response() takes care of whether to forward a "100 > Continue" response from the backend to the client, > ap_send_interim_response() won't send anything unless r->expecting_100, > but ENV:proxy-interim-response can force things. > > Is setting r->expecting_100 in ap_proxy_create_hdrbrgd() for > ap_proxy_http_process_response() to forward the interim response(s)? > If so, the bad path is that ap_http_filter() will first use > r->expecting_100 (and reset it) for sending its own interim response (which > isn't expected!). > That's because the request body is prefetched (and then forwarded) before > ap_proxy_http_process_response() is called, hence r->expecting_100 will > never reach ap_proxy_http_process_response(), and no upcoming "100 > Continue" response from the backend will be forwarded to the client (unless > ENV:proxy-interim-response is "RFC"). > > However there are 2 cases where mod_proxy_http expects a "100 Continue" : > 1. it forwards an "Expect: 100" from the client, or/and > 2. it adds/uses the "Expect: 100" as a ping/continue-pong. > > And the RFC2616 states : > - 10.1 Informational 1xx > [...] > Proxies MUST forward 1xx responses, unless the connection between the > proxy and its client has been closed, or unless the proxy itself > requested the generation of the 1xx response. (For example, if a > proxy adds a "Expect: 100-continue" field when it forwards a request, > then it need not forward the corresponding 100 (Continue) > response(s).) > > For case 1 (with or without case 2), let ap_proxy_http_process_response() > choose as usual whether to forward the interims. > > For case 2 (exclusive), there is no need to forward the "100 Continue", so > no r->expecting_100 setting is required in ap_proxy_create_hdrbrgd(), and > apr_table_setn("proxy-interim-response", "Suppress") would even be > appropriate. > > Hence my proposal to simply not touch r->expecting_100 in > ap_proxy_create_hdrbrgd()... > > I would > now > rather propose : > > Index: modules/proxy/proxy_util.c > =================================================================== > --- modules/proxy/proxy_util.c (revision 1530798) > +++ modules/proxy/proxy_util.c (working copy) > @@ -3169,8 +3169,17 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo > * to backend > */ > if (do_100_continue) { > - apr_table_mergen(r->headers_in, "Expect", "100-Continue"); > > - r->expecting_100 = 1; > + if (!ap_find_token(r->pool, apr_table_get(r->headers_in, > "Expect"), > + "100-Continue")) { > + apr_table_mergen(r->headers_in, "Expect", "100-Continue"); > + } > + if (!r->expecting_100) { > + /* The client does no expect a 100-Continue response, > + * strip any > + */ > + apr_table_setn(r->subprocess_env, "proxy-interim-response", > + "Suppress"); > + } > } > > /* X-Forwarded-*: handling > [END] > > > Apart from this issue, for case 1 (with or without case 2), mod_proxy_http > seems to be in a awkward position relative to the 100-Continue Proxy > described in RFC2616 but more precisely in > http://tools.ietf.org/html/draft-ietf-httpbis-p2-semantics-24#section-5.1.1: > > - 5.1.1. Expect > > [...] > A proxy MUST, upon receiving an HTTP/1.1 (or later) request-line and > a complete header section that contains a 100-continue expectation > and indicates a request message body will follow, either send an > immediate response with a final status code, if that status can be > determined by examining just the request-line and header fields, or > begin forwarding the request toward the origin server by sending a > corresponding request-line and header section to the next inbound > server. If the proxy believes (from configuration or past > interaction) that the next inbound server only supports HTTP/1.0, the > proxy MAY generate an immediate 100 (Continue) response to encourage > the client to begin sending the message body. > > > Since mod_proxy_http won't forward the response (and interims) before > having forwarded the whole request, it acts more as an Origin for the > Client and a Client for the Origin, isn't that broken (per se) for the > purpose of a end-to-end 100-Continue expectation? > > RFC2616 10.1 (above above) states that "Proxies MUST forward 1xx responses > [unless client's connection closed]". > But with the current implementation, the client as nothing to "Continue" > at that point, its whole body is already gone. > Since taking care of not writing on a closed connection looks weird (for a > RFC), couldn't "closed" mean EOS here, and mod_proxy_http should not > forward any "100 Continue" response whatever ENV:proxy-interim-response is? > > Finally, is it a planned work to make mod_proxy_http compliant with > applications that require request/response's available data (at least > headers) be forwarded when they arrive? > > Regards. > >