You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@httpd.apache.org by GitBox <gi...@apache.org> on 2022/04/01 11:14:05 UTC
[GitHub] [httpd] icing opened a new pull request #306: H1 chunked input
icing opened a new pull request #306:
URL: https://github.com/apache/httpd/pull/306
This PR is the first in introducing the changes in #291 in smaller, more easily digestible increments.
What it does:
* adds new meta bucket types `REQUEST`, `RESPONSE` and `HEADERS` to the API.
* adds a new method for setting standard response headers `Date` and `Server`
* adds helper methods for formatting parts of HTTP/1.x, like headers and end chunks for use in non-core parts of the server, e.g. mod_proxy
* *splits* the `HTTP_IN` filter into a "generic HTTP" and "specific HTTP/1.x" filter. The latter one named `HTTP1_BODY_IN`.
* Uses `HTTP1_BODY_IN` only for requests with HTTP version <= 1.1
* Removes the chunked input simulation from `mod_http2`
* adds `body_indeterminate` flag to `request_rec` that indicates that a request body *may* be present and needs to be read/discarded. This replaces logic that thinks without `Content-Length` and `Transfer-Encoding`, no request body can exist.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] icing commented on a change in pull request #306: H1 chunked input
Posted by GitBox <gi...@apache.org>.
icing commented on a change in pull request #306:
URL: https://github.com/apache/httpd/pull/306#discussion_r840483641
##########
File path: include/http_protocol.h
##########
@@ -67,6 +67,18 @@ AP_DECLARE(request_rec *) ap_create_request(conn_rec *c);
*/
request_rec *ap_read_request(conn_rec *c);
+/**
+ * Assign the method, uri and protocol to the request.
+ * @param r The current request
+ * @param method the HTTP method
+ * @param uri the request uri
+ * @param protocol the request protocol
+ * @return 1 on success, 0 on failure
+ */
+AP_DECLARE(int) ap_assign_request(request_rec *r,
+ const char *method, const char *uri,
+ const char *protocol);
Review comment:
This is exposed now, but not yet used or even implemented. It will come in a later PR. I just wanted the API changes to be complete.
##########
File path: include/http_protocol.h
##########
@@ -1027,6 +1039,252 @@ AP_DECLARE(apr_bucket *) ap_bucket_error_create(int error, const char *buf,
apr_pool_t *p,
apr_bucket_alloc_t *list);
+/** @see ap_bucket_type_request */
+typedef struct ap_bucket_request ap_bucket_request;
+
+/**
+ * @struct ap_bucket_request
+ * @brief A bucket referring to a HTTP request
+ *
+ */
+struct ap_bucket_request {
+ /** Number of buckets using this memory */
+ apr_bucket_refcount refcount;
+ apr_pool_t *pool; /* pool that holds the contents, not for modification */
+ const char *method; /* request method */
+ const char *uri; /* request uri */
+ const char *protocol; /* request protocol */
+ apr_table_t *headers; /* request headers */
+};
Review comment:
All HTTP fields that define the meta data for a request. Will be used in future changes to pass a HTTP request into general processing, irregardless of the format it arrived at the server.
##########
File path: include/httpd.h
##########
@@ -1144,6 +1144,14 @@ struct request_rec {
* the elements of this field.
*/
ap_request_bnotes_t bnotes;
+ /** Indicates that the request has a body of unknown length and
+ * protocol handlers need to read it, even if only to discard the
+ * data. In HTTP/1.1 this is set on chunked transfer encodings, but
+ * newer HTTP versions can transfer such bodies by other means. The
+ * absence of a "Transfer-Encoding" header is no longer sufficient
+ * to conclude that no body is there.
+ */
+ int body_indeterminate;
Review comment:
!= 0 iff a body of indeterminate length is present. In HTTP/1,.1 this is set on `Transfer-Encoding`. In HTTP/2 this is set when the remote side has not yet indicated an EOS on a request.
##########
File path: modules/http/http_filters.c
##########
@@ -254,21 +254,28 @@ static apr_status_t parse_chunk_size(http_ctx_t *ctx, const char *buffer,
}
static apr_status_t read_chunked_trailers(http_ctx_t *ctx, ap_filter_t *f,
- apr_bucket_brigade *b, int merge)
+ apr_bucket_brigade *b)
Review comment:
Read HTTP/1.1 end chunk and append it as a HEADERS bucket to the brigade (if indeed trailers were present).
##########
File path: modules/http/http_protocol.c
##########
@@ -1485,3 +1485,108 @@ AP_DECLARE(void) ap_clear_method_list(ap_method_list_t *l)
l->method_list->nelts = 0;
}
+/* Send a request's HTTP response headers to the client.
+ */
+AP_DECLARE(apr_status_t) ap_http1_append_headers(apr_bucket_brigade *bb,
+ request_rec *r,
+ apr_table_t *headers)
+{
+ const apr_array_header_t *elts;
+ const apr_table_entry_t *t_elt;
+ const apr_table_entry_t *t_end;
+ struct iovec *vec;
+ struct iovec *vec_next;
+
+ elts = apr_table_elts(headers);
+ if (elts->nelts == 0) {
+ return APR_SUCCESS;
+ }
+ t_elt = (const apr_table_entry_t *)(elts->elts);
+ t_end = t_elt + elts->nelts;
+ vec = (struct iovec *)apr_palloc(r->pool, 4 * elts->nelts *
+ sizeof(struct iovec));
+ vec_next = vec;
+
+ /* For each field, generate
+ * name ": " value CRLF
+ */
+ do {
+ if (t_elt->key && t_elt->val) {
+ vec_next->iov_base = (void*)(t_elt->key);
+ vec_next->iov_len = strlen(t_elt->key);
+ vec_next++;
+ vec_next->iov_base = ": ";
+ vec_next->iov_len = sizeof(": ") - 1;
+ vec_next++;
+ vec_next->iov_base = (void*)(t_elt->val);
+ vec_next->iov_len = strlen(t_elt->val);
+ vec_next++;
+ vec_next->iov_base = CRLF;
+ vec_next->iov_len = sizeof(CRLF) - 1;
+ vec_next++;
+ }
+ t_elt++;
+ } while (t_elt < t_end);
+
+ if (APLOGrtrace4(r)) {
+ t_elt = (const apr_table_entry_t *)(elts->elts);
+ do {
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, r, " %s: %s",
+ t_elt->key, t_elt->val);
+ t_elt++;
+ } while (t_elt < t_end);
+ }
+
+#if APR_CHARSET_EBCDIC
+ {
+ apr_size_t len;
+ char *tmp = apr_pstrcatv(r->pool, vec, vec_next - vec, &len);
+ ap_xlate_proto_to_ascii(tmp, len);
+ return apr_brigade_write(bb, NULL, NULL, tmp, len);
+ }
+#else
+ return apr_brigade_writev(bb, NULL, NULL, vec, vec_next - vec);
+#endif
+}
+
+AP_DECLARE(apr_status_t) ap_http1_terminate_header(apr_bucket_brigade *bb)
+{
+ char crlf[] = CRLF;
+ apr_size_t buflen;
+
+ buflen = strlen(crlf);
+ ap_xlate_proto_to_ascii(crlf, buflen);
+ return apr_brigade_write(bb, NULL, NULL, crlf, buflen);
+}
+
+AP_DECLARE(void) ap_http1_add_end_chunk(apr_bucket_brigade *b,
+ apr_bucket *eos,
+ request_rec *r,
+ apr_table_t *trailers)
+{
+ if (!trailers || apr_is_empty_table(trailers)) {
+ apr_bucket *e;
+
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
+ "append empty end chunk");
+ e = apr_bucket_immortal_create(ZERO_ASCII CRLF_ASCII
+ CRLF_ASCII, 5, b->bucket_alloc);
+ if (eos) {
+ APR_BUCKET_INSERT_BEFORE(eos, e);
+ }
+ else {
+ APR_BRIGADE_INSERT_TAIL(b, e);
+ }
+ }
+ else {
+ apr_bucket_brigade *tmp;
+
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
+ "append end chunk with trailers");
+ tmp = eos? apr_brigade_split_ex(b, eos, NULL) : NULL;
+ apr_brigade_write(b, NULL, NULL, ZERO_ASCII CRLF_ASCII, 3);
+ ap_http1_append_headers(b, r, trailers);
+ ap_http1_terminate_header(b);
+ if (tmp) APR_BRIGADE_CONCAT(b, tmp);
+ }
+}
Review comment:
Implementation of the API visible helper functions for HTTP/1.x formatting.
##########
File path: modules/http2/h2_c2.c
##########
@@ -478,6 +478,16 @@ static apr_status_t c2_process(h2_conn_ctx_t *conn_ctx, conn_rec *c)
/* the request_rec->server carries the timeout value that applies */
h2_conn_ctx_set_timeout(conn_ctx, r->server->timeout);
+ /* We only handle this one request on the connection and tell everyone
+ * that there is no need to keep it "clean" if something fails. Also,
+ * this prevents mod_reqtimeout from doing funny business with monitoring
+ * keepalive timeouts.
+ */
+ r->connection->keepalive = AP_CONN_CLOSE;
+
+ if (conn_ctx->beam_in && !apr_table_get(r->headers_in, "Content-Length")) {
+ r->body_indeterminate = 1;
+ }
Review comment:
Using the new request_rec field in HTTP/2.
##########
File path: server/protocol.c
##########
@@ -2385,6 +2406,38 @@ static int send_header(void *data, const char *key, const char *val)
}
#endif
+AP_DECLARE(void) ap_set_std_response_headers(request_rec *r)
+{
+ const char *server = NULL, *date;
+ char *s;
+
+ /* Before generating a response, we make sure that `Date` and `Server`
+ * headers are present. When proxying requests, we preserver existing
+ * values and replace them otherwise.
+ */
+ if (r->proxyreq != PROXYREQ_NONE) {
+ date = apr_table_get(r->headers_out, "Date");
+ if (!date) {
+ s = apr_palloc(r->pool, APR_RFC822_DATE_LEN);
+ ap_recent_rfc822_date(s, r->request_time);
+ date = s;
+ }
+ server = apr_table_get(r->headers_out, "Server");
+ }
+ else {
+ s = apr_palloc(r->pool, APR_RFC822_DATE_LEN);
+ ap_recent_rfc822_date(s, r->request_time);
+ date = s;
+ }
+
+ apr_table_setn(r->headers_out, "Date", date);
+
+ if (!server)
+ server = ap_get_server_banner();
+ if (server && *server)
+ apr_table_setn(r->headers_out, "Server", server);
+}
+
Review comment:
This code is a copy of our current HTTP/1.x response formatting. It will in future PRs be used in general HTTP response processing. This is then the one place where standard response headers are added (or preserved in proxy responses).
##########
File path: include/http_protocol.h
##########
@@ -1027,6 +1039,252 @@ AP_DECLARE(apr_bucket *) ap_bucket_error_create(int error, const char *buf,
apr_pool_t *p,
apr_bucket_alloc_t *list);
+/** @see ap_bucket_type_request */
+typedef struct ap_bucket_request ap_bucket_request;
+
+/**
+ * @struct ap_bucket_request
+ * @brief A bucket referring to a HTTP request
+ *
+ */
+struct ap_bucket_request {
+ /** Number of buckets using this memory */
+ apr_bucket_refcount refcount;
+ apr_pool_t *pool; /* pool that holds the contents, not for modification */
+ const char *method; /* request method */
+ const char *uri; /* request uri */
+ const char *protocol; /* request protocol */
+ apr_table_t *headers; /* request headers */
+};
+
+/** @see ap_bucket_type_request */
+AP_DECLARE_DATA extern const apr_bucket_type_t ap_bucket_type_request;
+
+/**
+ * Determine if a bucket is a request bucket
+ * @param e The bucket to inspect
+ * @return true or false
+ */
+#define AP_BUCKET_IS_REQUEST(e) (e->type == &ap_bucket_type_request)
+
+/**
+ * Make the bucket passed in a request bucket
+ * Copies all parameters to the given pool.
+ * @param b The bucket to make into a request bucket
+ * @param method the HTTP method
+ * @param uri the uri requested
+ * @param protocol the protocol requested
+ * @param headers the table of response headers.
+ * @param p A pool to allocate out of.
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_request_make(
+ apr_bucket *b,
+ const char *method,
+ const char *uri,
+ const char *protocol,
+ apr_table_t *headers,
+ apr_pool_t *p);
+
+/**
+ * Make the bucket passed in a request bucket
+ * Uses all paramters without copying.
+ * @param b The bucket to make into a request bucket
+ * @param method the HTTP method
+ * @param uri the uri requested
+ * @param protocol the protocol requested
+ * @param headers the table of response headers.
+ * @param p A pool to allocate out of.
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_request_maken(
+ apr_bucket *b,
+ const char *method,
+ const char *uri,
+ const char *protocol,
+ apr_table_t *headers,
+ apr_pool_t *p);
+
+/**
+ * Create a bucket referring to a HTTP request.
+ * Copies all parameters to the given pool.
+ * @param method the HTTP method
+ * @param uri the uri requested
+ * @param protocol the protocol requested
+ * @param headers the table of response headers.
+ * @param p A pool to allocate the error string out of.
+ * @param list The bucket allocator from which to allocate the bucket
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_request_create(
+ const char *method,
+ const char *uri,
+ const char *protocol,
+ apr_table_t *headers,
+ apr_pool_t *p,
+ apr_bucket_alloc_t *list);
+
+/**
+ * Create a bucket referring to a HTTP request.
+ * Uses all paramters without copying.
+ * @param method the HTTP method
+ * @param uri the uri requested
+ * @param protocol the protocol requested
+ * @param headers the HTTP response headers.
+ * @param p A pool to allocate the error string out of.
+ * @param list The bucket allocator from which to allocate the bucket
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_request_createn(
+ const char *method,
+ const char *uri,
+ const char *protocol,
+ apr_table_t *headers,
+ apr_pool_t *p,
+ apr_bucket_alloc_t *list);
+
+/**
+ * Clone a request bucket into another pool/bucket_alloc that may
+ * have a separate lifetime than the source bucket/pool.
+ * @param source the request bucket to clone
+ * @param p A pool to allocate the data out of.
+ * @param list The bucket allocator from which to allocate the bucket
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_request_clone(apr_bucket *source,
+ apr_pool_t *p,
+ apr_bucket_alloc_t *list);
+
+/** @see ap_bucket_type_response */
+typedef struct ap_bucket_response ap_bucket_response;
+
+/**
+ * @struct ap_bucket_response
+ * @brief A bucket referring to a HTTP response
+ *
+ */
+struct ap_bucket_response {
+ /** Number of buckets using this memory */
+ apr_bucket_refcount refcount;
+ apr_pool_t *pool; /* pool that holds the contents, not for modification */
+ int status; /* The status code */
+ const char *reason; /* The optional HTTP reason for the status. */
+ apr_table_t *headers; /* The response headers */
+ apr_table_t *notes; /* internal notes about the response */
+};
+
+/** @see ap_bucket_type_headers */
+AP_DECLARE_DATA extern const apr_bucket_type_t ap_bucket_type_response;
+
+/**
+ * Determine if a bucket is a response bucket
+ * @param e The bucket to inspect
+ * @return true or false
+ */
+#define AP_BUCKET_IS_RESPONSE(e) (e->type == &ap_bucket_type_response)
+
+/**
+ * Make the bucket passed in a response bucket
+ * @param b The bucket to make into a response bucket
+ * @param status The HTTP status code of the response.
+ * @param reason textual description of status, can be NULL.
+ * @param headers the table of response headers.
+ * @param notes internal notes on the response
+ * @param p A pool to allocate out of.
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_response_make(apr_bucket *b, int status,
+ const char *reason, apr_table_t *headers,
+ apr_table_t *notes, apr_pool_t *p);
+
+/**
+ * Create a bucket referring to a HTTP response.
+ * @param status The HTTP status code.
+ * @param reason textual description of status, can be NULL.
+ * @param headers the HTTP response headers.
+ * @param notes internal notes on the response
+ * @param p A pool to allocate the error string out of.
+ * @param list The bucket allocator from which to allocate the bucket
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_response_create(
+ int status, const char *reason,
+ apr_table_t *headers,
+ apr_table_t *notes,
+ apr_pool_t *p,
+ apr_bucket_alloc_t *list);
+
+/**
+ * Clone a RESPONSE bucket into another pool/bucket_alloc that may
+ * have a separate lifetime than the source bucket/pool.
+ * @param source the response bucket to clone
+ * @param p A pool to allocate the data out of.
+ * @param list The bucket allocator from which to allocate the bucket
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_response_clone(apr_bucket *source,
+ apr_pool_t *p,
+ apr_bucket_alloc_t *list);
+
+/** @see ap_bucket_type_headers */
+typedef struct ap_bucket_headers ap_bucket_headers;
+
+/**
+ * @struct ap_bucket_headers
+ * @brief A bucket referring to an HTTP header set
+ *
+ */
+struct ap_bucket_headers {
+ /** Number of buckets using this memory */
+ apr_bucket_refcount refcount;
+ apr_pool_t *pool; /* pool that holds the contents, not for modification */
+ apr_table_t *headers; /* The headers */
+
+};
Review comment:
Send a bunch of headers up/down the filter chain. This is used to pass HTTP trailers.
##########
File path: include/http_protocol.h
##########
@@ -1047,13 +1305,47 @@ AP_DECLARE(void) ap_set_sub_req_protocol(request_rec *rnew, const request_rec *r
*/
AP_DECLARE(void) ap_finalize_sub_req_protocol(request_rec *sub_r);
+/**
+ * Set standard response headers, such as `Date` and `Server`
+ * in r->headers_out. Takes care of precedence of existing
+ * values from proxied requests.
+ */
+AP_DECLARE(void) ap_set_std_response_headers(request_rec *r);
+
/**
* Send an interim (HTTP 1xx) response immediately.
* @param r The request
* @param send_headers Whether to send&clear headers in r->headers_out
*/
AP_DECLARE(void) ap_send_interim_response(request_rec *r, int send_headers);
+/**
+ * Append the headers in HTTP/1.1 format to the brigade.
+ * @param b the brigade to append to
+ * @param r the reqeust this is done for (pool and logging)
+ * @param headers the headers to append
+ */
+AP_DECLARE(apr_status_t) ap_http1_append_headers(apr_bucket_brigade *b,
+ request_rec *r,
+ apr_table_t *headers);
+
+/**
+ * Append the HTTP/1.1 header termination (empty CRLF) to the brigade.
+ * @param b the brigade to append to
+ */
+AP_DECLARE(apr_status_t) ap_http1_terminate_header(apr_bucket_brigade *b);
+
+/**
+ * Insert/Append the last chunk in a HTTP/1.1 Transfer-Encoding chunked.
+ * @param b the brigade to add the chunk to
+ * @param eos the bucket before to add or NULL for insert at tail
+ * @param r the request handled
+ * @param trailers table of trailers or NULL
+ */
+AP_DECLARE(void) ap_http1_add_end_chunk(apr_bucket_brigade *b,
+ apr_bucket *eos,
+ request_rec *r,
+ apr_table_t *trailers);
Review comment:
These methods are used in core and will, in future PRs, also used in mod_proxy to avoid code duplication.
##########
File path: include/mod_core.h
##########
@@ -52,6 +53,10 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
ap_input_mode_t mode, apr_read_type_e block,
apr_off_t readbytes);
+apr_status_t ap_http1_body_in_filter(ap_filter_t *f, apr_bucket_brigade *b,
+ ap_input_mode_t mode, apr_read_type_e block,
+ apr_off_t readbytes);
+
Review comment:
The new filter containing the HTTP/1.x specific parts of `HTTP_IN`.
##########
File path: include/http_protocol.h
##########
@@ -1027,6 +1039,252 @@ AP_DECLARE(apr_bucket *) ap_bucket_error_create(int error, const char *buf,
apr_pool_t *p,
apr_bucket_alloc_t *list);
+/** @see ap_bucket_type_request */
+typedef struct ap_bucket_request ap_bucket_request;
+
+/**
+ * @struct ap_bucket_request
+ * @brief A bucket referring to a HTTP request
+ *
+ */
+struct ap_bucket_request {
+ /** Number of buckets using this memory */
+ apr_bucket_refcount refcount;
+ apr_pool_t *pool; /* pool that holds the contents, not for modification */
+ const char *method; /* request method */
+ const char *uri; /* request uri */
+ const char *protocol; /* request protocol */
+ apr_table_t *headers; /* request headers */
+};
+
+/** @see ap_bucket_type_request */
+AP_DECLARE_DATA extern const apr_bucket_type_t ap_bucket_type_request;
+
+/**
+ * Determine if a bucket is a request bucket
+ * @param e The bucket to inspect
+ * @return true or false
+ */
+#define AP_BUCKET_IS_REQUEST(e) (e->type == &ap_bucket_type_request)
+
+/**
+ * Make the bucket passed in a request bucket
+ * Copies all parameters to the given pool.
+ * @param b The bucket to make into a request bucket
+ * @param method the HTTP method
+ * @param uri the uri requested
+ * @param protocol the protocol requested
+ * @param headers the table of response headers.
+ * @param p A pool to allocate out of.
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_request_make(
+ apr_bucket *b,
+ const char *method,
+ const char *uri,
+ const char *protocol,
+ apr_table_t *headers,
+ apr_pool_t *p);
+
+/**
+ * Make the bucket passed in a request bucket
+ * Uses all paramters without copying.
+ * @param b The bucket to make into a request bucket
+ * @param method the HTTP method
+ * @param uri the uri requested
+ * @param protocol the protocol requested
+ * @param headers the table of response headers.
+ * @param p A pool to allocate out of.
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_request_maken(
+ apr_bucket *b,
+ const char *method,
+ const char *uri,
+ const char *protocol,
+ apr_table_t *headers,
+ apr_pool_t *p);
+
+/**
+ * Create a bucket referring to a HTTP request.
+ * Copies all parameters to the given pool.
+ * @param method the HTTP method
+ * @param uri the uri requested
+ * @param protocol the protocol requested
+ * @param headers the table of response headers.
+ * @param p A pool to allocate the error string out of.
+ * @param list The bucket allocator from which to allocate the bucket
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_request_create(
+ const char *method,
+ const char *uri,
+ const char *protocol,
+ apr_table_t *headers,
+ apr_pool_t *p,
+ apr_bucket_alloc_t *list);
+
+/**
+ * Create a bucket referring to a HTTP request.
+ * Uses all paramters without copying.
+ * @param method the HTTP method
+ * @param uri the uri requested
+ * @param protocol the protocol requested
+ * @param headers the HTTP response headers.
+ * @param p A pool to allocate the error string out of.
+ * @param list The bucket allocator from which to allocate the bucket
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_request_createn(
+ const char *method,
+ const char *uri,
+ const char *protocol,
+ apr_table_t *headers,
+ apr_pool_t *p,
+ apr_bucket_alloc_t *list);
+
+/**
+ * Clone a request bucket into another pool/bucket_alloc that may
+ * have a separate lifetime than the source bucket/pool.
+ * @param source the request bucket to clone
+ * @param p A pool to allocate the data out of.
+ * @param list The bucket allocator from which to allocate the bucket
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_request_clone(apr_bucket *source,
+ apr_pool_t *p,
+ apr_bucket_alloc_t *list);
+
+/** @see ap_bucket_type_response */
+typedef struct ap_bucket_response ap_bucket_response;
+
+/**
+ * @struct ap_bucket_response
+ * @brief A bucket referring to a HTTP response
+ *
+ */
+struct ap_bucket_response {
+ /** Number of buckets using this memory */
+ apr_bucket_refcount refcount;
+ apr_pool_t *pool; /* pool that holds the contents, not for modification */
+ int status; /* The status code */
+ const char *reason; /* The optional HTTP reason for the status. */
+ apr_table_t *headers; /* The response headers */
+ apr_table_t *notes; /* internal notes about the response */
+};
Review comment:
All meta data of a HTTP response. In later changes, the general HTTP processing will send these down the output filter chain and version specific filters will do the formatting.
##########
File path: include/http_protocol.h
##########
@@ -1047,13 +1305,47 @@ AP_DECLARE(void) ap_set_sub_req_protocol(request_rec *rnew, const request_rec *r
*/
AP_DECLARE(void) ap_finalize_sub_req_protocol(request_rec *sub_r);
+/**
+ * Set standard response headers, such as `Date` and `Server`
+ * in r->headers_out. Takes care of precedence of existing
+ * values from proxied requests.
+ */
+AP_DECLARE(void) ap_set_std_response_headers(request_rec *r);
+
Review comment:
Where ever responses are created, we need some standard additions like `Date` and `Server`, observing proper logic for proxied requests.
##########
File path: modules/http/chunk_filter.c
##########
@@ -65,28 +76,43 @@ apr_status_t ap_http_chunk_filter(ap_filter_t *f, apr_bucket_brigade *b)
e != APR_BRIGADE_SENTINEL(b);
e = APR_BUCKET_NEXT(e))
{
- if (APR_BUCKET_IS_EOS(e)) {
- /* there shouldn't be anything after the eos */
- ap_remove_output_filter(f);
- eos = e;
- break;
- }
- if (AP_BUCKET_IS_ERROR(e) &&
- (((ap_bucket_error *)(e->data))->status == HTTP_BAD_GATEWAY ||
- ((ap_bucket_error *)(e->data))->status == HTTP_GATEWAY_TIME_OUT)) {
- /*
- * We had a broken backend. Memorize this in the filter
- * context.
- */
- f->ctx = &bad_gateway_seen;
- continue;
- }
- if (APR_BUCKET_IS_FLUSH(e)) {
- flush = e;
- if (e != APR_BRIGADE_LAST(b)) {
- more = apr_brigade_split_ex(b, APR_BUCKET_NEXT(e), tmp);
+ if (APR_BUCKET_IS_METADATA(e)) {
+ if (APR_BUCKET_IS_EOS(e)) {
+ /* there shouldn't be anything after the eos */
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, f->r,
+ "ap_http_chunk_filter eos seen, removing filter");
+ ap_remove_output_filter(f);
+ eos = e;
+ break;
+ }
+ if (AP_BUCKET_IS_ERROR(e) &&
+ (((ap_bucket_error *)(e->data))->status == HTTP_BAD_GATEWAY ||
+ ((ap_bucket_error *)(e->data))->status == HTTP_GATEWAY_TIME_OUT)) {
+ /*
+ * We had a broken backend. Memorize this in the filter
+ * context.
+ */
+ ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, f->r,
+ "ap_http_chunk_filter bad gateway error, suppressing end chunk");
+ ctx->bad_gateway_seen = 1;
+ continue;
+ }
+ if (APR_BUCKET_IS_FLUSH(e)) {
+ flush = e;
+ if (e != APR_BRIGADE_LAST(b)) {
+ more = apr_brigade_split_ex(b, APR_BUCKET_NEXT(e), tmp);
+ }
+ break;
+ }
+ if (AP_BUCKET_IS_HEADERS(e)) {
+ ap_bucket_headers *hdrs = e->data;
+ if (!apr_is_empty_table(hdrs->headers)) {
+ if (!ctx->trailers) {
+ ctx->trailers = apr_table_make(f->r->pool, 5);
+ }
+ apr_table_overlap(ctx->trailers, hdrs->headers, APR_OVERLAP_TABLES_MERGE);
Review comment:
First use of the new HEADERS bucket to collect trailers on a HTTP/1.x response and serialize them.
##########
File path: modules/http/http_filters.c
##########
@@ -303,6 +309,122 @@ static apr_status_t read_chunked_trailers(http_ctx_t *ctx, ap_filter_t *f,
apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
ap_input_mode_t mode, apr_read_type_e block,
apr_off_t readbytes)
+{
Review comment:
What follows is the code that used to be at the end of the function. Diff just shows them as addition here and removal below.
##########
File path: modules/http/http_filters.c
##########
@@ -407,51 +530,6 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
}
}
- /* 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->at_eos || f->r->eos_sent || f->r->bytes_sent)) {
- if (!ap_is_HTTP_SUCCESS(f->r->status)) {
- ctx->state = BODY_NONE;
- ctx->at_eos = 1; /* send EOS below */
- }
- else if (!ctx->seen_data) {
- int saved_status = f->r->status;
- const char *saved_status_line = f->r->status_line;
- f->r->status = HTTP_CONTINUE;
- f->r->status_line = NULL;
- ap_send_interim_response(f->r, 0);
- AP_DEBUG_ASSERT(!f->r->expecting_100);
- f->r->status_line = saved_status_line;
- 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 [...]
- */
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, f->r, APLOGNO(10260)
- "request body already/partly received while "
- "100-continue is expected, omit sending interim "
- "response");
- f->r->expecting_100 = 0;
- }
- }
-
Review comment:
The code parts that show as added further above.
##########
File path: modules/http/http_filters.c
##########
@@ -519,8 +597,7 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
if (ctx->state == BODY_CHUNK_TRAILER) {
/* Treat UNSET as DISABLE - trailers aren't merged by default */
- return read_chunked_trailers(ctx, f, b,
- conf->merge_trailers == AP_MERGE_TRAILERS_ENABLE);
+ return read_chunked_trailers(ctx, f, b);
Review comment:
We do not need to merge here, has we append (possible) trailers as HEADERS bucket to the brigade to passing up the filter chain.
##########
File path: modules/proxy/proxy_util.c
##########
@@ -815,7 +815,8 @@ PROXY_DECLARE(int) ap_proxy_checkproxyblock(request_rec *r, proxy_server_conf *c
PROXY_DECLARE(int) ap_proxy_pre_http_request(conn_rec *c, request_rec *r)
{
ap_add_input_filter("HTTP_IN", NULL, r, c);
- return OK;
+ ap_add_input_filter("HTTP1_BODY_IN", NULL, r, c);
Review comment:
Adding this for proxy request. Could check on protocol version, but this only runs on HTTP/1.x requests.
##########
File path: server/protocol.c
##########
@@ -1583,6 +1596,14 @@ request_rec *ap_read_request(conn_rec *conn)
*/
ap_add_input_filter_handle(ap_http_input_filter_handle,
NULL, r, r->connection);
+ if (r->proto_num <= HTTP_VERSION(1,1)) {
+ ap_add_input_filter_handle(ap_http1_body_in_filter_handle,
+ NULL, r, r->connection);
+ if (r->proto_num >= HTTP_VERSION(1,0)
+ && apr_table_get(r->headers_in, "Transfer-Encoding")) {
+ r->body_indeterminate = 1;
+ }
+ }
Review comment:
Using the new filter and flag on `ap_read_request()` for the proper HTTP version.
##########
File path: modules/http2/h2_c2_filter.c
##########
@@ -668,147 +668,13 @@ apr_status_t h2_c2_filter_response_out(ap_filter_t *f, apr_bucket_brigade *bb)
}
Review comment:
Removing all the pain in HTTP/2 that needed to emulate `Transfer-Encoding: chunked` for request bodies without `Content-Length`, so that `HTTP_IN` could parse it and no one assumed there was no body.
##########
File path: modules/http/http_filters.c
##########
@@ -1636,25 +1713,22 @@ AP_DECLARE(int) ap_discard_request_body(request_rec *r)
AP_DECLARE(int) ap_setup_client_block(request_rec *r, int read_policy)
{
- const char *tenc = apr_table_get(r->headers_in, "Transfer-Encoding");
Review comment:
This is the first place were we eliminate checks on `Transfer-Encoding`, because HTTP version > 1.1 do no longer use it.
##########
File path: server/protocol.c
##########
@@ -1465,6 +1466,18 @@ static void apply_server_config(request_rec *r)
r->per_dir_config = r->server->lookup_defaults;
}
+AP_DECLARE(int) ap_assign_request(request_rec *r,
+ const char *method, const char *uri,
+ const char *protocol)
+{
+ /* dummy, for now */
+ (void)r;
+ (void)method;
+ (void)uri;
+ (void)protocol;
+ return 0;
+}
+
Review comment:
The proper implementation will come in a later PR when we start using REQUEST buckets.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org