core, modules: like r1657897 but for core and other modules than mod_proxy.

More uses of ap_map_http_request_error() and AP_FILTER_ERROR so that we never
return an HTTP error status from a handler if some filter generated a response
already.

That is, from a handler, either ap_get_brigade() (an input filter) returned
AP_FILTER_ERROR and we must forward it to ap_die(), or ap_pass_brigade() (an
output filter) failed with any status and we must return AP_FILTER_ERROR in
any case for ap_die() to determine whether a response is needed or not.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1665625 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/modules/cache/mod_file_cache.c b/modules/cache/mod_file_cache.c
index d77b8d28a5cedfb99e261379933c74eb616e1007..ca7b548356db1ac1e1f26d88c7158bf952255a6b 100644 (file)
--- a/modules/cache/mod_file_cache.c
+++ b/modules/cache/mod_file_cache.c
@@ -283,7 +283,7 @@ static int mmap_handler(request_rec *r, a_file *file)
     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;
 }
@@ -301,7 +301,7 @@ static int sendfile_handler(request_rec *r, a_file *file)
     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;
 }

diff --git a/modules/cluster/mod_heartmonitor.c b/modules/cluster/mod_heartmonitor.c
index b3d666ba226482636753d1b751b82f5b9b0b25c0..784e5dd0fa041f4a75630a99f467bd606bdb1525 100644 (file)
--- a/modules/cluster/mod_heartmonitor.c
+++ b/modules/cluster/mod_heartmonitor.c
@@ -753,7 +753,7 @@ static int hm_handler(request_rec *r)
     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);
 

diff --git a/modules/dav/fs/repos.c b/modules/dav/fs/repos.c
index 950646eed84fd7e20b384930e01fd4e4ee89b63e..6a5ff765f8461b9cf690884fe14c7ff6b2192196 100644 (file)
--- a/modules/dav/fs/repos.c
+++ b/modules/dav/fs/repos.c
@@ -1105,7 +1105,7 @@ static dav_error * dav_fs_deliver(const dav_resource *resource,
     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.");
     }
 

diff --git a/modules/dav/main/mod_dav.c b/modules/dav/main/mod_dav.c
index 8eee84f8237b5fc2ba6c8c9870d385d3eb9c5d53..1fff958954852918fd975eedd46b55506eeb40ae 100644 (file)
--- a/modules/dav/main/mod_dav.c
+++ b/modules/dav/main/mod_dav.c
@@ -582,6 +582,11 @@ static int dav_handle_err(request_rec *r, dav_error *err,
     /* 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;
 
@@ -1006,9 +1011,7 @@ static int dav_method_put(request_rec *r)
                                        "(URI: %s)", msg);
                 }
                 else {
-                    /* XXX: should this actually be HTTP_BAD_REQUEST? */
-                    http_err = ap_map_http_request_error(rc,
-                            HTTP_INTERNAL_SERVER_ERROR);
+                    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);

diff --git a/modules/filters/mod_reflector.c b/modules/filters/mod_reflector.c
index 469df8e82b1db3918b61979ed74a721ca2440c1a..961092d0ea7cb8214f6488a8c54f8641bf8bc15e 100644 (file)
--- a/modules/filters/mod_reflector.c
+++ b/modules/filters/mod_reflector.c
@@ -116,14 +116,8 @@ static int reflector_handler(request_rec * r)
                                     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);
@@ -160,7 +154,7 @@ static int reflector_handler(request_rec * r)
                     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;
                 }
 
             }

diff --git a/modules/generators/mod_asis.c b/modules/generators/mod_asis.c
index c947e30360f4c13276f98d3db43099045166126f..c2b651be2cd80fbbb2b49b438b33976bfe6fa44b 100644 (file)
--- a/modules/generators/mod_asis.c
+++ b/modules/generators/mod_asis.c
@@ -101,7 +101,7 @@ static int asis_handler(request_rec *r)
         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 {

diff --git a/modules/generators/mod_cgi.c b/modules/generators/mod_cgi.c
index 0602867cdf1c809e6393dc92c3976f9ddeffeba7..7c70119c9443d44ce26622605b916c0bca0149fd 100644 (file)
--- a/modules/generators/mod_cgi.c
+++ b/modules/generators/mod_cgi.c
@@ -857,7 +857,7 @@ static int cgi_handler(request_rec *r)
             }
             ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01225)
                           "Error reading request entity data");
-            return ap_map_http_request_error(rv, HTTP_INTERNAL_SERVER_ERROR);
+            return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
         }
 
         for (bucket = APR_BRIGADE_FIRST(bb);

diff --git a/modules/generators/mod_cgid.c b/modules/generators/mod_cgid.c
index 19505640287d7d08477c4d5090fc1117f7586111..50cc883067fe9ff318563b6cb405ca310acaf642 100644 (file)
--- a/modules/generators/mod_cgid.c
+++ b/modules/generators/mod_cgid.c
@@ -1502,7 +1502,7 @@ static int cgid_handler(request_rec *r)
             }
             ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01270)
                           "Error reading request entity data");
