From d167e6e416e5ee74293394dd6ce67c5e63a31bb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20B=C3=BChler?= Date: Sun, 25 Aug 2019 01:52:57 +0200 Subject: [PATCH] [core/mod_proxy] support http backends trying to run keep-alive Even if they shouldn't (due to HTTP/1.0 or Connection; close) some backends send HTTP/1.1 without Connection: close, and use Content-Length to signal end of response (and don't close the connection, as they wait for another request). Now Content-Length is used to find the end of the response (chunked transfer-encoding was already supported). mod_proxy now signals HTTP/1.1, but also sends "Connection: close": it doesn't reuse the connection yet. Change-Id: Ica0c9b3b7da79899412a746f21e7348ccd3d23ee --- include/lighttpd/http_response_parser.h | 2 + include/lighttpd/stream_http_response.h | 2 +- src/main/http_response_parser.rl | 9 +-- src/main/stream_http_response.c | 89 ++++++++++++++++++++++++- src/modules/fastcgi_stream.c | 2 +- src/modules/mod_proxy.c | 7 +- src/modules/mod_scgi.c | 2 +- tests/t-mod-proxy.py | 15 +++++ 8 files changed, 115 insertions(+), 13 deletions(-) diff --git a/include/lighttpd/http_response_parser.h b/include/lighttpd/http_response_parser.h index 36872c9..10617c9 100644 --- a/include/lighttpd/http_response_parser.h +++ b/include/lighttpd/http_response_parser.h @@ -12,6 +12,8 @@ struct liHttpResponseCtx { gboolean accept_cgi, accept_nph; gboolean drop_header; /* for 1xx responses */ + liHttpVersion http_version; + liChunkParserMark mark; GString *h_key, *h_value; }; diff --git a/include/lighttpd/stream_http_response.h b/include/lighttpd/stream_http_response.h index 6cae84b..15f5959 100644 --- a/include/lighttpd/stream_http_response.h +++ b/include/lighttpd/stream_http_response.h @@ -3,6 +3,6 @@ #include -LI_API liStream* li_stream_http_response_handle(liStream *http_in, liVRequest *vr, gboolean accept_cgi, gboolean accept_nph); +LI_API liStream* li_stream_http_response_handle(liStream *http_in, liVRequest *vr, gboolean accept_cgi, gboolean accept_nph, gboolean keepalive); #endif diff --git a/src/main/http_response_parser.rl b/src/main/http_response_parser.rl index c70da09..9c8a9cb 100644 --- a/src/main/http_response_parser.rl +++ b/src/main/http_response_parser.rl @@ -95,13 +95,13 @@ Quoted_String = DQUOTE ( QDText | Quoted_Pair )* DQUOTE; HTTP_Version = ( - "HTTP/1.0" %{ ctx->response->http_version = LI_HTTP_VERSION_1_0; } - | "HTTP/1.1" %{ ctx->response->http_version = LI_HTTP_VERSION_1_1; } - | "HTTP" "/" DIGIT+ "." DIGIT+ ) >{ ctx->response->http_version = LI_HTTP_VERSION_UNSET; }; + "HTTP/1.0" %{ ctx->http_version = LI_HTTP_VERSION_1_0; } + | "HTTP/1.1" %{ ctx->http_version = LI_HTTP_VERSION_1_1; } + | "HTTP" "/" DIGIT+ "." DIGIT+ ) >{ ctx->http_version = LI_HTTP_VERSION_UNSET; }; #HTTP_URL = "http:" "//" Host ( ":" Port )? ( abs_path ( "?" query )? )?; Status = (digit digit digit) >mark %status; - Response_Line = "HTTP/" digit+ "." digit+ SP Status SP (any - CTL - CR - LF)* CRLF; + Response_Line = HTTP_Version SP Status SP (any - CTL - CR - LF)* CRLF; # Field_Content = ( TEXT+ | ( Token | Separators | Quoted_String )+ ); Field_Content = ( (OCTET - CTL - DQUOTE) | SP | HT | Quoted_String )+; @@ -127,6 +127,7 @@ void li_http_response_parser_init(liHttpResponseCtx* ctx, liResponse *req, liChu ctx->accept_cgi = accept_cgi; ctx->accept_nph = accept_nph; ctx->drop_header = FALSE; + ctx->http_version = LI_HTTP_VERSION_UNSET; ctx->h_key = g_string_sized_new(0); ctx->h_value = g_string_sized_new(0); diff --git a/src/main/stream_http_response.c b/src/main/stream_http_response.c index 67ad6fa..05ea777 100644 --- a/src/main/stream_http_response.c +++ b/src/main/stream_http_response.c @@ -7,7 +7,8 @@ struct liStreamHttpResponse { liStream stream; liVRequest *vr; - gboolean response_headers_finished, transfer_encoding_chunked; + gboolean keepalive, response_headers_finished, transfer_encoding_chunked, wait_for_close; + goffset content_length; liFilterChunkedDecodeState chunked_decode_state; }; @@ -16,6 +17,9 @@ static void check_response_header(liStreamHttpResponse* shr) { GList *l; shr->transfer_encoding_chunked = FALSE; + /* if protocol doesn't support keep-alive just wait for stream end */ + shr->wait_for_close = !shr->keepalive; + shr->content_length = -1; /* Transfer-Encoding: chunked */ l = li_http_header_find_first(resp->headers, CONST_STR_LEN("transfer-encoding")); @@ -90,6 +94,73 @@ static void check_response_header(liStreamHttpResponse* shr) { return; } + if (!shr->transfer_encoding_chunked && shr->keepalive) { + /** + * if protocol has HTTP "keepalive" concept and encoding isn't chunked, + * we need to check for content-length or "connection: close" indications. + * otherwise we won't know when the response is done + */ + liHttpHeader *hh; + + switch (shr->parse_response_ctx.http_version) { + case LI_HTTP_VERSION_1_0: + if (!li_http_header_is(shr->vr->response.headers, CONST_STR_LEN("connection"), CONST_STR_LEN("keep-alive"))) + shr->wait_for_close = TRUE; + break; + case LI_HTTP_VERSION_1_1: + if (li_http_header_is(shr->vr->response.headers, CONST_STR_LEN("connection"), CONST_STR_LEN("close"))) + shr->wait_for_close = TRUE; + break; + case LI_HTTP_VERSION_UNSET: + break; + } + + /* content-length */ + hh = li_http_header_lookup(shr->vr->response.headers, CONST_STR_LEN("content-length")); + if (hh) { + const gchar *val = LI_HEADER_VALUE(hh); + gint64 r; + char *err; + + r = g_ascii_strtoll(val, &err, 10); + if (*err != '\0') { + VR_ERROR(shr->vr, "Backend response: content-length is not a number: %s", err); + li_vrequest_error(shr->vr); + return; + } + + /** + * negative content-length is not supported + * and is a bad request + */ + if (r < 0) { + VR_ERROR(shr->vr, "%s", "Backend response: content-length is negative"); + li_vrequest_error(shr->vr); + return; + } + + /** + * check if we had a over- or underrun in the string conversion + */ + if (r == G_MININT64 || r == G_MAXINT64) { + if (errno == ERANGE) { + VR_ERROR(shr->vr, "%s", "Backend response: content-length overflow"); + li_vrequest_error(shr->vr); + return; + } + } + + shr->content_length = r; + shr->wait_for_close = FALSE; + } + + if (!shr->wait_for_close && shr->content_length < 0) { + VR_ERROR(shr->vr, "%s", "Backend: need chunked transfer-encoding or content-length for keepalive connections"); + li_vrequest_error(shr->vr); + return; + } + } + shr->response_headers_finished = TRUE; li_vrequest_indirect_headers_ready(shr->vr); @@ -132,12 +203,23 @@ static void stream_http_response_data(liStreamHttpResponse* shr) { if (shr->stream.source->out->is_closed) { li_stream_disconnect(&shr->stream); } - } else { + } else if (shr->wait_for_close) { li_chunkqueue_steal_all(shr->stream.out, shr->stream.source->out); if (shr->stream.source->out->is_closed) { shr->stream.out->is_closed = TRUE; li_stream_disconnect(&shr->stream); } + } else { + g_assert(shr->content_length >= 0); + if (shr->content_length > 0) { + goffset moved; + moved = li_chunkqueue_steal_len(shr->stream.out, shr->stream.source->out, shr->content_length); + shr->content_length -= moved; + } + if (shr->content_length == 0) { + shr->stream.out->is_closed = TRUE; + li_stream_disconnect(&shr->stream); + } } li_stream_notify(&shr->stream); } @@ -170,8 +252,9 @@ static void stream_http_response_cb(liStream *stream, liStreamEvent event) { } } -LI_API liStream* li_stream_http_response_handle(liStream *http_in, liVRequest *vr, gboolean accept_cgi, gboolean accept_nph) { +LI_API liStream* li_stream_http_response_handle(liStream *http_in, liVRequest *vr, gboolean accept_cgi, gboolean accept_nph, gboolean keepalive) { liStreamHttpResponse *shr = g_slice_new0(liStreamHttpResponse); + shr->keepalive = keepalive; shr->response_headers_finished = FALSE; shr->vr = vr; li_stream_init(&shr->stream, &vr->wrk->loop, stream_http_response_cb); diff --git a/src/modules/fastcgi_stream.c b/src/modules/fastcgi_stream.c index 04e5929..1e34f71 100644 --- a/src/modules/fastcgi_stream.c +++ b/src/modules/fastcgi_stream.c @@ -859,7 +859,7 @@ liBackendResult li_fastcgi_backend_get(liVRequest *vr, liFastCGIBackendPool *bpo fastcgi_send_env(vr, ctx->fcgi_out.out, 1); li_stream_notify_later(&ctx->fcgi_out); - http_out = li_stream_http_response_handle(&ctx->fcgi_in, vr, TRUE, TRUE); + http_out = li_stream_http_response_handle(&ctx->fcgi_in, vr, TRUE, TRUE, FALSE); li_vrequest_handle_indirect(vr, NULL); li_vrequest_indirect_connect(vr, &ctx->fcgi_out, http_out); diff --git a/src/modules/mod_proxy.c b/src/modules/mod_proxy.c index 30a24cd..209556e 100644 --- a/src/modules/mod_proxy.c +++ b/src/modules/mod_proxy.c @@ -52,8 +52,9 @@ static void proxy_send_headers(liVRequest *vr, liChunkQueue *out) { switch (vr->request.http_version) { case LI_HTTP_VERSION_1_1: - /* g_string_append_len(head, CONST_STR_LEN(" HTTP/1.1\r\n")); */ - g_string_append_len(head, CONST_STR_LEN(" HTTP/1.0\r\n")); + g_string_append_len(head, CONST_STR_LEN(" HTTP/1.1\r\n")); + /* although we understand keep-alive signalling we don't reuse connection, so tell backend */ + g_string_append_len(head, CONST_STR_LEN("Connection: close\r\n")); break; case LI_HTTP_VERSION_1_0: default: @@ -213,7 +214,7 @@ static void proxy_connection_new(liVRequest *vr, liBackendConnection *bcon, prox proxy_send_headers(vr, outplug->out); li_stream_notify_later(outplug); - http_out = li_stream_http_response_handle(&iostream->stream_in, vr, TRUE, FALSE); + http_out = li_stream_http_response_handle(&iostream->stream_in, vr, TRUE, FALSE, TRUE); li_vrequest_handle_indirect(vr, NULL); li_vrequest_indirect_connect(vr, outplug, http_out); diff --git a/src/modules/mod_scgi.c b/src/modules/mod_scgi.c index 34b566f..679a394 100644 --- a/src/modules/mod_scgi.c +++ b/src/modules/mod_scgi.c @@ -307,7 +307,7 @@ static void scgi_connection_new(liVRequest *vr, liBackendConnection *bcon, scgi_ scgi_send_env(vr, outplug->out); li_stream_notify_later(outplug); - http_out = li_stream_http_response_handle(&iostream->stream_in, vr, TRUE, FALSE); + http_out = li_stream_http_response_handle(&iostream->stream_in, vr, TRUE, FALSE, FALSE); li_vrequest_handle_indirect(vr, NULL); li_vrequest_indirect_connect(vr, outplug, http_out); diff --git a/tests/t-mod-proxy.py b/tests/t-mod-proxy.py index 407bc8b..c38c361 100644 --- a/tests/t-mod-proxy.py +++ b/tests/t-mod-proxy.py @@ -19,6 +19,10 @@ class HttpBackendHandler(socketserver.StreamRequestHandler): if reqline[2].upper() != "HTTP/1.1": keepalive = False keepalive_default = False + if reqline[1].startswith("/keepalive"): + # simulate broken backend (HTTP/1.0 incompatible) + keepalive = True + keepalive_default = True # read headers; and GET has no body while True: hdr = self.rfile.readline().decode('utf-8').rstrip() @@ -85,11 +89,22 @@ rewrite "/foo(.*)" => "/dest$1"; backend_proxy; """ +# backend gets decoded %2F +class TestBackendForcedKeepalive(CurlRequest): + URL = "/keepalive" + EXPECT_RESPONSE_BODY = "/keepalive" + EXPECT_RESPONSE_CODE = 200 + no_docroot = True + config = """ +backend_proxy; +""" + class Test(GroupTest): group = [ TestSimple, TestProxiedRewrittenEncodedURL, TestProxiedRewrittenDecodedURL, + TestBackendForcedKeepalive, ] def Prepare(self):