On Tue, 12 Nov 2013 11:56:39 -0600
"William A. Rowe Jr." <wr...@rowe-clan.net> wrote:
>
> On Tue, 12 Nov 2013 11:48:16 -0500
> Jim Jagielski <j...@jagunet.com> wrote:
>
> > I intend to T&R 2.2.26 tomorrow... post now if that's
> > an issue or problem...
>
> As I mentioned earlier, two additional patches should possibly be
> considered for protocol correctness. The first you shepherded into
> trunk, so I'm particularly interested in your thoughts on backporting
> this, Jim...
>
> http://svn.apache.org/viewvc?view=revision&revision=1524192
> http://svn.apache.org/viewvc?view=revision&revision=1524770
> (Note that the commit log message is missing patch attribution)
>
> A backport is attached, as best as I've figured from the trunk-modulo-
> 2.2 code path.
>
> The second is the 100-continue behavior, when proxy-interim-response
> is set to RFC. As Yann noted in a very long and winding message
> thread, the core http filter is pushing a 100 continue interim
> status, and then mod_proxy_http is pushing back yet another interim
> status response. The core status response must be suppressed on
> proxy-interim-response RFC requests.
>
> It's not clear where that discussion thread has ended up, or whether
> there is a usable patch to enforce this behavior. As you had the most
> to contribute to that thread, can you give us your thoughts on its
> current status, Jim?
Let's let the question of adopting either or both of these changes
expire at the end of the day. If there is no strong support for
picking up either or both of these in 2.4.7, they can be pended for
some later release.
Committers - your thoughts?
Index: modules/http/http_filters.c
===================================================================
--- modules/http/http_filters.c (revision 1541150)
+++ modules/http/http_filters.c (working copy)
@@ -251,25 +251,33 @@
lenp = apr_table_get(f->r->headers_in, "Content-Length");
if (tenc) {
- if (!strcasecmp(tenc, "chunked")) {
+ if (strcasecmp(tenc, "chunked") == 0 /* fast path */
+ || ap_find_last_token(f->r->pool, tenc, "chunked")) {
ctx->state = BODY_CHUNK;
}
- /* test lenp, because it gives another case we can handle */
- else if (!lenp) {
- /* Something that isn't in HTTP, unless some future
- * edition defines new transfer ecodings, is unsupported.
+ else if (f->r->proxyreq == PROXYREQ_RESPONSE) {
+ /* http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23
+ * Section 3.3.3.3: "If a Transfer-Encoding header field is
+ * present in a response and the chunked transfer coding is not
+ * the final encoding, the message body length is determined by
+ * reading the connection until it is closed by the server."
*/
- ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, f->r,
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, f->r,
+ "Unknown Transfer-Encoding: %s; "
+ "using read-until-close", tenc);
+ tenc = NULL;
+ }
+ else {
+ /* Something that isn't a HTTP request, unless some future
+ * edition defines new transfer encodings, is unsupported.
+ */
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, f->r,
"Unknown Transfer-Encoding: %s", tenc);
return bail_out_on_error(ctx, f, HTTP_NOT_IMPLEMENTED);
}
- else {
- ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, f->r,
- "Unknown Transfer-Encoding: %s; using Content-Length", tenc);
- tenc = NULL;
- }
+ lenp = NULL;
}
- if (lenp && !tenc) {
+ if (lenp) {
char *endstr;
ctx->state = BODY_LENGTH;
Index: server/protocol.c
===================================================================
--- server/protocol.c (revision 1541150)
+++ server/protocol.c (working copy)
@@ -953,6 +953,8 @@
}
if (!r->assbackwards) {
+ const char *tenc;
+
ap_get_mime_headers_core(r, tmp_bb);
if (r->status != HTTP_OK) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
@@ -964,12 +966,34 @@
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. */
+ tenc = apr_table_get(r->headers_in, "Transfer-Encoding");
+ if (tenc) {
+ /* http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23
+ * Section 3.3.3.3: "If a Transfer-Encoding header field is
+ * present in a request and the chunked transfer coding is not
+ * the final encoding ...; the server MUST respond with the 400
+ * (Bad Request) status code and then close the connection".
+ */
+ if (!(strcasecmp(tenc, "chunked") == 0 /* fast path */
+ || ap_find_last_token(r->pool, tenc, "chunked"))) {
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ "client sent unknown Transfer-Encoding "
+ "(%s): %s", tenc, r->uri);
+ r->status = HTTP_BAD_REQUEST;
+ conn->keepalive = AP_CONN_CLOSE;
+ 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;
+ }
+
+ /* http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23
+ * Section 3.3.3.3: "If a message is received with both a
+ * Transfer-Encoding and a Content-Length header field, the
+ * Transfer-Encoding overrides the Content-Length. ... A sender
+ * MUST remove the received Content-Length field".
+ */
apr_table_unset(r->headers_in, "Content-Length");
}
}