-            return ap_map_http_request_error(rv, HTTP_INTERNAL_SERVER_ERROR);
+            return ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
         }
 
         for (bucket = APR_BRIGADE_FIRST(bb);

diff --git a/modules/http/http_filters.c b/modules/http/http_filters.c
index c413e7108ade9d2bb811c1b0a6c0f6762a114f99..4bc7c821130c570e56efa738e200201ce71aa196 100644 (file)
--- a/modules/http/http_filters.c
+++ b/modules/http/http_filters.c
@@ -377,7 +377,10 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
                 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;
+                }
             }
         }
     }
@@ -1597,6 +1600,13 @@ AP_DECLARE(long) ap_get_client_block(request_rec *r, char *buffer,
     /* 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) {
         apr_bucket *e;
 

diff --git a/modules/proxy/mod_proxy_ajp.c b/modules/proxy/mod_proxy_ajp.c
index a0f3253c0e1c597199a892dc01ba581f2cdb2e44..bce7fac068142bdce7f270486b276a4296bf210c 100644 (file)
--- a/modules/proxy/mod_proxy_ajp.c
+++ b/modules/proxy/mod_proxy_ajp.c
@@ -395,7 +395,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
                                 rv = HTTP_REQUEST_TIME_OUT;
                             }
                             else if (status == AP_FILTER_ERROR) {
-                                data_sent = -1;
+                                rv = AP_FILTER_ERROR;
                             }
                             output_failed = 1;
                             break;
@@ -611,12 +611,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
                       "output: %i", backend_failed, output_failed);
         /* We had a failure: Close connection to backend */
         conn->close = 1;
-        if (data_sent < 0) {
-            /* Return AP_FILTER_ERROR to let ap_die() handle the error */
-            rv = AP_FILTER_ERROR;
-            data_sent = 0;
-        }
-        else if (data_sent) {
+        if (data_sent) {
             /* Return DONE to avoid error messages being added to the stream */
             rv = DONE;
         }
@@ -627,8 +622,8 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
         /* 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;
         }
     }

diff --git a/modules/ssl/ssl_engine_io.c b/modules/ssl/ssl_engine_io.c
index 4bd23a7e50ea4aff287d3e5af921b2f1ab1cf0d0..f0f970f8275b3d336358b6ca8797beb5565c4527 100644 (file)
--- a/modules/ssl/ssl_engine_io.c
+++ b/modules/ssl/ssl_engine_io.c
@@ -1806,7 +1806,7 @@ int ssl_io_buffer_fill(request_rec *r, apr_size_t maxlen)
         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

diff --git a/modules/ssl/ssl_engine_kernel.c b/modules/ssl/ssl_engine_kernel.c
index 2fdd616cd5480167df3260cb17758a01078dc757..0d5588db6d4e0a9f97c13800b325db83286bf2ef 100644 (file)
--- a/modules/ssl/ssl_engine_kernel.c
+++ b/modules/ssl/ssl_engine_kernel.c
@@ -133,7 +133,7 @@ int ssl_hook_ReadReq(request_rec *r)
         && (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;
         }
     }
 

diff --git a/modules/test/mod_dialup.c b/modules/test/mod_dialup.c
index 460d31ad7a6b97281a4cd546b15c8cc784dc7c39..a3abbc35f3e74dc3f1c54b373ddbeb453941d612 100644 (file)
--- a/modules/test/mod_dialup.c
+++ b/modules/test/mod_dialup.c
@@ -85,10 +85,10 @@ dialup_send_pulse(dialup_baton_t *db)
 
         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;
         }
     }
 
@@ -121,7 +121,7 @@ dialup_callback(void *baton)
         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);

diff --git a/server/core.c b/server/core.c
index 253438eacd0684b37afb85d0a2df9c70b6cee8c3..7928bbc98dd3423481ea63b6f65a3043147b9387 100644 (file)
--- a/server/core.c
+++ b/server/core.c
@@ -4675,7 +4675,7 @@ static int default_handler(request_rec *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) */

diff --git a/server/error_bucket.c b/server/error_bucket.c
index 9c118e6144b56dc30adae16f87b4caea42e872b9..52b55c35c431cc7a847f6295d97963cadd51ee25 100644 (file)
--- a/server/error_bucket.c
+++ b/server/error_bucket.c
@@ -61,6 +61,9 @@ AP_DECLARE(apr_bucket *) ap_bucket_error_create(int error, const char *buf,
     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);
 }
 

diff --git a/server/util_xml.c b/server/util_xml.c
index 26f1c6e842877e89faaea63246d110bf887fbbaa..4845194656ef935029705f68fc649d954090c314 100644 (file)
--- a/server/util_xml.c
+++ b/server/util_xml.c
@@ -59,6 +59,7 @@ AP_DECLARE(int) ap_xml_parse_input(request_rec * r, apr_xml_doc **pdoc)
                                 READ_BLOCKSIZE);
 
         if (status != APR_SUCCESS) {
+            result = ap_map_http_request_error(status, HTTP_BAD_REQUEST);
             goto read_error;
         }