From: Brian Pane Date: Tue, 28 Feb 2006 15:49:24 +0000 (+0000) Subject: Revert the refactoring of the request read code X-Git-Tag: 2.3.0~2522 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=761ef9ee176aef1a236f7f747ee9360acdcadeaf;p=thirdparty%2Fapache%2Fhttpd.git Revert the refactoring of the request read code git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@381679 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 241684fa821..391777e5d58 100644 --- a/CHANGES +++ b/CHANGES @@ -76,9 +76,6 @@ Changes with Apache 2.3.0 seconds to cache a document. [Brian Akins , Ruediger Pluem] - *) Refactored ap_read_request() to provide a foundation for - nonblocking reads of requests. [Brian Pane] - *) If a connection is aborted while waiting for a chunked line, flag the connection as errored out. [Justin Erenkrantz] diff --git a/include/ap_mmn.h b/include/ap_mmn.h index 9bc46d2a137..1819bd7e4bc 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -112,8 +112,6 @@ * 20051005.0 (2.1.8-dev) NET_TIME filter eliminated * 20051005.0 (2.3.0-dev) Bump MODULE_MAGIC_COOKIE to "AP24"! * 20051115.0 (2.3.0-dev) Added use_canonical_phys_port to core_dir_config - * 20051231.0 (2.3.0-dev) Added num_blank_lines, pending_header_line, and - * pending_header_size to request_rec * 20060110.0 (2.3.0-dev) Conversion of Authz to be provider based * addition of * removal of Satisfy, Allow, Deny, Order diff --git a/include/httpd.h b/include/httpd.h index 2b889a523e3..ec4d74d2416 100644 --- a/include/httpd.h +++ b/include/httpd.h @@ -967,16 +967,6 @@ struct request_rec { /** A flag to determine if the eos bucket has been sent yet */ int eos_sent; - /** Number of leading blank lines encountered before request header */ - int num_blank_lines; - - /** A buffered header line, used to support header folding while - * reading the request */ - char *pending_header_line; - - /** If nonzero, the number of bytes allocated to hold pending_header_line */ - apr_size_t pending_header_size; - /* Things placed at the end of the record to avoid breaking binary * compatibility. It would be nice to remember to reorder the entire * record to improve 64bit alignment the next time we need to break diff --git a/server/protocol.c b/server/protocol.c index e42e280776f..9f792ecce24 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -543,20 +543,80 @@ AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r, const char *uri) } } -static void set_the_request(request_rec *r, char *line) +static int read_request_line(request_rec *r, apr_bucket_brigade *bb) { const char *ll; const char *uri; const char *pro; + +#if 0 + conn_rec *conn = r->connection; +#endif int major = 1, minor = 0; /* Assume HTTP/1.0 if non-"HTTP" protocol */ char http[5]; apr_size_t len; - apr_socket_t *csd; + int num_blank_lines = 0; + int max_blank_lines = r->server->limit_req_fields; + + if (max_blank_lines <= 0) { + max_blank_lines = DEFAULT_LIMIT_REQUEST_FIELDS; + } + + /* Read past empty lines until we get a real request line, + * a read error, the connection closes (EOF), or we timeout. + * + * We skip empty lines because browsers have to tack a CRLF on to the end + * of POSTs to support old CERN webservers. But note that we may not + * have flushed any previous response completely to the client yet. + * We delay the flush as long as possible so that we can improve + * performance for clients that are pipelining requests. If a request + * is pipelined then we won't block during the (implicit) read() below. + * If the requests aren't pipelined, then the client is still waiting + * for the final buffer flush from us, and we will block in the implicit + * read(). B_SAFEREAD ensures that the BUFF layer flushes if it will + * have to block during a read. + */ + + do { + apr_status_t rv; + + /* insure ap_rgetline allocates memory each time thru the loop + * if there are empty lines + */ + r->the_request = NULL; + rv = ap_rgetline(&(r->the_request), (apr_size_t)(r->server->limit_req_line + 2), + &len, r, 0, bb); + + if (rv != APR_SUCCESS) { + r->request_time = apr_time_now(); + + /* ap_rgetline returns APR_ENOSPC if it fills up the + * buffer before finding the end-of-line. This is only going to + * happen if it exceeds the configured limit for a request-line. + */ + if (rv == APR_ENOSPC) { + r->status = HTTP_REQUEST_URI_TOO_LARGE; + r->proto_num = HTTP_VERSION(1,0); + r->protocol = apr_pstrdup(r->pool, "HTTP/1.0"); + } + return 0; + } + } while ((len <= 0) && (++num_blank_lines < max_blank_lines)); + + /* we've probably got something to do, ignore graceful restart requests */ r->request_time = apr_time_now(); - ll = r->the_request = line; + ll = r->the_request; r->method = ap_getword_white(r->pool, &ll); +#if 0 +/* XXX If we want to keep track of the Method, the protocol module should do + * it. That support isn't in the scoreboard yet. Hopefully next week + * sometime. rbb */ + ap_update_connection_status(AP_CHILD_THREAD_FROM_ID(conn->id), "Method", + r->method); +#endif + uri = ap_getword_white(r->pool, &ll); /* Provide quick information about the request method as soon as known */ @@ -579,6 +639,8 @@ static void set_the_request(request_rec *r, char *line) } r->protocol = apr_pstrmemdup(r->pool, pro, len); + /* XXX ap_update_connection_status(conn->id, "Protocol", r->protocol); */ + /* Avoid sscanf in the common case */ if (len == 8 && pro[0] == 'H' && pro[1] == 'T' && pro[2] == 'T' && pro[3] == 'P' @@ -588,173 +650,24 @@ static void set_the_request(request_rec *r, char *line) } else if (3 == sscanf(r->protocol, "%4s/%u.%u", http, &major, &minor) && (strcasecmp("http", http) == 0) - && (minor < HTTP_VERSION(1, 0)) ) { /* don't allow HTTP/0.1000 */ + && (minor < HTTP_VERSION(1, 0)) ) /* don't allow HTTP/0.1000 */ r->proto_num = HTTP_VERSION(major, minor); - } - else { + else r->proto_num = HTTP_VERSION(1, 0); - } - - /* 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(r->connection->conn_config, &core_module); - apr_socket_timeout_set(csd, r->connection->base_server->timeout); - - return; -} - -static apr_status_t set_mime_header(request_rec *r, char *line) -{ - char *value, *tmp_field; - - if (r->server->limit_req_fields) { - const apr_array_header_t *mime_headers = apr_table_elts(r->headers_in); - if (mime_headers->nelts >= r->server->limit_req_fields) { - apr_table_setn(r->notes, "error-notes", - "The number of request header fields " - "exceeds this server's limit."); - r->status = HTTP_BAD_REQUEST; - return APR_ENOSPC; - } - } - - if (!(value = strchr(line, ':'))) { /* Find ':' or */ - r->status = HTTP_BAD_REQUEST; /* abort bad request */ - apr_table_setn(r->notes, "error-notes", - apr_pstrcat(r->pool, - "Request header field is " - "missing ':' separator.
\n" - "
\n",
-                                   ap_escape_html(r->pool, line),
-                                   "
\n", NULL)); - return APR_EGENERAL; - } - - tmp_field = value - 1; /* last character of field-name */ - - *value++ = '\0'; /* NUL-terminate at colon */ - - while (*value == ' ' || *value == '\t') { - ++value; /* Skip to start of value */ - } - - /* Strip LWS after field-name: */ - while (tmp_field > line - && (*tmp_field == ' ' || *tmp_field == '\t')) { - *tmp_field-- = '\0'; - } - - /* Strip LWS after field-value: */ - tmp_field = strchr(value, '\0') - 1; - while (tmp_field > value - && (*tmp_field == ' ' || *tmp_field == '\t')) { - *tmp_field-- = '\0'; - } - apr_table_addn(r->headers_in, line, value); - return APR_SUCCESS; -} - -static apr_status_t process_request_line(request_rec *r, char *line, - int skip_first) -{ - if (!skip_first && (r->the_request == NULL)) { - /* This is the first line of the request */ - if ((line == NULL) || (*line == '\0')) { - /* We skip empty lines because browsers have to tack a CRLF on to the end - * of POSTs to support old CERN webservers. - */ - int max_blank_lines = r->server->limit_req_fields; - if (max_blank_lines <= 0) { - max_blank_lines = DEFAULT_LIMIT_REQUEST_FIELDS; - } - r->num_blank_lines++; - if (r->num_blank_lines < max_blank_lines) { - return APR_SUCCESS; - } - } - set_the_request(r, line); - } - else { - /* We've already read the first line of the request. This is either - * a header field or the blank line terminating the header - */ - if ((line == NULL) || (*line == '\0')) { - if (r->pending_header_line != NULL) { - apr_status_t rv = set_mime_header(r, r->pending_header_line); - if (rv != APR_SUCCESS) { - return rv; - } - r->pending_header_line = NULL; - } - if (r->status == HTTP_REQUEST_TIME_OUT) { - apr_table_compress(r->headers_in, APR_OVERLAP_TABLES_MERGE); - r->status = HTTP_OK; - } - } - else { - if ((*line == ' ') || (*line == '\t')) { - /* This line is a continuation of the previous one */ - if (r->pending_header_line == NULL) { - r->pending_header_line = line; - r->pending_header_size = 0; - } - else { - apr_size_t pending_len = strlen(r->pending_header_line); - apr_size_t fold_len = strlen(line); - if ((r->server->limit_req_fieldsize > 0) - && (pending_len + fold_len > - (apr_size_t) r->server->limit_req_fieldsize)) { - /* CVE-2004-0942 */ - r->status = HTTP_BAD_REQUEST; - return APR_ENOSPC; - } - if (pending_len + fold_len + 1 > r->pending_header_size) { - /* Allocate a new buffer big enough to hold the - * concatenated lines - */ - apr_size_t new_size = r->pending_header_size; - char *new_buf; - if (new_size == 0) { - new_size = pending_len + fold_len + 1; - } - else { - do { - new_size *= 2; - } while (new_size < pending_len + fold_len + 1); - } - new_buf = (char *)apr_palloc(r->pool, new_size); - memcpy(new_buf, r->pending_header_line, pending_len); - r->pending_header_line = new_buf; - r->pending_header_size = new_size; - } - memcpy(r->pending_header_line + pending_len, line, - fold_len + 1); - } - } - else { - /* Set aside this line in case the next line is a continuation - */ - if (r->pending_header_line != NULL) { - apr_status_t rv = set_mime_header(r, r->pending_header_line); - if (rv != APR_SUCCESS) { - return rv; - } - } - r->pending_header_line = line; - r->pending_header_size = 0; - } - } - } - return APR_SUCCESS; + return 1; } AP_DECLARE(void) ap_get_mime_headers_core(request_rec *r, apr_bucket_brigade *bb) { + char *last_field = NULL; + apr_size_t last_len = 0; + apr_size_t alloc_len = 0; char *field; + char *value; apr_size_t len; + int fields_read = 0; + char *tmp_field; /* * Read header lines until we get the empty separator line, a read error, @@ -762,6 +675,7 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_rec *r, apr_bucket_brigade *bb */ while(1) { apr_status_t rv; + int folded = 0; field = NULL; rv = ap_rgetline(&field, r->server->limit_req_fieldsize + 2, @@ -787,14 +701,120 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_rec *r, apr_bucket_brigade *bb } return; } - rv = process_request_line(r, field, 1); - if (rv != APR_SUCCESS) { - return; + + if (last_field != NULL) { + if ((len > 0) && ((*field == '\t') || *field == ' ')) { + /* This line is a continuation of the preceding line(s), + * so append it to the line that we've set aside. + * Note: this uses a power-of-two allocator to avoid + * doing O(n) allocs and using O(n^2) space for + * continuations that span many many lines. + */ + apr_size_t fold_len = last_len + len + 1; /* trailing null */ + + if (fold_len >= (apr_size_t)(r->server->limit_req_fieldsize)) { + r->status = HTTP_BAD_REQUEST; + /* report what we have accumulated so far before the + * overflow (last_field) as the field with the problem + */ + apr_table_setn(r->notes, "error-notes", + apr_pstrcat(r->pool, + "Size of a request header field " + "after folding " + "exceeds server limit.
\n" + "
\n",
+                                               ap_escape_html(r->pool, last_field),
+                                               "
\n", NULL)); + return; + } + + if (fold_len > alloc_len) { + char *fold_buf; + alloc_len += alloc_len; + if (fold_len > alloc_len) { + alloc_len = fold_len; + } + fold_buf = (char *)apr_palloc(r->pool, alloc_len); + memcpy(fold_buf, last_field, last_len); + last_field = fold_buf; + } + memcpy(last_field + last_len, field, len +1); /* +1 for nul */ + last_len += len; + folded = 1; + } + else /* not a continuation line */ { + + if (r->server->limit_req_fields + && (++fields_read > r->server->limit_req_fields)) { + r->status = HTTP_BAD_REQUEST; + apr_table_setn(r->notes, "error-notes", + "The number of request header fields " + "exceeds this server's limit."); + return; + } + + if (!(value = strchr(last_field, ':'))) { /* Find ':' or */ + r->status = HTTP_BAD_REQUEST; /* abort bad request */ + apr_table_setn(r->notes, "error-notes", + apr_pstrcat(r->pool, + "Request header field is " + "missing ':' separator.
\n" + "
\n",
+                                               ap_escape_html(r->pool,
+                                                              last_field),
+                                               "
\n", NULL)); + return; + } + + tmp_field = value - 1; /* last character of field-name */ + + *value++ = '\0'; /* NUL-terminate at colon */ + + while (*value == ' ' || *value == '\t') { + ++value; /* Skip to start of value */ + } + + /* Strip LWS after field-name: */ + while (tmp_field > last_field + && (*tmp_field == ' ' || *tmp_field == '\t')) { + *tmp_field-- = '\0'; + } + + /* Strip LWS after field-value: */ + tmp_field = last_field + last_len - 1; + while (tmp_field > value + && (*tmp_field == ' ' || *tmp_field == '\t')) { + *tmp_field-- = '\0'; + } + + apr_table_addn(r->headers_in, last_field, value); + + /* reset the alloc_len so that we'll allocate a new + * buffer if we have to do any more folding: we can't + * use the previous buffer because its contents are + * now part of r->headers_in + */ + alloc_len = 0; + + } /* end if current line is not a continuation starting with tab */ } - if ((field == NULL) || (*field == '\0')) { - return; + + /* Found a blank line, stop. */ + if (len == 0) { + break; + } + + /* Keep track of this line so that we can parse it on + * the next loop iteration. (In the folded case, last_field + * has been updated already.) + */ + if (!folded) { + last_field = field; + last_len = len; } } + + apr_table_compress(r->headers_in, APR_OVERLAP_TABLES_MERGE); } AP_DECLARE(void) ap_get_mime_headers(request_rec *r) @@ -805,11 +825,16 @@ AP_DECLARE(void) ap_get_mime_headers(request_rec *r) apr_brigade_destroy(tmp_bb); } -static request_rec *init_request(conn_rec *conn) +request_rec *ap_read_request(conn_rec *conn) { request_rec *r; apr_pool_t *p; - + const char *expect; + int access_status; + apr_bucket_brigade *tmp_bb; + 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)); @@ -851,14 +876,77 @@ static request_rec *init_request(conn_rec *conn) */ r->used_path_info = AP_REQ_DEFAULT_PATH_INFO; - return r; -} + tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc); -static request_rec *request_post_read(request_rec *r, conn_rec *conn) -{ - apr_socket_t *csd; - const char *expect; - int access_status; + /* Get the request... */ + if (!read_request_line(r, tmp_bb)) { + if (r->status == HTTP_REQUEST_URI_TOO_LARGE) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "request failed: URI too long (longer than %d)", r->server->limit_req_line); + 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; + } + + apr_brigade_destroy(tmp_bb); + return NULL; + } + + /* 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); + apr_socket_timeout_get(csd, &cur_timeout); + if (cur_timeout != conn->base_server->timeout) { + apr_socket_timeout_set(csd, conn->base_server->timeout); + cur_timeout = conn->base_server->timeout; + } + + if (!r->assbackwards) { + ap_get_mime_headers_core(r, tmp_bb); + if (r->status != HTTP_REQUEST_TIME_OUT) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "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; + } + + if (apr_table_get(r->headers_in, "Transfer-Encoding") + && apr_table_get(r->headers_in, "Content-Length")) { + /* 2616 section 4.4, point 3: "if both Transfer-Encoding + * and Content-Length are received, the latter MUST be + * ignored"; so unset it here to prevent any confusion + * later. */ + apr_table_unset(r->headers_in, "Content-Length"); + } + } + else { + if (r->header_only) { + /* + * Client asked for headers only with HTTP/0.9, which doesn't send + * headers! Have to dink things just to make sure the error message + * comes through... + */ + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "client sent invalid HTTP/0.9 request: HEAD %s", + r->uri); + r->header_only = 0; + r->status = HTTP_BAD_REQUEST; + 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; + } + } + + apr_brigade_destroy(tmp_bb); r->status = HTTP_OK; /* Until further notice. */ @@ -870,17 +958,9 @@ static request_rec *request_post_read(request_rec *r, conn_rec *conn) /* Toggle to the Host:-based vhost's timeout mode to fetch the * request body and send the response body, if needed. */ - csd = ap_get_module_config(conn->conn_config, &core_module); - apr_socket_timeout_set(csd, r->server->timeout); - - if (apr_table_get(r->headers_in, "Transfer-Encoding") && - apr_table_get(r->headers_in, "Content-Length")) { - /* 2616 section 4.4, point 3: "if both Transfer-Encoding - * and Content-Length are received, the latter MUST be - * ignored"; so unset it here to prevent any confusion - * later. - */ - apr_table_unset(r->headers_in, "Content-Length"); + if (cur_timeout != r->server->timeout) { + apr_socket_timeout_set(csd, r->server->timeout); + cur_timeout = r->server->timeout; } /* we may have switched to another server */ @@ -952,101 +1032,6 @@ static request_rec *request_post_read(request_rec *r, conn_rec *conn) return r; } -static apr_status_t read_partial_request(request_rec *r) { - apr_bucket_brigade *tmp_bb; - apr_status_t rv = APR_SUCCESS; - - /* Read and process lines of the request until we - * encounter a complete request header, an error, or EAGAIN - */ - tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc); - while (r->status == HTTP_REQUEST_TIME_OUT) { - char *line = NULL; - apr_size_t line_length; - apr_size_t length_limit; - int first_line = (r->the_request == NULL); - if (first_line) { - length_limit = r->server->limit_req_line; - } - else { - if (r->assbackwards) { - r->status = HTTP_OK; - break; - } - length_limit = r->server->limit_req_fieldsize; - } - /* TODO: use a nonblocking call in place of ap_rgetline */ - rv = ap_rgetline(&line, length_limit + 2, - &line_length, r, 0, tmp_bb); - if (rv == APR_SUCCESS) { - rv = process_request_line(r, line, 0); - } - if (rv != APR_SUCCESS) { - r->request_time = apr_time_now(); - /* ap_rgetline returns APR_ENOSPC if it fills up the - * buffer before finding the end-of-line. This is only going to - * happen if it exceeds the configured limit for a request-line. - */ - if (rv == APR_ENOSPC) { - if (first_line) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, - "request failed: URI too long (longer than %d)", - r->server->limit_req_line); - r->status = HTTP_REQUEST_URI_TOO_LARGE; - r->proto_num = HTTP_VERSION(1,0); - r->protocol = apr_pstrdup(r->pool, "HTTP/1.0"); - } - else { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, - "request failed: header line too long " - "(longer than %d)", - r->server->limit_req_fieldsize); - r->status = HTTP_BAD_REQUEST; - } - } - break; - } - } - apr_brigade_destroy(tmp_bb); - return rv; -} - -request_rec *ap_read_request(conn_rec *conn) -{ - request_rec *r; - apr_status_t rv; - - r = init_request(conn); - - rv = read_partial_request(r); - /* TODO poll if EAGAIN */ - if (r->status != HTTP_OK) { - ap_send_error_response(r, 0); - ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r); - ap_run_log_transaction(r); - return NULL; - } - - if (r->assbackwards && r->header_only) { - /* - * Client asked for headers only with HTTP/0.9, which doesn't send - * headers! Have to dink things just to make sure the error message - * comes through... - */ - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, - "client sent invalid HTTP/0.9 request: HEAD %s", - r->uri); - r->header_only = 0; - r->status = HTTP_BAD_REQUEST; - ap_send_error_response(r, 0); - ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r); - ap_run_log_transaction(r); - return r; - } - - return request_post_read(r, conn); -} - /* if a request with a body creates a subrequest, clone the original request's * input headers minus any headers pertaining to the body which has already * been read. out-of-line helper function for ap_set_sub_req_protocol.