Merge r1752347 from trunk:

Force mod_proxy_fcgi to read the whole FCGI response
even when the content has not been modified (HTTP 304).

The problem is described in PR 59838. This patch should
avoid bogus reads causing the following issues with
HTTP 304 responses:
- AH01068: Got bogus version X, expected 1
- AH01069: Got bogus rid X, expected 1
- AH01075: Error dispatching request to :
- HTTP 503 logged instead of 304 (even if the external
  client gets correctly a 304)

As discussed on IRC the HTTP_PRECONDITION_FAILED use case
should be handled like the HTTP_NOT_MODIFIED one but it will
be done in a separate commit.

Submitted by: elukey
Reviewed/backported by: jim

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1757670 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index f40f106e0ed773fd52c0c7061b4d6c784f4aeecf..4af97f686fb3f32f395599dd76275fcca5227a97 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,10 @@
 
 Changes with Apache 2.4.24
 
+  *) mod_proxy_fcgi: read the whole FCGI response even when the content has
+     not been modified (HTTP 304) to avoid subsequent bougus reads and
+     confusing error messages logged. [Luca Toscano]
+
   *) mod_http2: h2 status resource follows latest draft, see
      http://www.ietf.org/id/draft-benfield-http2-debug-state-01.txt
      [Stefan Eissing]

diff --git a/STATUS b/STATUS
index 72ad621ebfdafe49a5c265d83858711f8612cf44..ce873ad8987db12c43d1deac7ac135f7c5e4b4a3 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -117,13 +117,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) mod_proxy_fcgi: read the whole FCGI response even when the content has
-     not been modified (HTTP 304) to avoid subsequent bogus reads and
-     confusing error messages logged.
-     trunk patch: http://svn.apache.org/r1752347
-     2.4.x patch: trunk works (modulo CHANGES)
-     +1: elukey, ylavic, sf
-
   *) mod_proxy_fcgi: avoid loops serving proxied error documents.
      trunk patch: http://svn.apache.org/r1753167
      2.4.x patch: trunk works (modulo CHANGES)

diff --git a/modules/proxy/mod_proxy_fcgi.c b/modules/proxy/mod_proxy_fcgi.c
index cad33df9fadb31c6aecf8646183de5bc0992a9eb..edbd23daa4abfbf9064c05f1d7bc267eb05f864f 100644 (file)
--- a/modules/proxy/mod_proxy_fcgi.c
+++ b/modules/proxy/mod_proxy_fcgi.c
@@ -658,18 +658,24 @@ recv_again:
                                 rv = ap_pass_brigade(r->output_filters, ob);
                                 if (rv != APR_SUCCESS) {
                                     *err = "passing headers brigade to output filters";
+                                    break;
                                 }
                                 else if (status == HTTP_NOT_MODIFIED) {
                                     /* The 304 response MUST NOT contain
-                                     * a message-body, ignore it. */
+                                     * a message-body, ignore it.
+                                     * The break is not added since there might
+                                     * be more bytes to read from the FCGI
+                                     * connection. Even if the message-body is
+                                     * ignored we want to avoid subsequent
+                                     * bogus reads. */
                                     ignore_body = 1;
                                 }
                                 else {
                                     ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01070)
                                                     "Error parsing script headers");
                                     rv = APR_EINVAL;
+                                    break;
                                 }
-                                break;
                             }
 
                             if (conf->error_override &&