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 <[email protected]> wrote:
>
>
>
> On Tue, Oct 8, 2013 at 9:01 PM, Jim Jagielski <[email protected]> wrote:
>
>>
>> On Oct 8, 2013, at 1:25 PM, Yann Ylavic <[email protected]> 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.
>
>
