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.