httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: mod_proxy ping and r->expecting_100
Date Wed, 16 Oct 2013 00:07:08 GMT
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.dev@gmail.com> wrote:

>
>
>
> On Tue, Oct 8, 2013 at 9:01 PM, Jim Jagielski <jim@jagunet.com> wrote:
>
>>
>> On Oct 8, 2013, at 1:25 PM, Yann Ylavic <ylavic.dev@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.
>
>

Mime
View raw message