Mailing-List: contact cvs-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <20051218120751.74899.qmail@minotaur.apache.org>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r357461 - in /httpd/httpd/trunk: CHANGES
 modules/cache/mod_disk_cache.c modules/http/chunk_filter.c
 modules/proxy/mod_proxy_http.c server/core_filters.c
Date: Sun, 18 Dec 2005 12:07:49 -0000
To: cvs@httpd.apache.org
From: rpluem@apache.org

Author: rpluem
Date: Sun Dec 18 04:07:39 2005
New Revision: 357461

URL: http://svn.apache.org/viewcvs?rev=357461&view=rev
Log:
* If the mod_proxy backend connection broke in the middle of the response,
  then
  - Do not cache it.
  - Signal the client that something went wrong by closing the connection
    and not sending the last-chunk marker if the response was T-E chunked.

server/core_filters.c         : Close the connection to the client by setting
                                c->keepalive to AP_CONN_CLOSE.
modules/http/chunk_filter.c   : Do not send last-chunk marker in the case
                                the backend broke.
modules/proxy/mod_proxy_http.c: Signal that the backend connection broke.
modules/cache/mod_disk_cache.c: Respect r->no_cache for discarding the response


Submitted by: Roy T. Fielding, Jim Jagielski, Ruediger Pluem
Reviewed by: Roy T. Fielding, Jim Jagielski, Ruediger Pluem

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/cache/mod_disk_cache.c
    httpd/httpd/trunk/modules/http/chunk_filter.c
    httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
    httpd/httpd/trunk/server/core_filters.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/CHANGES?rev=357461&r1=357460&r2=357461&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Sun Dec 18 04:07:39 2005
@@ -6,7 +6,8 @@
      connection as errored out.  [Justin Erenkrantz]
 
   *) mod_proxy: If we get an error reading the upstream response,
