Attached is the rollup of everything that we have collected
to address c-l/t-e conformance in the httpd-2.1 trunk, which
should apply clean to httpd-2.0 branch. I'll commit in 24 hrs
by lazy consensus, but would rather see a few +1's first.
Jeff's patch (without the protocol.c patch) demonstrated to me
how broken 2.1 really was. I've refactored that code entirely.
After this refactoring (and still without the protocol.c patch)
the attached chunked.req works correctly against an array of
Apache versions. I totally agree with Jeff's plan, the only
way to address this is to backport the corrected 2.1 proxy
request handler.
The 'TraceEnable extended' option has really proved invaluable
for this testing; I would encourage anyone investigating these
issues to check it out.
Bill
p.s. I've found this mini-configlet very useful for my test array;
ProxyPass /13/ http://localhost:8013/
ProxyPassReverse /13/ http://localhost:8013/
ProxyPass /20/ http://localhost:8020/
ProxyPassReverse /20/ http://localhost:8020/
ProxyPass /21/ http://localhost:8021/
ProxyPassReverse /21/ http://localhost:8021/
ProxyPass /99/ http://localhost:8099/
ProxyPassReverse /99/ http://localhost:8099/
TraceEnable extended
Index: server/core.c
===================================================================
--- server/core.c (revision 210154)
+++ server/core.c (working copy)
@@ -458,6 +458,8 @@
conf->redirect_limit = 0; /* 0 == unset */
conf->subreq_limit = 0;
+ conf->trace_enable = AP_TRACE_UNSET;
+
return (void *)conf;
}
@@ -489,6 +491,10 @@
? virt->subreq_limit
: base->subreq_limit;
+ conf->trace_enable = (virt->trace_enable != AP_TRACE_UNSET)
+ ? virt->trace_enable
+ : base->trace_enable;
+
return conf;
}
@@ -1587,7 +1593,7 @@
methnum = ap_method_number_of(method);
if (methnum == M_TRACE && !tog) {
- return "TRACE cannot be controlled by <Limit>";
+ return "TRACE cannot be controlled by <Limit>, see TraceEnable";
}
else if (methnum == M_INVALID) {
/* method has not been registered yet, but resorce restriction
@@ -3113,6 +3119,28 @@
return rv;
}
+static const char *set_trace_enable(cmd_parms *cmd, void *dummy,
+ const char *arg1)
+{
+ core_server_config *conf = ap_get_module_config(cmd->server->module_config,
+ &core_module);
+
+ if (strcasecmp(arg1, "on") == 0) {
+ conf->trace_enable = AP_TRACE_ENABLE;
+ }
+ else if (strcasecmp(arg1, "off") == 0) {
+ conf->trace_enable = AP_TRACE_DISABLE;
+ }
+ else if (strcasecmp(arg1, "extended") == 0) {
+ conf->trace_enable = AP_TRACE_EXTENDED;
+ }
+ else {
+ return "TraceEnable must be one of 'on', 'off', or 'extended'";
+ }
+
+ return NULL;
+}
+
/* Note --- ErrorDocument will now work from .htaccess files.
* The AllowOverride of Fileinfo allows webmasters to turn it off
*/
@@ -3338,6 +3366,8 @@
AP_INIT_TAKE1("EnableExceptionHook", ap_mpm_set_exception_hook, NULL,
RSRC_CONF,
"Controls whether exception hook may be called after a crash"),
#endif
+AP_INIT_TAKE1("TraceEnable", set_trace_enable, NULL, RSRC_CONF,
+ "'on' (default), 'off' or 'extended' to trace request body
content"),
{ NULL }
};
Index: server/protocol.c
===================================================================
--- server/protocol.c (revision 210154)
+++ server/protocol.c (working copy)
@@ -885,6 +885,15 @@
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) {
Index: modules/http/http_protocol.c
===================================================================
--- modules/http/http_protocol.c (revision 210154)
+++ modules/http/http_protocol.c (working copy)
@@ -1321,6 +1321,9 @@
apr_int64_t mask;
apr_array_header_t *allow = apr_array_make(r->pool, 10, sizeof(char *));
apr_hash_index_t *hi = apr_hash_first(r->pool, methods_registry);
+ /* For TRACE below */
+ core_server_config *conf =
+ ap_get_module_config(r->server->module_config, &core_module);
mask = r->allowed_methods->method_mask;
@@ -1338,8 +1341,9 @@
}
}
- /* TRACE is always allowed */
- *(const char **)apr_array_push(allow) = "TRACE";
+ /* TRACE is tested on a per-server basis */
+ if (conf->trace_enable != AP_TRACE_DISABLE)
+ *(const char **)apr_array_push(allow) = "TRACE";
list = apr_array_pstrcat(r->pool, allow, ',');
@@ -1364,9 +1368,16 @@
AP_DECLARE_NONSTD(int) ap_send_http_trace(request_rec *r)
{
+ core_server_config *conf;
int rv;
- apr_bucket_brigade *b;
+ apr_bucket_brigade *bb;
header_struct h;
+ apr_bucket *b;
+ int body;
+ char *bodyread, *bodyoff;
+ apr_size_t bodylen = 0;
+ apr_size_t bodybuf;
+ long res;
if (r->method_number != M_TRACE) {
return DECLINED;
@@ -1376,24 +1387,88 @@
while (r->prev) {
r = r->prev;
}
+ conf = (core_server_config *)ap_get_module_config(r->server->module_config,
+ &core_module);
- if ((rv = ap_setup_client_block(r, REQUEST_NO_BODY))) {
+ if (conf->trace_enable == AP_TRACE_DISABLE) {
+ apr_table_setn(r->notes, "error-notes",
+ "TRACE denied by server configuration");
+ return HTTP_FORBIDDEN;
+ }
+
+ if (conf->trace_enable == AP_TRACE_EXTENDED)
+ /* XX should be = REQUEST_CHUNKED_PASS */
+ body = REQUEST_CHUNKED_DECHUNK;
+ else
+ body = REQUEST_NO_BODY;
+
+ if ((rv = ap_setup_client_block(r, body))) {
+ if (rv == HTTP_REQUEST_ENTITY_TOO_LARGE)
+ apr_table_setn(r->notes, "error-notes",
+ "TRACE with a request body is not allowed");
return rv;
}
+ if (ap_should_client_block(r)) {
+
+ if (r->remaining > 0) {
+ if (r->remaining > 65536) {
+ apr_table_setn(r->notes, "error-notes",
+ "Extended TRACE request bodies cannot exceed 64k\n");
+ return HTTP_REQUEST_ENTITY_TOO_LARGE;
+ }
+ /* always 32 extra bytes to catch chunk header exceptions */
+ bodybuf = (apr_size_t)r->remaining + 32;
+ }
+ else {
+ /* Add an extra 8192 for chunk headers */
+ bodybuf = 73730;
+ }
+
+ bodyoff = bodyread = apr_palloc(r->pool, bodybuf);
+
+ /* only while we have enough for a chunked header */
+ while ((!bodylen || bodybuf >= 32) &&
+ (res = ap_get_client_block(r, bodyoff, bodybuf)) > 0) {
+ bodylen += res;
+ bodybuf -= res;
+ bodyoff += res;
+ }
+ if (res > 0 && bodybuf < 32) {
+ /* discard_rest_of_request_body into our buffer */
+ while (ap_get_client_block(r, bodyread, bodylen) > 0)
+ ;
+ apr_table_setn(r->notes, "error-notes",
+ "Extended TRACE request bodies cannot exceed 64k\n");
+ return HTTP_REQUEST_ENTITY_TOO_LARGE;
+ }
+
+ if (res < 0) {
+ return HTTP_BAD_REQUEST;
+ }
+ }
+
ap_set_content_type(r, "message/http");
/* Now we recreate the request, and echo it back */
- b = apr_brigade_create(r->pool, r->connection->bucket_alloc);
- apr_brigade_putstrs(b, NULL, NULL, r->the_request, CRLF, NULL);
+ bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
+ apr_brigade_putstrs(bb, NULL, NULL, r->the_request, CRLF, NULL);
h.pool = r->pool;
- h.bb = b;
+ h.bb = bb;
apr_table_do((int (*) (void *, const char *, const char *))
form_header_field, (void *) &h, r->headers_in, NULL);
- apr_brigade_puts(b, NULL, NULL, CRLF);
- ap_pass_brigade(r->output_filters, b);
+ apr_brigade_puts(bb, NULL, NULL, CRLF);
+ /* If configured to accept a body, echo the body */
+ if (bodylen) {
+ b = apr_bucket_pool_create(bodyread, bodylen,
+ r->pool, bb->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(bb, b);
+ }
+
+ ap_pass_brigade(r->output_filters, bb);
+
return DONE;
}
Index: modules/proxy/proxy_http.c
===================================================================
--- modules/proxy/proxy_http.c (revision 210154)
+++ modules/proxy/proxy_http.c (working copy)
@@ -264,7 +264,7 @@
if ((backend->connection->id == c->id) &&
(backend->port == p_conn->port) &&
(backend->hostname) &&
- (!apr_strnatcasecmp(backend->hostname, p_conn->name))) {
+ (!strcasecmp(backend->hostname, p_conn->name))) {
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
"proxy: keepalive address match (keep original
socket)");
} else {
@@ -467,6 +467,9 @@
input_brigade = apr_brigade_create(p, bucket_alloc);
+ add_te_chunked(p, bucket_alloc, header_brigade);
+ terminate_headers(bucket_alloc, header_brigade);
+
do {
char chunk_hdr[20]; /* must be here due to transient bucket. */
@@ -515,8 +518,6 @@
/* we never sent the header brigade, so go ahead and
* take care of that now
*/
- add_te_chunked(p, bucket_alloc, header_brigade);
- terminate_headers(bucket_alloc, header_brigade);
b = header_brigade;
APR_BRIGADE_CONCAT(b, input_brigade);
header_brigade = NULL;
@@ -533,9 +534,8 @@
if (header_brigade) {
/* we never sent the header brigade because there was no request body;
- * send it now without T-E
+ * send it now
*/
- terminate_headers(bucket_alloc, header_brigade);
b = header_brigade;
}
else {
@@ -569,7 +569,16 @@
apr_bucket_alloc_t *bucket_alloc = r->connection->bucket_alloc;
apr_bucket_brigade *b, *input_brigade;
apr_bucket *e;
+ apr_off_t cl_val;
+ apr_off_t bytes;
+ apr_off_t bytes_streamed = 0;
+ if (old_cl_val) {
+ add_cl(p, bucket_alloc, header_brigade, old_cl_val);
+ cl_val = atol(old_cl_val);
+ }
+ terminate_headers(bucket_alloc, header_brigade);
+
input_brigade = apr_brigade_create(p, bucket_alloc);
do {
@@ -581,6 +590,8 @@
return status;
}
+ apr_brigade_length(input_brigade, 1, &bytes);
+
/* If this brigade contains EOS, either stop or remove it. */
if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade))) {
seen_eos = 1;
@@ -601,8 +612,6 @@
/* we never sent the header brigade, so go ahead and
* take care of that now
*/
- add_cl(p, bucket_alloc, header_brigade, old_cl_val);
- terminate_headers(bucket_alloc, header_brigade);
b = header_brigade;
APR_BRIGADE_CONCAT(b, input_brigade);
header_brigade = NULL;
@@ -615,16 +624,21 @@
if (status != APR_SUCCESS) {
return status;
}
+
+ bytes_streamed += bytes;
} while (!seen_eos);
+ if (bytes_streamed != cl_val) {
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_ENOTIMPL, r->server,
+ "proxy: client %s given Content-Length did not match",
+ " number of body bytes read", r->connection->remote_ip);
+ return APR_EOF;
+ }
+
if (header_brigade) {
/* we never sent the header brigade since there was no request
- * body; send it now, and only specify C-L if client specified
- * C-L: 0
+ * body; send it now
*/
- if (!strcmp(old_cl_val, "0")) {
- add_cl(p, bucket_alloc, header_brigade, old_cl_val);
- }
terminate_headers(bucket_alloc, header_brigade);
b = header_brigade;
}
@@ -642,7 +656,8 @@
request_rec *r,
proxy_http_conn_t *p_conn,
conn_rec *origin,
- apr_bucket_brigade *header_brigade)
+ apr_bucket_brigade *header_brigade,
+ int force_cl)
{
int seen_eos = 0;
apr_status_t status;
@@ -737,7 +752,7 @@
} while (!seen_eos);
- if (bytes_spooled) {
+ if (bytes_spooled || force_cl) {
add_cl(p, bucket_alloc, header_brigade, apr_off_t_toa(p,
bytes_spooled));
}
terminate_headers(bucket_alloc, header_brigade);
@@ -770,101 +785,14 @@
return status;
}
-static apr_status_t send_request_body(apr_pool_t *p,
- request_rec *r,
- proxy_http_conn_t *p_conn,
- conn_rec *origin,
- apr_bucket_brigade *header_brigade,
- int force10)
-{
- enum {RB_INIT, RB_STREAM_CL, RB_STREAM_CHUNKED, RB_SPOOL_CL} rb_method =
RB_INIT;
- const char *old_cl_val, *te_val;
- int cl_zero; /* client sent "Content-Length: 0", which we forward on to
server */
- apr_status_t status;
-
- /* send CL or use chunked encoding?
- *
- * . CL is the most friendly to the origin server since it is the
- * most widely supported
- * . CL stinks if we don't know the length since we have to buffer
- * the data in memory or on disk until we get the entire data
- *
- * special cases to check for:
- * . if we're using HTTP/1.0 to origin server, then we must send CL
- * . if client sent C-L and there are no input resource filters, the
- * the body size can't change so we send the same CL and stream the
- * body
- * . if client used chunked or proxy-sendchunks is set, we'll also
- * use chunked
- *
- * normal case:
- * we have to compute content length by reading the entire request
- * body; if request body is not small, we'll spool the remaining input
- * to a temporary file
- *
- * special envvars to override the normal decision:
- * . proxy-sendchunks
- * use chunked encoding; not compatible with force-proxy-request-1.0
- * . proxy-sendcl
- * spool the request body to compute C-L
- * . proxy-sendunchangedcl
- * use C-L from client and spool the request body
- */
- old_cl_val = apr_table_get(r->headers_in, "Content-Length");
- cl_zero = old_cl_val && !strcmp(old_cl_val, "0");
-
- if (!force10
- && !cl_zero
- && apr_table_get(r->subprocess_env, "proxy-sendchunks")) {
- rb_method = RB_STREAM_CHUNKED;
- }
- else if (!cl_zero
- && apr_table_get(r->subprocess_env, "proxy-sendcl")) {
- rb_method = RB_SPOOL_CL;
- }
- else {
- if (old_cl_val &&
- (r->input_filters == r->proto_input_filters
- || cl_zero
- || apr_table_get(r->subprocess_env, "proxy-sendunchangedcl"))) {
- rb_method = RB_STREAM_CL;
- }
- else if (force10) {
- rb_method = RB_SPOOL_CL;
- }
- else if ((te_val = apr_table_get(r->headers_in, "Transfer-Encoding"))
- && !strcasecmp(te_val, "chunked")) {
- rb_method = RB_STREAM_CHUNKED;
- }
- else {
- rb_method = RB_SPOOL_CL;
- }
- }
-
- switch(rb_method) {
- case RB_STREAM_CHUNKED:
- status = stream_reqbody_chunked(p, r, p_conn, origin, header_brigade);
- break;
- case RB_STREAM_CL:
- status = stream_reqbody_cl(p, r, p_conn, origin, header_brigade,
old_cl_val);
- break;
- case RB_SPOOL_CL:
- status = spool_reqbody_cl(p, r, p_conn, origin, header_brigade);
- break;
- default:
- ap_assert(1 != 1);
- }
-
- return status;
-}
-
static
apr_status_t ap_proxy_http_request(apr_pool_t *p, request_rec *r,
proxy_http_conn_t *p_conn, conn_rec
*origin,
proxy_server_conf *conf,
apr_uri_t *uri,
char *url, apr_bucket_brigade *bb,
- char *server_portstr) {
+ char *server_portstr)
+{
conn_rec *c = r->connection;
char *buf;
apr_bucket *e;
@@ -872,8 +800,15 @@
const apr_table_entry_t *headers_in;
int counter;
apr_status_t status;
+ apr_bucket_brigade *header_brigade;
+ enum rb_methods {RB_INIT, RB_STREAM_CL, RB_STREAM_CHUNKED, RB_SPOOL_CL};
+ enum rb_methods rb_method = RB_INIT;
+ const char *old_cl_val = NULL;
+ const char *old_te_val = NULL;
int force10;
+ header_brigade = apr_brigade_create(p, origin->bucket_alloc);
+
/*
* Send the HTTP/1.1 request to the remote server
*/
@@ -881,40 +816,46 @@
/* strip connection listed hop-by-hop headers from the request */
/* even though in theory a connection: close coming from the client
* should not affect the connection to the server, it's unlikely
- * that subsequent client requests will hit this thread/process, so
- * we cancel server keepalive if the client does.
+ * that subsequent client requests will hit this thread/process,
+ * so we cancel server keepalive if the client does.
*/
- p_conn->close += ap_proxy_liststr(apr_table_get(r->headers_in,
- "Connection"), "close");
+ if (ap_proxy_liststr(apr_table_get(r->headers_in,
+ "Connection"), "close")) {
+ p_conn->close++;
+ /* XXX: we are abusing r->headers_in rather than a copy,
+ * give the core output handler a clue the client would
+ * rather just close.
+ */
+ c->keepalive = AP_CONN_CLOSE;
+ }
+ ap_proxy_clear_connection(p, r->headers_in);
+
/* sub-requests never use keepalives */
if (r->main) {
p_conn->close++;
}
- ap_proxy_clear_connection(p, r->headers_in);
- if (p_conn->close) {
- apr_table_setn(r->headers_in, "Connection", "close");
- origin->keepalive = AP_CONN_CLOSE;
- }
-
- if ( apr_table_get(r->subprocess_env,"force-proxy-request-1.0")) {
+ if (apr_table_get(r->subprocess_env, "force-proxy-request-1.0")
+ || ((r->proto_num < HTTP_VERSION(1,1))
+ && !apr_table_get(r->subprocess_env, "force-proxy-request-1.1")))
{
buf = apr_pstrcat(p, r->method, " ", url, " HTTP/1.0" CRLF, NULL);
force10 = 1;
} else {
buf = apr_pstrcat(p, r->method, " ", url, " HTTP/1.1" CRLF, NULL);
force10 = 0;
+ p_conn->close++;
}
- if ( apr_table_get(r->subprocess_env,"proxy-nokeepalive")) {
- apr_table_unset(r->headers_in, "Connection");
+ if (apr_table_get(r->subprocess_env, "proxy-nokeepalive")) {
origin->keepalive = AP_CONN_CLOSE;
+ p_conn->close++;
}
ap_xlate_proto_to_ascii(buf, strlen(buf));
e = apr_bucket_pool_create(buf, strlen(buf), p, c->bucket_alloc);
- APR_BRIGADE_INSERT_TAIL(bb, e);
- if ( conf->preserve_host == 0 ) {
+ APR_BRIGADE_INSERT_TAIL(header_brigade, e);
+ if (conf->preserve_host == 0) {
if (uri->port_str && uri->port != DEFAULT_HTTP_PORT) {
- buf = apr_pstrcat(p, "Host: ", uri->hostname, ":", uri->port_str,
CRLF,
- NULL);
+ buf = apr_pstrcat(p, "Host: ", uri->hostname, ":", uri->port_str,
+ CRLF, NULL);
} else {
buf = apr_pstrcat(p, "Host: ", uri->hostname, CRLF, NULL);
}
@@ -937,13 +878,21 @@
}
ap_xlate_proto_to_ascii(buf, strlen(buf));
e = apr_bucket_pool_create(buf, strlen(buf), p, c->bucket_alloc);
- APR_BRIGADE_INSERT_TAIL(bb, e);
+ APR_BRIGADE_INSERT_TAIL(header_brigade, e);
/* handle Via */
if (conf->viaopt == via_block) {
/* Block all outgoing Via: headers */
apr_table_unset(r->headers_in, "Via");
} else if (conf->viaopt != via_off) {
+ const char *server_name = ap_get_server_name(r);
+ /* If USE_CANONICAL_NAME_OFF was configured for the proxy virtual host,
+ * then the server name returned by ap_get_server_name() is the
+ * origin server name (which does make too much sense with Via:
headers)
+ * so we use the proxy vhost's name instead.
+ */
+ if (server_name == r->hostname)
+ server_name = r->server->server_hostname;
/* Create a "Via:" request header entry and merge it */
/* Generate outgoing Via: header with/without server comment: */
apr_table_mergen(r->headers_in, "Via",
@@ -951,12 +900,12 @@
? apr_psprintf(p, "%d.%d %s%s (%s)",
HTTP_VERSION_MAJOR(r->proto_num),
HTTP_VERSION_MINOR(r->proto_num),
- ap_get_server_name(r), server_portstr,
+ server_name, server_portstr,
AP_SERVER_BASEVERSION)
: apr_psprintf(p, "%d.%d %s%s",
HTTP_VERSION_MAJOR(r->proto_num),
HTTP_VERSION_MINOR(r->proto_num),
- ap_get_server_name(r), server_portstr)
+ server_name, server_portstr)
);
}
@@ -988,7 +937,7 @@
* determine, where the original request came from.
*/
apr_table_mergen(r->headers_in, "X-Forwarded-For",
- r->connection->remote_ip);
+ c->remote_ip);
/* Add X-Forwarded-Host: so that upstream knows what the
* original request hostname was.
@@ -1002,7 +951,7 @@
* XXX: This duplicates Via: - do we strictly need it?
*/
apr_table_mergen(r->headers_in, "X-Forwarded-Server",
- r->server->server_hostname);
+ r->server->server_hostname);
}
/* send request headers */
@@ -1010,85 +959,184 @@
headers_in_array = apr_table_elts(r->headers_in);
headers_in = (const apr_table_entry_t *) headers_in_array->elts;
for (counter = 0; counter < headers_in_array->nelts; counter++) {
- if (headers_in[counter].key == NULL || headers_in[counter].val == NULL
+ if (headers_in[counter].key == NULL
+ || headers_in[counter].val == NULL
- /* Clear out hop-by-hop request headers not to send
- * RFC2616 13.5.1 says we should strip these headers
- */
- /* Already sent */
- || !apr_strnatcasecmp(headers_in[counter].key, "Host")
+ /* Already sent */
+ || !strcasecmp(headers_in[counter].key, "Host")
- || !apr_strnatcasecmp(headers_in[counter].key, "Keep-Alive")
- || !apr_strnatcasecmp(headers_in[counter].key, "TE")
- || !apr_strnatcasecmp(headers_in[counter].key, "Trailer")
- || !apr_strnatcasecmp(headers_in[counter].key, "Transfer-Encoding")
- || !apr_strnatcasecmp(headers_in[counter].key, "Upgrade")
+ /* Clear out hop-by-hop request headers not to send
+ * RFC2616 13.5.1 says we should strip these headers
+ */
+ || !strcasecmp(headers_in[counter].key, "Keep-Alive")
+ || !strcasecmp(headers_in[counter].key, "TE")
+ || !strcasecmp(headers_in[counter].key, "Trailer")
+ || !strcasecmp(headers_in[counter].key, "Upgrade")
- /* We'll add appropriate Content-Length later, if appropriate.
+ /* XXX: @@@ FIXME: "Proxy-Authorization" should *only* be
+ * suppressed if THIS server requested the authentication,
+ * not when a frontend proxy requested it!
+ *
+ * The solution to this problem is probably to strip out
+ * the Proxy-Authorisation header in the authorisation
+ * code itself, not here. This saves us having to signal
+ * somehow whether this request was authenticated or not.
*/
- || !apr_strnatcasecmp(headers_in[counter].key, "Content-Length")
+ || !strcasecmp(headers_in[counter].key,"Proxy-Authorization")
+ || !strcasecmp(headers_in[counter].key,"Proxy-Authenticate")) {
+ continue;
+ }
- /* XXX: @@@ FIXME: "Proxy-Authorization" should *only* be
- * suppressed if THIS server requested the authentication,
- * not when a frontend proxy requested it!
- *
- * The solution to this problem is probably to strip out
- * the Proxy-Authorisation header in the authorisation
- * code itself, not here. This saves us having to signal
- * somehow whether this request was authenticated or not.
+ /* Skip Transfer-Encoding and Content-Length for now.
*/
- ||
!apr_strnatcasecmp(headers_in[counter].key,"Proxy-Authorization")
- ||
!apr_strnatcasecmp(headers_in[counter].key,"Proxy-Authenticate")) {
+ if (!strcasecmp(headers_in[counter].key, "Transfer-Encoding")) {
+ old_te_val = headers_in[counter].val;
continue;
}
+ if (!strcasecmp(headers_in[counter].key, "Content-Length")) {
+ old_cl_val = headers_in[counter].val;
+ continue;
+ }
+
/* for sub-requests, ignore freshness/expiry headers */
if (r->main) {
- if (headers_in[counter].key == NULL || headers_in[counter].val
== NULL
- || !apr_strnatcasecmp(headers_in[counter].key, "If-Match")
- || !apr_strnatcasecmp(headers_in[counter].key,
"If-Modified-Since")
- || !apr_strnatcasecmp(headers_in[counter].key, "If-Range")
- || !apr_strnatcasecmp(headers_in[counter].key,
"If-Unmodified-Since")
- || !apr_strnatcasecmp(headers_in[counter].key,
"If-None-Match")) {
- continue;
- }
-
- /* If you POST to a page that gets server-side parsed
- * by mod_include, and the parsing results in a reverse
- * proxy call, the proxied request will be a GET, but
- * its request_rec will have inherited the Content-Length
- * of the original request (the POST for the enclosing
- * page). We can't send the original POST's request body
- * as part of the proxied subrequest, so we need to avoid
- * sending the corresponding content length. Otherwise,
- * the server to which we're proxying will sit there
- * forever, waiting for a request body that will never
- * arrive.
- */
- if ((r->method_number == M_GET) && headers_in[counter].key &&
- !apr_strnatcasecmp(headers_in[counter].key,
- "Content-Length")) {
- continue;
- }
+ if (headers_in[counter].key == NULL
+ || headers_in[counter].val == NULL
+ || !strcasecmp(headers_in[counter].key, "If-Match")
+ || !strcasecmp(headers_in[counter].key, "If-Modified-Since")
+ || !strcasecmp(headers_in[counter].key, "If-Range")
+ || !strcasecmp(headers_in[counter].key, "If-Unmodified-Since")
+ || !strcasecmp(headers_in[counter].key, "If-None-Match")) {
+ continue;
+ }
}
-
buf = apr_pstrcat(p, headers_in[counter].key, ": ",
headers_in[counter].val, CRLF,
NULL);
ap_xlate_proto_to_ascii(buf, strlen(buf));
e = apr_bucket_pool_create(buf, strlen(buf), p, c->bucket_alloc);
- APR_BRIGADE_INSERT_TAIL(bb, e);
+ APR_BRIGADE_INSERT_TAIL(header_brigade, e);
}
+ /* Use chunked request body encoding or send a content-length body?
+ *
+ * Always prefer chunked (most efficient) UNLESS;
+ *
+ * * we have no request body (only RB_STREAM_CL forwards no body)
+ *
+ * * we have no T-E, and
+ *
+ * * no filters are inserted or C-L is 0
+ * (we will use RB_STREAM_CL to forward the body quickly)
+ *
+ * * force-proxy-request-1.0 is set
+ *
+ * * proxy_sendcl is set
+ *
+ * * or we have a T-E body, and
+ *
+ * * force-proxy-request-1.0 is set
+ *
+ * * proxy-sendchunks is not set, and proxy-sendcl is set
+ *
+ * Performance notes:
+ *
+ * We have to compute content length by reading the entire request
+ * body; if request body is not small, we'll spool the remaining
+ * input to a temporary file. Chunked is always preferable.
+ *
+ * We can only recycle the client's C-L if the T-E header is absent,
+ * and if the filters are unchanged (the body won't be resized).
+ *
+ * special envvars to override the normal decision:
+ * . proxy-sendchunks (priority over sendcl for T-E bodies)
+ * proxy-sendcl (priority over sendchunks for C-L bodies)
+ */
+
+ if (old_te_val) {
+ if (old_cl_val) {
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_ENOTIMPL, r->server,
+ "proxy: client %s requested Transfer-Encoding body"
+ " with Content-Length (C-L ignored)",
+ c->remote_ip);
+ origin->keepalive = AP_CONN_CLOSE;
+ p_conn->close++;
+ }
+ /* WE only understand chunked. Other modules might inject
+ * (and therefore, decode) other flavors but we don't know
+ * that the can and have done so unless they they remove
+ * their decoding from the headers_in T-E list.
+ * XXX: Make this extensible, but in doing so, presume the
+ * encoding has been done by the extensions' handler, and
+ * do not modify add_te_chunked's logic
+ */
+ if (strcmp(old_te_val, "chunked") != 0) {
+ ap_log_error(APLOG_MARK, APLOG_ERR, APR_ENOTIMPL, r->server,
+ "proxy: %s Transfer-Encoding is not supported",
+ old_te_val);
+ return APR_ENOTIMPL;
+ }
+ else if (force10
+ || (!apr_table_get(r->subprocess_env, "proxy-sendchunks")
+ && apr_table_get(r->subprocess_env, "proxy-sendcl"))) {
+ rb_method = RB_SPOOL_CL;
+ }
+ else {
+ rb_method = RB_STREAM_CHUNKED;
+ }
+ }
+ else if (old_cl_val) {
+ if (r->input_filters == r->proto_input_filters) {
+ rb_method = RB_STREAM_CL;
+ }
+ else if (force10
+ || apr_table_get(r->subprocess_env, "proxy-sendcl")) {
+ rb_method = RB_SPOOL_CL;
+ }
+ else {
+ rb_method = RB_STREAM_CHUNKED;
+ }
+ }
+ else {
+ /* This is an appropriate default; very efficient for no-body
+ * requests, and has the behavior that it will not add T-E
+ * or C-L when the old_cl_val is NULL.
+ */
+ rb_method = RB_SPOOL_CL;
+ }
+
+ /* Handle Connection: header */
+ if (!force10 && p_conn->close) {
+ buf = apr_pstrdup(p, "Connection: close" CRLF);
+ ap_xlate_proto_to_ascii(buf, strlen(buf));
+ e = apr_bucket_pool_create(buf, strlen(buf), p, c->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(header_brigade, e);
+ }
+
/* send the request data, if any. */
- status = send_request_body(p, r, p_conn, origin, bb, force10);
+ switch(rb_method) {
+ case RB_STREAM_CHUNKED:
+ status = stream_reqbody_chunked(p, r, p_conn, origin, header_brigade);
+ break;
+ case RB_STREAM_CL:
+ status = stream_reqbody_cl(p, r, p_conn, origin, header_brigade,
+ old_cl_val);
+ break;
+ case RB_SPOOL_CL:
+ status = spool_reqbody_cl(p, r, p_conn, origin, header_brigade,
+ old_cl_val != NULL);
+ break;
+ }
if (status != APR_SUCCESS) {
ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server,
- "proxy: request failed to %pI (%s)",
- p_conn->addr, p_conn->name);
+ "proxy: pass request data failed to %s"
+ " from %s (%s)",
+ p_conn->name ? p_conn->name: "",
+ c->remote_ip,
+ c->remote_host ? c->remote_host: "");
return status;
}
-
return APR_SUCCESS;
}
@@ -1102,6 +1150,7 @@
char *server_portstr) {
conn_rec *c = r->connection;
char buffer[HUGE_STRING_LEN];
+ const char *buf;
char keepchar;
request_rec *rp;
apr_bucket *e;
@@ -1199,20 +1248,37 @@
r->status = HTTP_BAD_GATEWAY;
r->status_line = "bad gateway";
return r->status;
+ }
- } else {
- /* strip connection listed hop-by-hop headers from response */
- const char *buf;
- p_conn->close += ap_proxy_liststr(apr_table_get(r->headers_out,
- "Connection"),
- "close");
- ap_proxy_clear_connection(p, r->headers_out);
- if ((buf = apr_table_get(r->headers_out, "Content-Type"))) {
- ap_set_content_type(r, apr_pstrdup(p, buf));
- }
- ap_proxy_pre_http_request(origin,rp);
+ /* can't have both Content-Length and Transfer-Encoding */
+ if (apr_table_get(r->headers_out, "Transfer-Encoding")
+ && apr_table_get(r->headers_out, "Content-Length")) {
+ /*
+ * 2616 section 4.4, point 3: "if both Transfer-Encoding
+ * and Content-Length are received, the latter MUST be
+ * ignored";
+ *
+ * To help mitigate HTTP Splitting, unset Content-Length
+ * and shut down the backend server connection
+ * XXX: We aught to treat such a response as uncachable
+ */
+ apr_table_unset(r->headers_out, "Content-Length");
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+ "proxy: server %s returned Transfer-Encoding"
+ " and Content-Length", p_conn->name);
+ p_conn->close += 1;
}
+ /* strip connection listed hop-by-hop headers from response */
+ p_conn->close += ap_proxy_liststr(apr_table_get(r->headers_out,
+ "Connection"),
+ "close");
+ ap_proxy_clear_connection(p, r->headers_out);
+ if ((buf = apr_table_get(r->headers_out, "Content-Type"))) {
+ ap_set_content_type(r, apr_pstrdup(p, buf));
+ }
+ ap_proxy_pre_http_request(origin,rp);
+
/* handle Via header in response */
if (conf->viaopt != via_off && conf->viaopt != via_block) {
/* create a "Via:" response header entry and merge it */
Index: modules/proxy/mod_proxy.c
===================================================================
--- modules/proxy/mod_proxy.c (revision 210154)
+++ modules/proxy/mod_proxy.c (working copy)
@@ -350,6 +350,42 @@
apr_table_set(r->headers_in, "Max-Forwards",
apr_psprintf(r->pool, "%ld", (maxfwd > 0) ? maxfwd : 0));
+ if (r->method_number == M_TRACE) {
+ core_server_config *coreconf = (core_server_config *)
+ ap_get_module_config(sconf, &core_module);
+
+ if (coreconf->trace_enable == AP_TRACE_DISABLE)
+ {
+ /* Allow "error-notes" string to be printed by
ap_send_error_response()
+ * Note; this goes nowhere, canned error response need an overhaul.
+ */
+ apr_table_setn(r->notes, "error-notes",
+ "TRACE forbidden by server configuration");
+ apr_table_setn(r->notes, "verbose-error-to", "*");
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ "proxy: TRACE forbidden by server configuration");
+ return HTTP_FORBIDDEN;
+ }
+
+ /* Can't test ap_should_client_block, we aren't ready to send
+ * the client a 100 Continue response till the connection has
+ * been established
+ */
+ if (coreconf->trace_enable != AP_TRACE_EXTENDED
+ && (r->read_length || r->read_chunked || r->remaining))
+ {
+ /* Allow "error-notes" string to be printed by
ap_send_error_response()
+ * Note; this goes nowhere, canned error response need an overhaul.
+ */
+ apr_table_setn(r->notes, "error-notes",
+ "TRACE with request body is not allowed");
+ apr_table_setn(r->notes, "verbose-error-to", "*");
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ "proxy: TRACE with request body is not allowed");
+ return HTTP_REQUEST_ENTITY_TOO_LARGE;
+ }
+ }
+
url = r->filename + 6;
p = strchr(url, ':');
if (p == NULL)
Index: include/ap_mmn.h
===================================================================
--- include/ap_mmn.h (revision 210154)
+++ include/ap_mmn.h (working copy)
@@ -83,6 +83,7 @@
* ap_finalize_sub_req_protocol on Win32 and NetWare
* 20020903.9 (2.0.51-dev) create pcommands and initialize arrays before
* calling ap_setup_prelinked_modules
+ * 20020903.10 (2.0.55-dev) added trace_enable to core_server_config
*/
#define MODULE_MAGIC_COOKIE 0x41503230UL /* "AP20" */
@@ -90,7 +91,7 @@
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 20020903
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 9 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 10 /* 0...n */
/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a
Index: include/http_core.h
===================================================================
--- include/http_core.h (revision 210154)
+++ include/http_core.h (working copy)
@@ -546,6 +546,14 @@
/* recursion backstopper */
int redirect_limit; /* maximum number of internal redirects */
int subreq_limit; /* maximum nesting level of subrequests */
+
+ /* TRACE control */
+#define AP_TRACE_UNSET -1
+#define AP_TRACE_DISABLE 0
+#define AP_TRACE_ENABLE 1
+#define AP_TRACE_EXTENDED 2
+ int trace_enable;
+
} core_server_config;
/* for AddOutputFiltersByType in core.c */
TRACE / HTTP/1.1
Host: localhost
TRACE / HTTP/1.1
Host: localhost
Content-Length: 12
Test This!
TRACE / HTTP/1.1
Host: localhost
Transfer-Encoding: chunked
c
Test This!
0
TRACE / HTTP/1.1
Host: localhost
Transfer-Encoding: chunked
Content-Length: 12
c
Test This!
0
TRACE / HTTP/1.1
Host: localhost
Content-Length: 75
Transfer-Encoding: chunked
c
Test This!
0
TRACE / HTTP/1.1
Host: localhost
Content-Length: 12
Connection:close
Test This!
