You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2021/01/16 13:49:50 UTC
svn commit: r1885571 - in /httpd/httpd/branches/2.4.x: CHANGES STATUS
include/ap_mmn.h include/http_protocol.h modules/http/http_filters.c
modules/http/http_protocol.c modules/proxy/mod_proxy_http.c server/protocol.c
Author: minfrin
Date: Sat Jan 16 13:49:50 2021
New Revision: 1885571
URL: http://svn.apache.org/viewvc?rev=1885571&view=rev
Log:
Backport to 2.4:
*) mod_proxy_http: Fix 100-continue deadlock for spooled request bodies. BZ 63855.
trunk patch: https://svn.apache.org/r1769760
https://svn.apache.org/r1770220
https://svn.apache.org/r1868576
https://svn.apache.org/r1868653
https://svn.apache.org/r1883639
https://svn.apache.org/r1883640
https://svn.apache.org/r1884218
https://svn.apache.org/r1884220
2.4.x patch: http://people.apache.org/~ylavic/patches/2.4.x-bz63855-1on5.patch
https://github.com/apache/httpd/pull/154
+1: ylavic, covener, minfrin
Modified:
httpd/httpd/branches/2.4.x/CHANGES
httpd/httpd/branches/2.4.x/STATUS
httpd/httpd/branches/2.4.x/include/ap_mmn.h
httpd/httpd/branches/2.4.x/include/http_protocol.h
httpd/httpd/branches/2.4.x/modules/http/http_filters.c
httpd/httpd/branches/2.4.x/modules/http/http_protocol.c
httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy_http.c
httpd/httpd/branches/2.4.x/server/protocol.c
Modified: httpd/httpd/branches/2.4.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1885571&r1=1885570&r2=1885571&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Sat Jan 16 13:49:50 2021
@@ -1,6 +1,12 @@
-*- coding: utf-8 -*-
Changes with Apache 2.4.47
+ *) http: Allow unknown response status' lines returned in the form of
+ "HTTP/x.x xxx Status xxx". [Yann Ylavic]
+
+ *) mod_proxy_http: Fix 100-continue deadlock for spooled request bodies,
+ leading to Request Timeout (408). PR 63855. [Yann Ylavic]
+
*) core: Remove headers on 304 Not Modified as specified by RFC7234, as
opposed to passing an explicit subset of headers. PR 61820.
[Giovanni Bechis]
Modified: httpd/httpd/branches/2.4.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1885571&r1=1885570&r2=1885571&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/STATUS (original)
+++ httpd/httpd/branches/2.4.x/STATUS Sat Jan 16 13:49:50 2021
@@ -138,18 +138,6 @@ RELEASE SHOWSTOPPERS:
PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
[ start all new proposals below, under PATCHES PROPOSED. ]
- *) mod_proxy_http: Fix 100-continue deadlock for spooled request bodies. BZ 63855.
- trunk patch: https://svn.apache.org/r1769760
- https://svn.apache.org/r1770220
- https://svn.apache.org/r1868576
- https://svn.apache.org/r1868653
- https://svn.apache.org/r1883639
- https://svn.apache.org/r1883640
- https://svn.apache.org/r1884218
- https://svn.apache.org/r1884220
- 2.4.x patch: http://people.apache.org/~ylavic/patches/2.4.x-bz63855-1on5.patch
- https://github.com/apache/httpd/pull/154
- +1: ylavic, covener, minfrin
PATCHES PROPOSED TO BACKPORT FROM TRUNK:
Modified: httpd/httpd/branches/2.4.x/include/ap_mmn.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/include/ap_mmn.h?rev=1885571&r1=1885570&r2=1885571&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/include/ap_mmn.h (original)
+++ httpd/httpd/branches/2.4.x/include/ap_mmn.h Sat Jan 16 13:49:50 2021
@@ -537,6 +537,7 @@
* 20120211.93 (2.4.44-dev) Add ap_parse_strict_length()
* 20120211.94 (2.4.47-dev) Add ap_proxy_define_match_worker()
* 20120211.95 (2.4.47-dev) Add proxy check_trans hook
+ * 20120211.96 (2.4.47-dev) Add ap_get_status_line_ex()
*/
#define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
@@ -544,7 +545,7 @@
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 20120211
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 95 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 96 /* 0...n */
/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a
Modified: httpd/httpd/branches/2.4.x/include/http_protocol.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/include/http_protocol.h?rev=1885571&r1=1885570&r2=1885571&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/include/http_protocol.h (original)
+++ httpd/httpd/branches/2.4.x/include/http_protocol.h Sat Jan 16 13:49:50 2021
@@ -466,6 +466,17 @@ AP_DECLARE(int) ap_index_of_response(int
*/
AP_DECLARE(const char *) ap_get_status_line(int status);
+/**
+ * Return the Status-Line for a given status code (excluding the
+ * HTTP-Version field). If an invalid status code is passed,
+ * "500 Internal Server Error" will be returned, whereas an unknown
+ * status will be returned like "xxx Status xxx".
+ * @param p The pool to allocate from when status is unknown
+ * @param status The HTTP status code
+ * @return The Status-Line
+ */
+AP_DECLARE(const char *) ap_get_status_line_ex(apr_pool_t *p, int status);
+
/* Reading a block of data from the client connection (e.g., POST arg) */
/**
Modified: httpd/httpd/branches/2.4.x/modules/http/http_filters.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/http/http_filters.c?rev=1885571&r1=1885570&r2=1885571&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/http/http_filters.c (original)
+++ httpd/httpd/branches/2.4.x/modules/http/http_filters.c Sat Jan 16 13:49:50 2021
@@ -79,7 +79,8 @@ typedef struct http_filter_ctx
BODY_CHUNK_END_LF, /* got CR after data, expect LF */
BODY_CHUNK_TRAILER /* trailers */
} state;
- unsigned int eos_sent :1;
+ unsigned int eos_sent :1,
+ seen_data:1;
apr_bucket_brigade *bb;
} http_ctx_t;
@@ -348,7 +349,6 @@ apr_status_t ap_http_filter(ap_filter_t
http_ctx_t *ctx = f->ctx;
apr_status_t rv;
int http_error = HTTP_REQUEST_ENTITY_TOO_LARGE;
- apr_bucket_brigade *bb;
int again;
/* just get out of the way of things we don't want. */
@@ -361,7 +361,6 @@ apr_status_t ap_http_filter(ap_filter_t
f->ctx = ctx = apr_pcalloc(f->r->pool, sizeof(*ctx));
ctx->state = BODY_NONE;
ctx->bb = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
- bb = ctx->bb;
/* LimitRequestBody does not apply to proxied responses.
* Consider implementing this check in its own filter.
@@ -448,42 +447,43 @@ apr_status_t ap_http_filter(ap_filter_t
ctx->eos_sent = 1;
return APR_SUCCESS;
}
+ }
- /* Since we're about to read data, send 100-Continue if needed.
- * Only valid on chunked and C-L bodies where the C-L is > 0. */
- if ((ctx->state == BODY_CHUNK ||
- (ctx->state == BODY_LENGTH && ctx->remaining > 0)) &&
- f->r->expecting_100 && f->r->proto_num >= HTTP_VERSION(1,1) &&
- !(f->r->eos_sent || f->r->bytes_sent)) {
- if (!ap_is_HTTP_SUCCESS(f->r->status)) {
- ctx->state = BODY_NONE;
- ctx->eos_sent = 1;
- }
- else {
- char *tmp;
- int len;
-
- /* if we send an interim response, we're no longer
- * in a state of expecting one.
- */
- f->r->expecting_100 = 0;
- tmp = apr_pstrcat(f->r->pool, AP_SERVER_PROTOCOL " ",
- ap_get_status_line(HTTP_CONTINUE), CRLF CRLF,
- NULL);
- len = strlen(tmp);
- ap_xlate_proto_to_ascii(tmp, len);
- apr_brigade_cleanup(bb);
- e = apr_bucket_pool_create(tmp, len, f->r->pool,
- f->c->bucket_alloc);
- APR_BRIGADE_INSERT_HEAD(bb, e);
- e = apr_bucket_flush_create(f->c->bucket_alloc);
- APR_BRIGADE_INSERT_TAIL(bb, e);
-
- rv = ap_pass_brigade(f->c->output_filters, bb);
- if (rv != APR_SUCCESS) {
- return AP_FILTER_ERROR;
- }
- }
+ /* Since we're about to read data, send 100-Continue if needed.
+ * Only valid on chunked and C-L bodies where the C-L is > 0.
+ *
+ * If the read is to be nonblocking though, the caller may not want to
+ * handle this just now (e.g. mod_proxy_http), and is prepared to read
+ * nothing if the client really waits for 100 continue, so we don't
+ * send it now and wait for later blocking read.
+ *
+ * In any case, even if r->expecting remains set at the end of the
+ * request handling, ap_set_keepalive() will finally do the right
+ * thing (i.e. "Connection: close" the connection).
+ */
+ if (block == APR_BLOCK_READ
+ && (ctx->state == BODY_CHUNK
+ || (ctx->state == BODY_LENGTH && ctx->remaining > 0))
+ && f->r->expecting_100 && f->r->proto_num >= HTTP_VERSION(1,1)
+ && !(ctx->eos_sent || f->r->eos_sent || f->r->bytes_sent)) {
+ if (!ap_is_HTTP_SUCCESS(f->r->status)) {
+ ctx->state = BODY_NONE;
+ ctx->eos_sent = 1; /* send EOS below */
+ }
+ else if (!ctx->seen_data) {
+ int saved_status = f->r->status;
+ f->r->status = HTTP_CONTINUE;
+ ap_send_interim_response(f->r, 0);
+ AP_DEBUG_ASSERT(!f->r->expecting_100);
+ f->r->status = saved_status;
+ }
+ else {
+ /* https://tools.ietf.org/html/rfc7231#section-5.1.1
+ * A server MAY omit sending a 100 (Continue) response if it
+ * has already received some or all of the message body for
+ * the corresponding request [...]
+ */
+ f->r->expecting_100 = 0;
}
}
@@ -534,9 +534,11 @@ apr_status_t ap_http_filter(ap_filter_t
int parsing = 0;
rv = apr_bucket_read(e, &buffer, &len, APR_BLOCK_READ);
-
if (rv == APR_SUCCESS) {
parsing = 1;
+ if (len > 0) {
+ ctx->seen_data = 1;
+ }
rv = parse_chunk_size(ctx, buffer, len,
f->r->server->limit_req_fieldsize, strict);
}
@@ -598,6 +600,9 @@ apr_status_t ap_http_filter(ap_filter_t
/* How many bytes did we just read? */
apr_brigade_length(b, 0, &totalread);
+ if (totalread > 0) {
+ ctx->seen_data = 1;
+ }
/* If this happens, we have a bucket of unknown length. Die because
* it means our assumptions have changed. */
Modified: httpd/httpd/branches/2.4.x/modules/http/http_protocol.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/http/http_protocol.c?rev=1885571&r1=1885570&r2=1885571&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/http/http_protocol.c (original)
+++ httpd/httpd/branches/2.4.x/modules/http/http_protocol.c Sat Jan 16 13:49:50 2021
@@ -986,14 +986,17 @@ AP_DECLARE(const char *) ap_method_name_
* from status_lines[shortcut[i]] to status_lines[shortcut[i+1]-1];
* or use NULL to fill the gaps.
*/
-AP_DECLARE(int) ap_index_of_response(int status)
+static int index_of_response(int status)
{
- static int shortcut[6] = {0, LEVEL_200, LEVEL_300, LEVEL_400,
- LEVEL_500, RESPONSE_CODES};
+ static int shortcut[6] = {0, LEVEL_200, LEVEL_300, LEVEL_400, LEVEL_500,
+ RESPONSE_CODES};
int i, pos;
- if (status < 100) { /* Below 100 is illegal for HTTP status */
- return LEVEL_500;
+ if (status < 100) { /* Below 100 is illegal for HTTP status */
+ return -1;
+ }
+ if (status > 999) { /* Above 999 is also illegal for HTTP status */
+ return -1;
}
for (i = 0; i < 5; i++) {
@@ -1004,11 +1007,29 @@ AP_DECLARE(int) ap_index_of_response(int
return pos;
}
else {
- return LEVEL_500; /* status unknown (falls in gap) */
+ break;
}
}
}
- return LEVEL_500; /* 600 or above is also illegal */
+ return -2; /* Status unknown (falls in gap) or above 600 */
+}
+
+AP_DECLARE(int) ap_index_of_response(int status)
+{
+ int index = index_of_response(status);
+ return (index < 0) ? LEVEL_500 : index;
+}
+
+AP_DECLARE(const char *) ap_get_status_line_ex(apr_pool_t *p, int status)
+{
+ int index = index_of_response(status);
+ if (index >= 0) {
+ return status_lines[index];
+ }
+ else if (index == -2) {
+ return apr_psprintf(p, "%i Status %i", status, status);
+ }
+ return status_lines[LEVEL_500];
}
AP_DECLARE(const char *) ap_get_status_line(int status)
Modified: httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy_http.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy_http.c?rev=1885571&r1=1885570&r2=1885571&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy_http.c Sat Jan 16 13:49:50 2021
@@ -261,7 +261,6 @@ typedef struct {
rb_methods rb_method;
- int expecting_100;
unsigned int do_100_continue:1,
prefetch_nonblocking:1;
} proxy_http_req_t;
@@ -580,7 +579,7 @@ static int ap_proxy_http_prefetch(proxy_
conn_rec *origin = p_conn->connection;
if (apr_table_get(r->subprocess_env, "force-proxy-request-1.0")) {
- if (req->expecting_100) {
+ if (r->expecting_100) {
return HTTP_EXPECTATION_FAILED;
}
force10 = 1;
@@ -1499,8 +1498,9 @@ int ap_proxy_http_process_response(proxy
*
* So let's make it configurable.
*
- * We need to set "r->expecting_100 = 1" otherwise origin
- * server behaviour will apply.
+ * We need to force "r->expecting_100 = 1" for RFC behaviour
+ * otherwise ap_send_interim_response() does nothing when
+ * the client did not ask for 100-continue.
*/
const char *policy = apr_table_get(r->subprocess_env,
"proxy-interim-response");
@@ -1509,11 +1509,7 @@ int ap_proxy_http_process_response(proxy
if (!policy
|| (!strcasecmp(policy, "RFC")
&& (proxy_status != HTTP_CONTINUE
- || (req->expecting_100 = 1)))) {
- if (proxy_status == HTTP_CONTINUE) {
- r->expecting_100 = req->expecting_100;
- req->expecting_100 = 0;
- }
+ || (r->expecting_100 = 1)))) {
ap_send_interim_response(r, 1);
}
/* FIXME: refine this to be able to specify per-response-status
@@ -1601,12 +1597,10 @@ int ap_proxy_http_process_response(proxy
* here though, because error_override or a potential retry on
* another backend could finally read that data and finalize
* the request processing, making keep-alive possible. So what
- * we do is restoring r->expecting_100 for ap_set_keepalive()
- * to do the right thing according to the final response and
+ * we do is leaving r->expecting_100 alone, ap_set_keepalive()
+ * will do the right thing according to the final response and
* any later update of r->expecting_100.
*/
- r->expecting_100 = req->expecting_100;
- req->expecting_100 = 0;
}
/* Once only! */
@@ -2010,17 +2004,7 @@ static int proxy_http_handler(request_re
/* Should we handle end-to-end or ping 100-continue? */
if ((r->expecting_100 && (dconf->forward_100_continue || input_brigade))
|| PROXY_DO_100_CONTINUE(worker, r)) {
- /* We need to reset r->expecting_100 or prefetching will cause
- * ap_http_filter() to send "100 Continue" response by itself. So
- * we'll use req->expecting_100 in mod_proxy_http to determine whether
- * the client should be forwarded "100 continue", and r->expecting_100
- * will be restored at the end of the function with the actual value of
- * req->expecting_100 (i.e. cleared only if mod_proxy_http sent the
- * "100 Continue" according to its policy).
- */
req->do_100_continue = req->prefetch_nonblocking = 1;
- req->expecting_100 = r->expecting_100;
- r->expecting_100 = 0;
}
/* Should we block while prefetching the body or try nonblocking and flush
* data to the backend ASAP?
@@ -2155,10 +2139,6 @@ cleanup:
req->backend->close = 1;
ap_proxy_http_cleanup(proxy_function, r, req->backend);
}
- if (req->expecting_100) {
- /* Restore r->expecting_100 if we didn't touch it */
- r->expecting_100 = req->expecting_100;
- }
return status;
}
Modified: httpd/httpd/branches/2.4.x/server/protocol.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/server/protocol.c?rev=1885571&r1=1885570&r2=1885571&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/server/protocol.c (original)
+++ httpd/httpd/branches/2.4.x/server/protocol.c Sat Jan 16 13:49:50 2021
@@ -2201,7 +2201,8 @@ static int send_header(void *data, const
AP_DECLARE(void) ap_send_interim_response(request_rec *r, int send_headers)
{
hdr_ptr x;
- char *status_line = NULL;
+ char *response_line = NULL;
+ const char *status_line;
request_rec *rr;
if (r->proto_num < HTTP_VERSION(1,1)) {
@@ -2232,13 +2233,19 @@ AP_DECLARE(void) ap_send_interim_respons
}
}
- status_line = apr_pstrcat(r->pool, AP_SERVER_PROTOCOL, " ", r->status_line, CRLF, NULL);
- ap_xlate_proto_to_ascii(status_line, strlen(status_line));
+ status_line = r->status_line;
+ if (status_line == NULL) {
+ status_line = ap_get_status_line_ex(r->pool, r->status);
+ }
+ response_line = apr_pstrcat(r->pool,
+ AP_SERVER_PROTOCOL " ", status_line, CRLF,
+ NULL);
+ ap_xlate_proto_to_ascii(response_line, strlen(response_line));
x.f = r->connection->output_filters;
x.bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
- ap_fputs(x.f, x.bb, status_line);
+ ap_fputs(x.f, x.bb, response_line);
if (send_headers) {
apr_table_do(send_header, &x, r->headers_out, NULL);
apr_table_clear(r->headers_out);