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?
And thanks for offering to RM - please remember not to leapfrog the
versions of autoconf/libtool, lest we potentially break configure
behavior on the more obscure platforms, and trigger incompatibilities
in configure.in which only occur on newer versions of autoconf.
Libtool 1.5.26 and autoconf 2.67 were used for 2.2.25 release; any later
1.5 libtool or 2.6x series autoconf aught to work but you would want to
pre- buildconf and review any newer versions before tagging.
I'm happy to RM with that same toolchain as I offered in the first place,
if that environment poses a headache for you. Only the two questions
above seemed relevant to me before moving on with this tag.
Cheers,
Bill
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");
}
}
