httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe Jr." <wr...@rowe-clan.net>
Subject Re: http_filter.c r1524770 open issue?
Date Wed, 11 Dec 2013 23:34:11 GMT
On Sat, 23 Nov 2013 19:10:21 +0100
Yann Ylavic <ylavic.dev@gmail.com> wrote:

> On Sat, Nov 23, 2013 at 6:52 PM, Yann Ylavic <ylavic.dev@gmail.com>
> wrote:
> 
> > On Tue, Nov 19, 2013 at 3:27 PM, Yann Ylavic <ylavic.dev@gmail.com>
> > wrote:
> >
> >> On Mon, Nov 18, 2013 at 6:28 PM, William A. Rowe Jr.
> >> <wrowe@rowe-clan.net
> >> > wrote:
> >>
> >>>
> >>> By closing our write-end of the connection, we can signal to the
> >>> server that we can't efficiently forward their response to the
> >>> client (owing to the fact that the server believes this to be a
> >>> keep-alive connection, and that we can't know where this specific
> >>> response ends, until the server has given up on receiving another
> >>> keep-alive request).
> >>>
> >>
> >> This would be a good way to ensure the connection is closed by the
> >> origin, but half-closes are sometimes (and even often) mishandled,
> >> the origin might consider the connection is fully closed
> >> (unwritable) when the read-end is closed, that could be an issue
> >> too.
> >>
> >> Otherwise, the following patch could do the trick :
> >>
> >> Index: modules/http/http_filters.c
> >> ===================================================================
> >> --- modules/http/http_filters.c    (revision 1541907)
> >> +++ modules/http/http_filters.c    (working copy)
> >> @@ -291,13 +291,19 @@ apr_status_t ap_http_filter(ap_filter_t *f,
> >> apr_bu
> >>           * denoted by C-L or T-E: chunked.
> >>           *
> >>           * Note that since the proxy uses this filter to handle
> >> the
> >> -         * proxied *response*, proxy responses MUST be exempt.
> >> +         * proxied *response*, proxy responses MUST be exempt, but
> >> +         * ensure the connection is closed after the response.
> >>           */
> >> -        if (ctx->state == BODY_NONE && f->r->proxyreq !=
> >> PROXYREQ_RESPONSE) {
> >> -            e = apr_bucket_eos_create(f->c->bucket_alloc);
> >> -            APR_BRIGADE_INSERT_TAIL(b, e);
> >> -            ctx->eos_sent = 1;
> >> -            return APR_SUCCESS;
> >> +        if (ctx->state == BODY_NONE) {
> >> +            if (f->r->proxyreq != PROXYREQ_RESPONSE) {
> >> +                e = apr_bucket_eos_create(f->c->bucket_alloc);
> >> +                APR_BRIGADE_INSERT_TAIL(b, e);
> >> +                ctx->eos_sent = 1;
> >> +                return APR_SUCCESS;
> >> +            }
> >> +            f->r->
> >> connection->keepalive = AP_CONN_CLOSE;
> >> +
> >> apr_socket_shutdown(ap_get_conn_socket(f->r->connection),
> >> +                                APR_SHUTDOWN_WRITE);
> >>          }
> >>
> >
> > Actually the shutdown would break SSL (which may need to write
> > during read, ~roughly~).
> > Maybe just closing the connection at the end of the response is
> > enough, which is assured by setting connection->keepalive to
> > AP_CONN_CLOSE here and ap_proxy_http_response() setting the
> > backend->close below in that case.
> >
> 
> Forget about that (chicken and egg problem), the "end of the
> response" is the close on the backend side, so there is no way to
> forcibly do it, just trust...

No, the linger_close logic does all of the read-till-end logic.  Yes,
closing the write end may cause the connection to forcibly terminate,
but we were already in the process of closing that connection.  The
back end SSL should give up on their failure to read-before-write due
to the closed socket.

All we need to do is to return a 400-class error response to the client,
mark the backend connection closed, and close the write side of our
backend connection, letting the linger_close pump do the rest of our
work.  That is, provided that the linger_close logic in the proxy
worker pool behaves as expected.






Mime
View raw message