From: Ruediger Pluem Date: Mon, 16 Nov 2020 12:24:12 +0000 (+0000) Subject: Merge r1881620, r1881635 from trunk: X-Git-Tag: 2.4.47~233 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f7173e91099ce59dc1654e5a94b3cde7a892f7fa;p=thirdparty%2Fapache%2Fhttpd.git Merge r1881620, r1881635 from trunk: Process early errors via a dummy HTTP/1.1 request as well Process early errors via a dummy HTTP/1.1 request as well to ensure that the request gets logged correctly and possible custom error pages are considered. The previous way of directly sending a HTTP/2 answer with the HTTP status code appropriate for the error is more efficient, but does not log the request nor sents a possible custom error page. * modules/http2/h2.h: Add http_status to h2_request struct and define H2_HTTP_STATUS_UNSET. * modules/http2/h2_request.c(h2_request_create_rec): Check if http_status is set for the request and die with the status code it contains if set. * modules/http2/h2_session.c(on_header_cb): Adjust the error condition now that we mark early errors via http_status: Only return an error if the status is not success and http_status is not H2_HTTP_STATUS_UNSET. * modules/http2/h2_stream.c(set_error_response): Set http_status on the request instead of creating headers for a response and a respective brigade. Github: closes #137 * Changelog for r1881620 Submitted by: rpluem Reviewed by: rpluem, giovanni, ylavic Github: closes #142 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1883475 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 2603312f31c..92c4d1d85eb 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,10 @@ -*- coding: utf-8 -*- Changes with Apache 2.4.47 + *) mod_http2: Log requests and sent the configured error response in case of + early detected errors like too many or too long headers. + [Ruediger Pluem, Stefan Eissing] + *) mod_md: lowered the required minimal libcurl version from 7.50 to 7.29 as proposed by . [Stefan Eissing] diff --git a/STATUS b/STATUS index 3591db74edc..e5416fc7bca 100644 --- a/STATUS +++ b/STATUS @@ -154,16 +154,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK: 2.4.x patch: http://people.apache.org/~covener/patches/wstunnel-decline.diff +1 covener, jim, rpluem, jfclere - *) mod_http2: Log requests and sent the configured error response in case of - early detected errors like too many or too long headers. - Trunk version of patch: - https://svn.apache.org/r1881620 - https://svn.apache.org/r1881635 - Backport version for 2.4.x of patch: - https://github.com/apache/httpd/pull/142 - https://github.com/apache/httpd/pull/142.diff - +1: rpluem, giovanni, ylavic - *) core: Avoid a core dump if 'AllowOverride nonfatal' is used in a configuration file. Trunk version of patch: diff --git a/modules/http2/h2.h b/modules/http2/h2.h index e074204c739..08f59c44f90 100644 --- a/modules/http2/h2.h +++ b/modules/http2/h2.h @@ -141,8 +141,19 @@ struct h2_request { unsigned int chunked : 1; /* iff request body needs to be forwarded as chunked */ unsigned int serialize : 1; /* iff this request is written in HTTP/1.1 serialization */ apr_off_t raw_bytes; /* RAW network bytes that generated this request - if known. */ + int http_status; /* Store a possible HTTP status code that gets + * defined before creating the dummy HTTP/1.1 + * request e.g. due to an error already + * detected. + */ }; +/* + * A possible HTTP status code is not defined yet. See the http_status field + * in struct h2_request above for further explanation. + */ +#define H2_HTTP_STATUS_UNSET (0) + typedef struct h2_headers h2_headers; struct h2_headers { diff --git a/modules/http2/h2_request.c b/modules/http2/h2_request.c index 149acad8182..51f61843630 100644 --- a/modules/http2/h2_request.c +++ b/modules/http2/h2_request.c @@ -79,11 +79,12 @@ apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool, } req = apr_pcalloc(pool, sizeof(*req)); - req->method = apr_pstrdup(pool, r->method); - req->scheme = scheme; - req->authority = authority; - req->path = path; - req->headers = apr_table_make(pool, 10); + req->method = apr_pstrdup(pool, r->method); + req->scheme = scheme; + req->authority = authority; + req->path = path; + req->headers = apr_table_make(pool, 10); + req->http_status = H2_HTTP_STATUS_UNSET; if (r->server) { req->serialize = h2_config_rgeti(r, H2_CONF_SER_HEADERS); } @@ -327,6 +328,13 @@ request_rec *h2_request_create_rec(const h2_request *req, conn_rec *c) } } + if (req->http_status != H2_HTTP_STATUS_UNSET) { + access_status = req->http_status; + r->status = HTTP_OK; + /* Be safe and close the connection */ + c->keepalive = AP_CONN_CLOSE; + } + /* * Add the HTTP_IN filter here to ensure that ap_discard_request_body * called by ap_die and by ap_send_error_response works correctly on diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c index 33c9f7f28d1..56254f2a898 100644 --- a/modules/http2/h2_session.c +++ b/modules/http2/h2_session.c @@ -311,7 +311,9 @@ static int on_header_cb(nghttp2_session *ngh2, const nghttp2_frame *frame, status = h2_stream_add_header(stream, (const char *)name, namelen, (const char *)value, valuelen); - if (status != APR_SUCCESS && !h2_stream_is_ready(stream)) { + if (status != APR_SUCCESS + && (!stream->rtmp + || stream->rtmp->http_status == H2_HTTP_STATUS_UNSET)) { return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } return 0; diff --git a/modules/http2/h2_stream.c b/modules/http2/h2_stream.c index 77b764e1ded..dce1fb45b4b 100644 --- a/modules/http2/h2_stream.c +++ b/modules/http2/h2_stream.c @@ -639,16 +639,7 @@ void h2_stream_set_request(h2_stream *stream, const h2_request *r) static void set_error_response(h2_stream *stream, int http_status) { if (!h2_stream_is_ready(stream)) { - conn_rec *c = stream->session->c; - apr_bucket *b; - h2_headers *response; - - response = h2_headers_die(http_status, stream->request, stream->pool); - prep_output(stream); - b = apr_bucket_eos_create(c->bucket_alloc); - APR_BRIGADE_INSERT_HEAD(stream->out_buffer, b); - b = h2_bucket_headers_create(c->bucket_alloc, response); - APR_BRIGADE_INSERT_HEAD(stream->out_buffer, b); + stream->rtmp->http_status = http_status; } }