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.
