will override other parameters given in the same directive. This could be
a missing + or - prefix. PR 52820 [Christophe Jaillet]
+ *) core, modules: Avoid error response/document handling by the core if some
+ handler or input filter already did it while reading the request (causing
+ a double response body). [Yann Ylavic]
+
+ *) mod_proxy_ajp: Fix client connection errors handling and logged status
+ when it occurs. PR 56823. [Yann Ylavic]
+
*) mod_proxy: Use the correct server name for SNI in case the backend
SSL connection itself is established via a proxy server.
PR 57139 [Szabolcs Gyurko <szabolcs gyurko.org>]
PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
[ start all new proposals below, under PATCHES PROPOSED. ]
- *) core, modules: Avoid error response/document handling by the core if some
- handler or input filter already did it while reading the request (causing
- a double response body).
- trunk patch: http://svn.apache.org/r1482522 (partial, ap_map_http_request_error() things only!)
- http://svn.apache.org/r1529988
- http://svn.apache.org/r1529991
- http://svn.apache.org/r1643537
- http://svn.apache.org/r1643543
- http://svn.apache.org/r1657897
- http://svn.apache.org/r1665625
- http://svn.apache.org/r1665721
- http://svn.apache.org/r1674056
- 2.4.x patch: http://people.apache.org/~ylavic/httpd-2.4.x-ap_map_http_request_error-v2.patch
- +1: ylavic, minfrin, wrowe
-
PATCHES PROPOSED TO BACKPORT FROM TRUNK:
[ New proposals should be added at the end of the list ]
* 20120211.44 (2.4.13-dev) Add cgi_pass_auth and AP_CGI_PASS_AUTH_* to
* core_dir_config
* 20120211.45 (2.4.13-dev) Add ap_proxy_connection_reusable()
+ * 20120211.46 (2.4.13-dev) Add ap_map_http_request_error()
*/
#define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 20120211
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 45 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 46 /* 0...n */
/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a
*/
AP_DECLARE(long) ap_get_client_block(request_rec *r, char *buffer, apr_size_t bufsiz);
+/*
+ * Map specific APR codes returned by the filter stack to HTTP error
+ * codes, or the default status code provided. Use it as follows:
+ *
+ * return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
+ *
+ * If the filter has already handled the error, AP_FILTER_ERROR will
+ * be returned, which is cleanly passed through.
+ *
+ * These mappings imply that the filter stack is reading from the
+ * downstream client, the proxy will map these codes differently.
+ * @param rv APR status code
+ * @param status Default HTTP code should the APR code not be recognised
+ * @return Mapped HTTP status code
+ */
+AP_DECLARE(int) ap_map_http_request_error(apr_status_t rv, int status);
+
/**
* In HTTP/1.1, any method can have a body. However, most GET handlers
* wouldn't know what to do with a request body if they received one.
APR_BRIGADE_INSERT_TAIL(bb, b);
if (ap_pass_brigade(r->output_filters, bb) != APR_SUCCESS)
- return HTTP_INTERNAL_SERVER_ERROR;
+ return AP_FILTER_ERROR;
#endif
return OK;
}
APR_BRIGADE_INSERT_TAIL(bb, b);
if (ap_pass_brigade(r->output_filters, bb) != APR_SUCCESS)
- return HTTP_INTERNAL_SERVER_ERROR;
+ return AP_FILTER_ERROR;
#endif
return OK;
}
input_brigade = apr_brigade_create(r->connection->pool, r->connection->bucket_alloc);
status = ap_get_brigade(r->input_filters, input_brigade, AP_MODE_READBYTES, APR_BLOCK_READ, MAX_MSG_LEN);
if (status != APR_SUCCESS) {
- return HTTP_INTERNAL_SERVER_ERROR;
+ return ap_map_http_request_error(status, HTTP_BAD_REQUEST);
}
apr_brigade_flatten(input_brigade, buf, &len);
APR_BRIGADE_INSERT_TAIL(bb, bkt);
if ((status = ap_pass_brigade(output, bb)) != APR_SUCCESS) {
- return dav_new_error(pool, HTTP_FORBIDDEN, 0, status,
+ return dav_new_error(pool, AP_FILTER_ERROR, 0, status,
"Could not write contents to filter.");
}
/* log the errors */
dav_log_err(r, err, APLOG_ERR);
+ if (!ap_is_HTTP_VALID_RESPONSE(err->status)) {
+ /* we have responded already */
+ return AP_FILTER_ERROR;
+ }
+
if (response == NULL) {
dav_error *stackerr = err;
"(URI: %s)", msg);
}
else {
- /* XXX: should this actually be HTTP_BAD_REQUEST? */
- http_err = HTTP_INTERNAL_SERVER_ERROR;
- msg = apr_psprintf(r->pool, "An error occurred while reading"
- " the request body (URI: %s)", msg);
+ http_err = ap_map_http_request_error(rc, HTTP_BAD_REQUEST);
+ msg = apr_psprintf(r->pool,
+ "An error occurred while reading"
+ " the request body (URI: %s)", msg);
}
err = dav_new_error(r->pool, http_err, 0, rc, msg);
break;
APR_BLOCK_READ, HUGE_STRING_LEN);
if (status != APR_SUCCESS) {
- if (status == AP_FILTER_ERROR) {
- apr_brigade_destroy(bbin);
- return status;
- }
- else {
- apr_brigade_destroy(bbin);
- return HTTP_BAD_REQUEST;
- }
+ apr_brigade_destroy(bbin);
+ return ap_map_http_request_error(status, HTTP_BAD_REQUEST);
}
for (bucket = APR_BRIGADE_FIRST(bbin);
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r, APLOGNO(01410)
"reflector_handler: ap_pass_brigade returned %i",
status);
- return HTTP_INTERNAL_SERVER_ERROR;
+ return AP_FILTER_ERROR;
}
}
if (rv != APR_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01236)
"mod_asis: ap_pass_brigade failed for file %s", r->filename);
- return HTTP_INTERNAL_SERVER_ERROR;
+ return AP_FILTER_ERROR;
}
}
else {
}
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01225)
"Error reading request entity data");
- return HTTP_INTERNAL_SERVER_ERROR;
+ return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
}
for (bucket = APR_BRIGADE_FIRST(bb);
}
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01270)
"Error reading request entity data");
- return HTTP_INTERNAL_SERVER_ERROR;
+ return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
}
for (bucket = APR_BRIGADE_FIRST(bb);
e = apr_bucket_flush_create(f->c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(bb, e);
- ap_pass_brigade(f->c->output_filters, bb);
+ rv = ap_pass_brigade(f->c->output_filters, bb);
+ if (rv != APR_SUCCESS) {
+ return AP_FILTER_ERROR;
+ }
}
}
return ap_pass_brigade(f->next, b);
}
+/*
+ * Map specific APR codes returned by the filter stack to HTTP error
+ * codes, or the default status code provided. Use it as follows:
+ *
+ * return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
+ *
+ * If the filter has already handled the error, AP_FILTER_ERROR will
+ * be returned, which is cleanly passed through.
+ *
+ * These mappings imply that the filter stack is reading from the
+ * downstream client, the proxy will map these codes differently.
+ */
+AP_DECLARE(int) ap_map_http_request_error(apr_status_t rv, int status)
+{
+ switch (rv) {
+ case AP_FILTER_ERROR: {
+ return AP_FILTER_ERROR;
+ }
+ case APR_ENOSPC: {
+ return HTTP_REQUEST_ENTITY_TOO_LARGE;
+ }
+ case APR_ENOTIMPL: {
+ return HTTP_NOT_IMPLEMENTED;
+ }
+ case APR_ETIMEDOUT: {
+ return HTTP_REQUEST_TIME_OUT;
+ }
+ default: {
+ return status;
+ }
+ }
+}
+
/* In HTTP/1.1, any method can have a body. However, most GET handlers
* wouldn't know what to do with a request body if they received one.
* This helper routine tests for and reads any message body in the request,
AP_DECLARE(int) ap_discard_request_body(request_rec *r)
{
apr_bucket_brigade *bb;
- int rv, seen_eos;
+ int seen_eos;
+ apr_status_t rv;
/* Sometimes we'll get in a state where the input handling has
* detected an error where we want to drop the connection, so if
APR_BLOCK_READ, HUGE_STRING_LEN);
if (rv != APR_SUCCESS) {
- /* FIXME: If we ever have a mapping from filters (apr_status_t)
- * to HTTP error codes, this would be a good place for them.
- *
- * If we received the special case AP_FILTER_ERROR, it means
- * that the filters have already handled this error.
- * Otherwise, we should assume we have a bad request.
- */
- if (rv == AP_FILTER_ERROR) {
- apr_brigade_destroy(bb);
- return rv;
- }
- else {
- apr_brigade_destroy(bb);
- return HTTP_BAD_REQUEST;
- }
+ apr_brigade_destroy(bb);
+ return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
}
for (bucket = APR_BRIGADE_FIRST(bb);
/* We lose the failure code here. This is why ap_get_client_block should
* not be used.
*/
+ if (rv == AP_FILTER_ERROR) {
+ /* AP_FILTER_ERROR means a filter has responded already,
+ * we are DONE.
+ */
+ apr_brigade_destroy(bb);
+ return -1;
+ }
if (rv != APR_SUCCESS) {
/* if we actually fail here, we want to just return and
* stop trying to read data from the client.
case M_TRACE: {
int access_status;
r->proxyreq = PROXYREQ_NONE;
- if ((access_status = ap_send_http_trace(r)))
- ap_die(access_status, r);
- else
- ap_finalize_request_protocol(r);
+ access_status = ap_send_http_trace(r);
+ ap_die(access_status, r);
return OK;
}
case M_OPTIONS: {
int access_status;
r->proxyreq = PROXYREQ_NONE;
- if ((access_status = ap_send_http_options(r)))
- ap_die(access_status, r);
- else
- ap_finalize_request_protocol(r);
+ access_status = ap_send_http_options(r);
+ ap_die(access_status, r);
return OK;
}
default: {
apr_byte_t conn_reuse = 0;
const char *tenc;
int havebody = 1;
- int output_failed = 0;
+ int client_failed = 0;
int backend_failed = 0;
apr_off_t bb_len;
int data_sent = 0;
int request_ended = 0;
int headers_sent = 0;
- int rv = 0;
+ int rv = OK;
apr_int32_t conn_poll_fd;
apr_pollfd_t *conn_poll;
proxy_server_conf *psf =
if (status != APR_SUCCESS) {
/* We had a failure: Close connection to backend */
conn->close = 1;
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00871)
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r, APLOGNO(00871)
"ap_get_brigade failed");
apr_brigade_destroy(input_brigade);
- return HTTP_BAD_REQUEST;
+ return ap_map_http_request_error(status, HTTP_BAD_REQUEST);
}
/* have something */
if (status != APR_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r, APLOGNO(00880)
"ap_get_brigade failed");
- output_failed = 1;
+ if (APR_STATUS_IS_TIMEUP(status)) {
+ rv = HTTP_REQUEST_TIME_OUT;
+ }
+ else if (status == AP_FILTER_ERROR) {
+ rv = AP_FILTER_ERROR;
+ }
+ client_failed = 1;
break;
}
bufsiz = maxsize;
if (status != APR_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r, APLOGNO(00881)
"apr_brigade_flatten failed");
- output_failed = 1;
+ rv = HTTP_INTERNAL_SERVER_ERROR;
+ client_failed = 1;
break;
}
}
"error processing body.%s",
r->connection->aborted ?
" Client aborted connection." : "");
- output_failed = 1;
+ client_failed = 1;
}
data_sent = 1;
apr_brigade_cleanup(output_brigade);
output_brigade) != APR_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00888)
"error processing end");
- output_failed = 1;
+ client_failed = 1;
}
/* XXX: what about flush here? See mod_jk */
data_sent = 1;
/*
* If connection has been aborted by client: Stop working.
- * Nevertheless, we regard our operation so far as a success:
- * So reset output_failed to 0 and set result to CMD_AJP13_END_RESPONSE
- * But: Close this connection to the backend.
+ * Pretend we are done (data_sent) to avoid further processing.
*/
if (r->connection->aborted) {
- conn->close = 1;
- output_failed = 0;
- result = CMD_AJP13_END_RESPONSE;
- request_ended = 1;
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02821)
+ "client connection aborted");
+ /* no response yet (or ever), set status for access log */
+ if (!headers_sent) {
+ r->status = HTTP_BAD_REQUEST;
+ }
+ client_failed = 1;
+ /* return DONE */
+ data_sent = 1;
+ break;
}
/*
* We either have finished successfully or we failed.
* So bail out
*/
- if ((result == CMD_AJP13_END_RESPONSE) || backend_failed
- || output_failed)
+ if ((result == CMD_AJP13_END_RESPONSE)
+ || backend_failed || client_failed)
break;
/* read the response */
*/
apr_brigade_cleanup(output_brigade);
- if (backend_failed || output_failed) {
+ if (backend_failed || client_failed) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00890)
- "Processing of request failed backend: %i, "
- "output: %i", backend_failed, output_failed);
+ "Processing of request failed backend: %i, client: %i",
+ backend_failed, client_failed);
/* We had a failure: Close connection to backend */
conn->close = 1;
- /* Return DONE to avoid error messages being added to the stream */
if (data_sent) {
+ /* Return DONE to avoid error messages being added to the stream */
rv = DONE;
}
}
/* We had a failure: Close connection to backend */
conn->close = 1;
backend_failed = 1;
- /* Return DONE to avoid error messages being added to the stream */
if (data_sent) {
+ /* Return DONE to avoid error messages being added to the stream */
rv = DONE;
}
}
rv = HTTP_INTERNAL_SERVER_ERROR;
}
}
+ else if (client_failed) {
+ int level = (r->connection->aborted) ? APLOG_DEBUG : APLOG_ERR;
+ ap_log_rerror(APLOG_MARK, level, status, r, APLOGNO(02822)
+ "dialog with client %pI failed",
+ r->connection->client_addr);
+ if (rv == OK) {
+ rv = HTTP_BAD_REQUEST;
+ }
+ }
/*
* Ensure that we sent an EOS bucket thru the filter chain, if we already
* one to the brigade already (no longer making it empty). So we should
* not do this in this case.
*/
- if (data_sent && !r->eos_sent && APR_BRIGADE_EMPTY(output_brigade)) {
+ if (data_sent && !r->eos_sent && !r->connection->aborted
+ && APR_BRIGADE_EMPTY(output_brigade)) {
e = apr_bucket_eos_create(r->connection->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(output_brigade, e);
}
/* If we have added something to the brigade above, send it */
- if (!APR_BRIGADE_EMPTY(output_brigade))
- ap_pass_brigade(r->output_filters, output_brigade);
+ if (!APR_BRIGADE_EMPTY(output_brigade)
+ && ap_pass_brigade(r->output_filters, output_brigade) != APR_SUCCESS) {
+ rv = AP_FILTER_ERROR;
+ }
apr_brigade_destroy(output_brigade);
rv = ap_get_brigade(r->input_filters, ib, AP_MODE_READBYTES,
APR_BLOCK_READ, len);
if (rv != APR_SUCCESS) {
- return HTTP_INTERNAL_SERVER_ERROR;
+ return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
}
apr_brigade_flatten(ib, buf, &len);
buf[len] = '\0';
static apr_status_t dispatch(proxy_conn_rec *conn, proxy_dir_conf *conf,
request_rec *r, apr_pool_t *setaside_pool,
- apr_uint16_t request_id,
- const char **err)
+ apr_uint16_t request_id, const char **err,
+ int *bad_request, int *has_responded)
{
apr_bucket_brigade *ib, *ob;
int seen_end_of_headers = 0, done = 0, ignore_body = 0;
iobuf_size);
if (rv != APR_SUCCESS) {
*err = "reading input brigade";
+ *bad_request = 1;
break;
}
apr_brigade_cleanup(ob);
tmp_b = apr_bucket_eos_create(c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(ob, tmp_b);
+
+ *has_responded = 1;
r->status = status;
- ap_pass_brigade(r->output_filters, ob);
- if (status == HTTP_NOT_MODIFIED) {
+ rv = ap_pass_brigade(r->output_filters, ob);
+ if (rv != APR_SUCCESS) {
+ *err = "passing headers brigade to output filters";
+ }
+ else if (status == HTTP_NOT_MODIFIED) {
/* The 304 response MUST NOT contain
* a message-body, ignore it. */
ignore_body = 1;
/* Send the part of the body that we read while
* reading the headers.
*/
+ *has_responded = 1;
rv = ap_pass_brigade(r->output_filters, ob);
if (rv != APR_SUCCESS) {
*err = "passing brigade to output filters";
* along smaller chunks
*/
if (script_error_status == HTTP_OK && !ignore_body) {
+ *has_responded = 1;
rv = ap_pass_brigade(r->output_filters, ob);
if (rv != APR_SUCCESS) {
*err = "passing brigade to output filters";
if (script_error_status == HTTP_OK) {
b = apr_bucket_eos_create(c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(ob, b);
+
+ *has_responded = 1;
rv = ap_pass_brigade(r->output_filters, ob);
if (rv != APR_SUCCESS) {
*err = "passing brigade to output filters";
if (script_error_status != HTTP_OK) {
ap_die(script_error_status, r); /* send ErrorDocument */
+ *has_responded = 1;
}
return rv;
apr_status_t rv;
apr_pool_t *temp_pool;
const char *err;
+ int bad_request = 0,
+ has_responded = 0;
/* Step 1: Send AP_FCGI_BEGIN_REQUEST */
rv = send_begin_request(conn, request_id);
}
/* Step 3: Read records from the back end server and handle them. */
- rv = dispatch(conn, conf, r, temp_pool, request_id, &err);
+ rv = dispatch(conn, conf, r, temp_pool, request_id,
+ &err, &bad_request, &has_responded);
if (rv != APR_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01075)
"Error dispatching request to %s: %s%s%s",
err ? err : "",
err ? ")" : "");
conn->close = 1;
+ if (has_responded) {
+ return AP_FILTER_ERROR;
+ }
+ if (bad_request) {
+ return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
+ }
return HTTP_SERVICE_UNAVAILABLE;
}
" from %s (%s)", p_conn->addr,
p_conn->hostname ? p_conn->hostname: "",
c->client_ip, c->remote_host ? c->remote_host: "");
- return HTTP_BAD_REQUEST;
+ return ap_map_http_request_error(status, HTTP_BAD_REQUEST);
}
}
" from %s (%s)", p_conn->addr,
p_conn->hostname ? p_conn->hostname: "",
c->client_ip, c->remote_host ? c->remote_host: "");
- return HTTP_BAD_REQUEST;
+ return ap_map_http_request_error(status, HTTP_BAD_REQUEST);
}
}
" from %s (%s)", p_conn->addr,
p_conn->hostname ? p_conn->hostname: "",
c->client_ip, c->remote_host ? c->remote_host: "");
- return HTTP_BAD_REQUEST;
+ return ap_map_http_request_error(status, HTTP_BAD_REQUEST);
}
}
" from %s (%s)",
p_conn->addr, p_conn->hostname ? p_conn->hostname: "",
c->client_ip, c->remote_host ? c->remote_host: "");
- return HTTP_BAD_REQUEST;
+ return ap_map_http_request_error(status, HTTP_BAD_REQUEST);
}
apr_brigade_length(temp_brigade, 1, &bytes);
}
}
- /* XXX: What could we do with that return code? */
- (void)ap_pass_brigade(r->output_filters, bb);
+ if (ap_pass_brigade(r->output_filters, bb)) {
+ return AP_FILTER_ERROR;
+ }
return OK;
}
if (rv) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(02015)
"could not read request body for SSL buffer");
- return HTTP_INTERNAL_SERVER_ERROR;
+ return ap_map_http_request_error(rv, HTTP_INTERNAL_SERVER_ERROR);
}
/* Iterate through the returned brigade: setaside each bucket
&& (upgrade = apr_table_get(r->headers_in, "Upgrade")) != NULL
&& ap_find_token(r->pool, upgrade, "TLS/1.0")) {
if (upgrade_connection(r)) {
- return HTTP_INTERNAL_SERVER_ERROR;
+ return AP_FILTER_ERROR;
}
}
apr_brigade_cleanup(db->tmpbb);
- if (status != OK) {
+ if (status != APR_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, status, db->r, APLOGNO(01867)
"dialup: pulse: ap_pass_brigade failed:");
- return status;
+ return AP_FILTER_ERROR;
}
}
return;
}
else {
- ap_log_rerror(APLOG_MARK, APLOG_ERR, status, db->r, APLOGNO(01868)
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, db->r, APLOGNO(01868)
"dialup: pulse returned: %d", status);
db->r->status = HTTP_OK;
ap_die(status, db->r);
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r, APLOGNO(00133)
"default_handler: ap_pass_brigade returned %i",
status);
- return HTTP_INTERNAL_SERVER_ERROR;
+ return AP_FILTER_ERROR;
}
}
else { /* unusual method (not GET or POST) */
APR_BUCKET_INIT(b);
b->free = apr_bucket_free;
b->list = list;
+ if (!ap_is_HTTP_VALID_RESPONSE(error)) {
+ error = HTTP_INTERNAL_SERVER_ERROR;
+ }
return ap_bucket_error_make(b, error, buf, p);
}
APR_BLOCK_READ, HUGE_STRING_LEN);
if (rv != APR_SUCCESS) {
apr_brigade_destroy(bb);
- return (rv == AP_FILTER_ERROR) ? rv : HTTP_BAD_REQUEST;
+ return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
}
for (bucket = APR_BRIGADE_FIRST(bb);
READ_BLOCKSIZE);
if (status != APR_SUCCESS) {
+ result = ap_map_http_request_error(status, HTTP_BAD_REQUEST);
goto read_error;
}