-     close the connection.  [Brian Akins, Justin Erenkrantz]
+     close the connection.
+     [Justin Erenkrantz, Roy T. Fielding, Jim Jagielski, Ruediger Pluem]
 
   *) mod_ssl: Fix a possible crash during access control checks if a
      non-SSL request is processed for an SSL vhost (such as the

Modified: httpd/httpd/trunk/modules/cache/mod_disk_cache.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/cache/mod_disk_cache.c?rev=357461&r1=357460&r2=357461&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_disk_cache.c (original)
+++ httpd/httpd/trunk/modules/cache/mod_disk_cache.c Sun Dec 18 04:07:39 2005
@@ -1010,7 +1010,7 @@
      * sanity checks.
      */
     if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) {
-        if (r->connection->aborted) {
+        if (r->connection->aborted || r->no_cache) {
             ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server,
                          "disk_cache: Discarding body for URL %s "
                          "because connection has been aborted.",

Modified: httpd/httpd/trunk/modules/http/chunk_filter.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/http/chunk_filter.c?rev=357461&r1=357460&r2=357461&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http/chunk_filter.c (original)
+++ httpd/httpd/trunk/modules/http/chunk_filter.c Sun Dec 18 04:07:39 2005
@@ -47,6 +47,7 @@
     apr_bucket_brigade *more;
     apr_bucket *e;
     apr_status_t rv;
+    int bad_gateway_seen = 0;
 
     for (more = NULL; b; b = more, more = NULL) {
         apr_off_t bytes = 0;
@@ -67,6 +68,13 @@
                 eos = e;
                 break;
             }
+            if (AP_BUCKET_IS_ERROR(e)
+                && (((ap_bucket_error *)(e->data))->status
+                    == HTTP_BAD_GATEWAY)) {
+                /* We had a broken backend. Memorize this. */
+                bad_gateway_seen = 1;
+                continue;
+            }
             if (APR_BUCKET_IS_FLUSH(e)) {
                 flush = e;
                 more = apr_brigade_split(b, APR_BUCKET_NEXT(e));
@@ -146,13 +154,19 @@
          *   2) the trailer
          *   3) the end-of-chunked body CRLF
          *
-         * If there is no EOS bucket, then do nothing.
+         * If there is no EOS bucket, or if we had seen an error bucket with
+         * status HTTP_BAD_GATEWAY then do nothing.
+         * The error bucket with status HTTP_BAD_GATEWAY indicates that the
+         * connection to the backend (mod_proxy) broke in the middle of the
+         * response. In order to signal the client that something went wrong
+         * we do not create the last-chunk marker and set c->keepalive to
+         * AP_CONN_CLOSE in the core output filter.
          *
          * XXX: it would be nice to combine this with the end-of-chunk
          * marker above, but this is a bit more straight-forward for
          * now.
          */
-        if (eos != NULL) {
+        if (eos && !bad_gateway_seen) {
             /* XXX: (2) trailers ... does not yet exist */
             e = apr_bucket_immortal_create(ASCII_ZERO ASCII_CRLF
                                            /* <trailers> */

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=357461&r1=357460&r2=357461&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Sun Dec 18 04:07:39 2005
@@ -1481,12 +1481,18 @@
                     }
                     else if (rv != APR_SUCCESS) {
                         /* In this case, we are in real trouble because
-                         * our backend bailed on us, so abort our
-                         * connection to our user too.
+                         * our backend bailed on us.
                          */
                         ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c,
                                       "proxy: error reading response");
-                        c->aborted = 1;
+                        r->no_cache = 1;
+                        e = ap_bucket_error_create(HTTP_BAD_GATEWAY,  NULL,
+                                                   c->pool, c->bucket_alloc);
+                        APR_BRIGADE_INSERT_TAIL(bb, e);
+                        e = apr_bucket_eos_create(c->bucket_alloc);
+                        APR_BRIGADE_INSERT_TAIL(bb, e);
+                        ap_pass_brigade(r->output_filters, bb);
+                        backend->close = 1;
                         break;
                     }
                     /* next time try a non-blocking read */

Modified: httpd/httpd/trunk/server/core_filters.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/server/core_filters.c?rev=357461&r1=357460&r2=357461&view=diff
==============================================================================
--- httpd/httpd/trunk/server/core_filters.c (original)
+++ httpd/httpd/trunk/server/core_filters.c Sun Dec 18 04:07:39 2005
@@ -315,7 +315,9 @@
                                              apr_size_t *bytes_written,
                                              conn_rec *c);
 
-static void remove_empty_buckets(apr_bucket_brigade *bb);
+static void detect_error_bucket(apr_bucket *bucket, conn_rec *c);
+
+static void remove_empty_buckets(apr_bucket_brigade *bb, conn_rec *c);
 
 static apr_status_t send_brigade_blocking(apr_socket_t *s,
                                           apr_bucket_brigade *bb,
@@ -487,7 +489,7 @@
     if (bb == NULL) {
         return;
     }
-    remove_empty_buckets(bb);
+    remove_empty_buckets(bb, c);
     if (!APR_BRIGADE_EMPTY(bb)) {
         c->data_in_output_filters = 1;
         if (make_a_copy) {
@@ -526,7 +528,7 @@
     struct iovec vec[MAX_IOVEC_TO_WRITE];
     apr_size_t nvec = 0;
 
-    remove_empty_buckets(bb);
+    remove_empty_buckets(bb, c);
 
     for (bucket = APR_BRIGADE_FIRST(bb);
          bucket != APR_BRIGADE_SENTINEL(bb);
@@ -596,16 +598,26 @@
         }
     }
 
-    remove_empty_buckets(bb);
+    remove_empty_buckets(bb, c);
 
     return APR_SUCCESS;
 }
 
-static void remove_empty_buckets(apr_bucket_brigade *bb)
+static void detect_error_bucket(apr_bucket *bucket, conn_rec *c)
+{
+    if (AP_BUCKET_IS_ERROR(bucket)
+        && (((ap_bucket_error *)(bucket->data))->status == HTTP_BAD_GATEWAY)) {
+        /* stream aborted and we have not ended it yet */
+        c->keepalive = AP_CONN_CLOSE;
+    }
+}
+
+static void remove_empty_buckets(apr_bucket_brigade *bb, conn_rec *c)
 {
     apr_bucket *bucket;
     while (((bucket = APR_BRIGADE_FIRST(bb)) != APR_BRIGADE_SENTINEL(bb)) &&
            (APR_BUCKET_IS_METADATA(bucket) || (bucket->length == 0))) {
+        detect_error_bucket(bucket, c);
         APR_BUCKET_REMOVE(bucket);
         apr_bucket_destroy(bucket);
     }
@@ -678,6 +690,7 @@
             for (i = offset; i < nvec; ) {
                 apr_bucket *bucket = APR_BRIGADE_FIRST(bb);
                 if (APR_BUCKET_IS_METADATA(bucket)) {
+                    detect_error_bucket(bucket, c);
                     APR_BUCKET_REMOVE(bucket);
                     apr_bucket_destroy(bucket);
                 }