So these cumulative 4 patches, each of which focuses on a different aspect
of the request handling behavior, represent all of the fixes to 2.4.x that
never
made it to 2.2.x. With these proposals adopted, I can propose a nearly
identical backport of the revised request handling logic to both 2.4.x and
2.2.x,
if folks would be kind enough to review these precursors.
Focusing on only ap_parse_uri through ap_read_request functions, I've
attached
the remaining delta between 2.2.x and 2.4.x, for anyone interested. All of
these
differences are to be expected.
On Tue, Aug 23, 2016 at 11:57 AM, <wr...@apache.org> wrote:
> Author: wrowe
> Date: Tue Aug 23 16:57:50 2016
> New Revision: 1757407
>
> URL: http://svn.apache.org/viewvc?rev=1757407&view=rev
> Log:
> The last proposed request handling backport to bring 2.2.x up to 2.4.x
> standards
>
> Modified:
> httpd/httpd/branches/2.2.x/STATUS
>
> Modified: httpd/httpd/branches/2.2.x/STATUS
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/
> STATUS?rev=1757407&r1=1757406&r2=1757407&view=diff
> ============================================================
> ==================
> --- httpd/httpd/branches/2.2.x/STATUS (original)
> +++ httpd/httpd/branches/2.2.x/STATUS Tue Aug 23 16:57:50 2016
> @@ -197,12 +197,30 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
> Support custom ErrorDocuments for HTTP 501 and 414 status codes.
> PR: 48357, 57167
> Submitted by: trawick, [Edward Lu <Chaosed0 gmail.com>]
> + Trunk version of patch
> http://svn.apache.org/r1392347
> http://svn.apache.org/r1635762
> Backport:
> https://raw.githubusercontent.com/wrowe/patches/master/
> backport-2.2.x-r1392347-r1635762.patch
> +1: wrowe
>
> + *) core: Limit to ten the number of tolerated empty lines between
> request.
> + Before this commit, the maximum number of empty lines was the same as
> + configured LimitRequestFields, defaulting to 100, which was way too
> much.
> + We now use a fixed/hard limit of 10 (DEFAULT_LIMIT_BLANK_LINES).
> + Exit early on ap_parse_uri failure, and ensure that proto_num and
> protocol
> + is set; this can happen with invalid CONNECT requests.
> + Submitted by: ylavic, rpluem
> + Note: http_request.c changes from this patch and follow-ups
> + r1710105, r1711902 are not applicable to the 2.2.x pipeline.
> + CHANGES is unnecessary, the regression was never released in
> 2.2.x.
> + Trunk version of patch
> + http://svn.apache.org/r1710095
> + http://svn.apache.org/r1727544
> + Backport:
> + https://raw.githubusercontent.com/wrowe/patches/master/
> backport-2.2.x-r1710095-1727544.patch
> + +1: wrowe
> +
>
> PATCHES/ISSUES THAT ARE STALLED
>
>
>
>
--- noblame 2016-08-23 11:55:52.053795974 -0500
+++ ../httpd-2.4-http/noblame 2016-08-22 19:21:10.134673139 -0500
@@ -120,11 +120,11 @@
}
} while ((len <= 0) && (--num_blank_lines >= 0));
-#ifdef AP_DEBUG_THE_REQUEST
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+ if (APLOGrtrace5(r)) {
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, r,
"Request received from client: %s",
ap_escape_logitem(r->pool, r->the_request));
-#endif
+ }
r->request_time = apr_time_now();
ll = r->the_request;
@@ -195,8 +195,8 @@
"\n<pre>\n",
ap_escape_html(r->pool, key),
"</pre>\n", NULL));
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "Request header exceeds "
- "LimitRequestFieldSize after merging: %s", key);
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00560) "Request header "
+ "exceeds LimitRequestFieldSize after merging: %s", key);
return 0;
}
@@ -265,7 +265,7 @@
"<pre>\n%.*s\n</pre>\n",
field_name_len(field_escaped),
field_escaped));
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00561)
"Request header exceeds LimitRequestFieldSize%s"
"%.*s",
*field ? ": " : "",
@@ -300,7 +300,7 @@
"<pre>\n%.*s\n</pre>\n",
field_name_len(field_escaped),
field_escaped));
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00562)
"Request header exceeds LimitRequestFieldSize "
"after folding: %.*s",
field_name_len(last_field), last_field);
@@ -329,7 +329,7 @@
apr_table_setn(r->notes, "error-notes",
"The number of request header fields "
"exceeds this server's limit.");
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00563)
"Number of request headers exceeds "
"LimitRequestFields");
return;
@@ -345,11 +345,10 @@
(int)LOG_NAME_MAX_LEN,
ap_escape_html(r->pool,
last_field)));
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00564)
"Request header field is missing ':' "
"separator: %.*s", (int)LOG_NAME_MAX_LEN,
last_field);
-
return;
}
@@ -428,9 +427,11 @@
apr_socket_t *csd;
apr_interval_time_t cur_timeout;
+
apr_pool_create(&p, conn->pool);
apr_pool_tag(p, "request");
r = apr_pcalloc(p, sizeof(request_rec));
+ AP_READ_REQUEST_ENTRY((intptr_t)r, (uintptr_t)conn);
r->pool = p;
r->connection = conn;
r->server = conn->base_server;
@@ -471,19 +472,24 @@
*/
r->used_path_info = AP_REQ_DEFAULT_PATH_INFO;
+ r->useragent_addr = conn->client_addr;
+ r->useragent_ip = conn->client_ip;
+
tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
+ ap_run_pre_read_request(r, conn);
+
/* Get the request... */
if (!read_request_line(r, tmp_bb)) {
if (r->status == HTTP_REQUEST_URI_TOO_LARGE
|| r->status == HTTP_BAD_REQUEST) {
if (r->status == HTTP_REQUEST_URI_TOO_LARGE) {
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00565)
"request failed: client's request-line exceeds LimitRequestLine (longer than %d)",
r->server->limit_req_line);
}
else if (r->method == NULL) {
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00566)
"request failed: invalid characters in URI");
}
access_status = r->status;
@@ -493,26 +499,27 @@
ap_run_log_transaction(r);
r = NULL;
apr_brigade_destroy(tmp_bb);
- return r;
+ goto traceout;
}
else if (r->status == HTTP_REQUEST_TIME_OUT) {
- ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
+ ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, NULL);
if (!r->connection->keepalives) {
ap_run_log_transaction(r);
}
apr_brigade_destroy(tmp_bb);
- return r;
+ goto traceout;
}
apr_brigade_destroy(tmp_bb);
- return NULL;
+ r = NULL;
+ goto traceout;
}
/* We may have been in keep_alive_timeout mode, so toggle back
* to the normal timeout mode as we fetch the header lines,
* as necessary.
*/
- csd = ap_get_module_config(conn->conn_config, &core_module);
+ csd = ap_get_conn_socket(conn);
apr_socket_timeout_get(csd, &cur_timeout);
if (cur_timeout != conn->base_server->timeout) {
apr_socket_timeout_set(csd, conn->base_server->timeout);
@@ -524,13 +531,13 @@
ap_get_mime_headers_core(r, tmp_bb);
if (r->status != HTTP_OK) {
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00567)
"request failed: error reading the headers");
ap_send_error_response(r, 0);
ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
ap_run_log_transaction(r);
apr_brigade_destroy(tmp_bb);
- return r;
+ goto traceout;
}
tenc = apr_table_get(r->headers_in, "Transfer-Encoding");
@@ -543,7 +550,7 @@
*/
if (!(strcasecmp(tenc, "chunked") == 0 /* fast path */
|| ap_find_last_token(r->pool, tenc, "chunked"))) {
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02539)
"client sent unknown Transfer-Encoding "
"(%s): %s", tenc, r->uri);
r->status = HTTP_BAD_REQUEST;
@@ -552,7 +559,7 @@
ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
ap_run_log_transaction(r);
apr_brigade_destroy(tmp_bb);
- return r;
+ goto traceout;
}
/* http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23
@@ -571,7 +578,7 @@
* headers! Have to dink things just to make sure the error message
* comes through...
*/
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00568)
"client sent invalid HTTP/0.9 request: HEAD %s",
r->uri);
r->header_only = 0;
@@ -580,7 +587,7 @@
ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
ap_run_log_transaction(r);
apr_brigade_destroy(tmp_bb);
- return r;
+ goto traceout;
}
}
@@ -613,7 +620,7 @@
* a Host: header, and the server MUST respond with 400 if it doesn't.
*/
access_status = HTTP_BAD_REQUEST;
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00569)
"client sent HTTP/1.1 request without hostname "
"(see RFC2616 section 14.23): %s", r->uri);
}
@@ -633,7 +640,8 @@
ap_die(access_status, r);
ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
ap_run_log_transaction(r);
- return NULL;
+ r = NULL;
+ goto traceout;
}
if (((expect = apr_table_get(r->headers_in, "Expect")) != NULL)
@@ -649,15 +657,19 @@
}
else {
r->status = HTTP_EXPECTATION_FAILED;
- ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00570)
"client sent an unrecognized expectation value of "
"Expect: %s", expect);
ap_send_error_response(r, 0);
ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
ap_run_log_transaction(r);
- return r;
+ goto traceout;
}
}
+ AP_READ_REQUEST_SUCCESS((uintptr_t)r, (char *)r->method, (char *)r->uri, (char *)r->server->defn_name, r->status);
+ return r;
+ traceout:
+ AP_READ_REQUEST_FAILURE((uintptr_t)r);
return r;
}
