You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2013/05/07 22:27:38 UTC

svn commit: r1480058 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Author: minfrin
Date: Tue May  7 20:27:37 2013
New Revision: 1480058

URL: http://svn.apache.org/r1480058
Log:
mod_proxy: Ensure network errors detected by the proxy are returned as
504 Gateway Timout as opposed to 502 Bad Gateway, in order to be
compliant with RFC2616 14.9.4 Cache Revalidation and Reload Controls.

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c
    httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
    httpd/httpd/trunk/modules/proxy/proxy_util.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1480058&r1=1480057&r2=1480058&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Tue May  7 20:27:37 2013
@@ -1,6 +1,11 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mod_proxy: Ensure network errors detected by the proxy are returned as
+     504 Gateway Timout as opposed to 502 Bad Gateway, in order to be
+     compliant with RFC2616 14.9.4 Cache Revalidation and Reload Controls.
+     [Graham Leggett, Co-Advisor <coad measurement-factory.com>]
+
   *) mod_cache: Ensure that we don't attempt to replace a cached response
      with an older response as per RFC2616 13.12. [Graham Leggett, Co-Advisor
      <coad measurement-factory.com>]

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c?rev=1480058&r1=1480057&r2=1480058&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c Tue May  7 20:27:37 2013
@@ -859,7 +859,11 @@ static int ftp_set_TYPE(char xfer_type, 
     /* 501 Syntax error in parameters or arguments. */
     /* 504 Command not implemented for that parameter. */
     /* 530 Not logged in. */
-    if (rc == -1 || rc == 421) {
+    if (rc == -1) {
+        ret = ap_proxyerror(r, HTTP_GATEWAY_TIME_OUT,
+                             "Error reading from remote server");
+    }
+    else if (rc == 421) {
         ret = ap_proxyerror(r, HTTP_BAD_GATEWAY,
                              "Error reading from remote server");
     }
@@ -891,6 +895,10 @@ static char *ftp_get_PWD(request_rec *r,
     /* 550 Requested action not taken. */
     switch (proxy_ftp_command("PWD" CRLF, r, ftp_ctrl, bb, &ftpmessage)) {
         case -1:
+            ap_proxyerror(r, HTTP_GATEWAY_TIME_OUT,
+                             "Failed to read PWD on ftp server");
+            break;
+
         case 421:
         case 550:
             ap_proxyerror(r, HTTP_BAD_GATEWAY,
@@ -1140,7 +1148,7 @@ static int proxy_ftp_handler(request_rec
      * them until we get a successful connection
      */
     if (APR_SUCCESS != err) {
-        return ap_proxyerror(r, HTTP_BAD_GATEWAY, apr_pstrcat(p,
+        return ap_proxyerror(r, HTTP_GATEWAY_TIME_OUT, apr_pstrcat(p,
                                                  "DNS lookup failure for: ",
                                                         connectname, NULL));
     }
@@ -1212,10 +1220,15 @@ static int proxy_ftp_handler(request_rec
     /* 220 Service ready for new user. */
     /* 421 Service not available, closing control connection. */
     rc = proxy_ftp_command(NULL, r, origin, bb, &ftpmessage);
-    if (rc == -1 || rc == 421) {
-        return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY, "Error reading from remote server");
+    if (rc == -1) {
+        return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                "Error reading from remote server");
+    }
+    else if (rc == 421) {
+        return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY,
+                "Error reading from remote server");
     }
-    if (rc == 120) {
+    else if (rc == 120) {
         /*
          * RFC2616 states: 14.37 Retry-After
          *
@@ -1241,7 +1254,7 @@ static int proxy_ftp_handler(request_rec
         }
         return ftp_proxyerror(r, backend, HTTP_SERVICE_UNAVAILABLE, ftpmessage);
     }
-    if (rc != 220) {
+    else if (rc != 220) {
         return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY, ftpmessage);
     }
 
@@ -1257,15 +1270,20 @@ static int proxy_ftp_handler(request_rec
     /* (This may include errors such as command line too long.) */
     /* 501 Syntax error in parameters or arguments. */
     /* 530 Not logged in. */
-    if (rc == -1 || rc == 421) {
-        return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY, "Error reading from remote server");
+    if (rc == -1) {
+        return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                "Error reading from remote server");
+    }
+    else if (rc == 421) {
+        return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY,
+                "Error reading from remote server");
     }
-    if (rc == 530) {
+    else if (rc == 530) {
         proxy_ftp_cleanup(r, backend);
         return ftp_unauthorized(r, 1);  /* log it: user name guessing
                                          * attempt? */
     }
-    if (rc != 230 && rc != 331) {
+    else if (rc != 230 && rc != 331) {
         return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY, ftpmessage);
     }
 
@@ -1285,21 +1303,25 @@ static int proxy_ftp_handler(request_rec
         /* 501 Syntax error in parameters or arguments. */
         /* 503 Bad sequence of commands. */
         /* 530 Not logged in. */
-        if (rc == -1 || rc == 421) {
+        if (rc == -1) {
+            return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                                  "Error reading from remote server");
+        }
+        else if (rc == 421) {
             return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY,
                                   "Error reading from remote server");
         }
-        if (rc == 332) {
+        else if (rc == 332) {
             return ftp_proxyerror(r, backend, HTTP_UNAUTHORIZED,
                   apr_pstrcat(p, "Need account for login: ", ftpmessage, NULL));
         }
         /* @@@ questionable -- we might as well return a 403 Forbidden here */
-        if (rc == 530) {
+        else if (rc == 530) {
             proxy_ftp_cleanup(r, backend);
             return ftp_unauthorized(r, 1);      /* log it: passwd guessing
                                                  * attempt? */
         }
-        if (rc != 230 && rc != 202) {
+        else if (rc != 230 && rc != 202) {
             return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY, ftpmessage);
         }
     }
@@ -1315,9 +1337,14 @@ static int proxy_ftp_handler(request_rec
             ++path;
 
         rc = proxy_ftp_command("CWD /" CRLF, r, origin, bb, &ftpmessage);
-        if (rc == -1 || rc == 421)
+        if (rc == -1) {
+            return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                    "Error reading from remote server");
+        }
+        else if (rc == 421) {
             return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY,
-                                  "Error reading from remote server");
+                    "Error reading from remote server");
+        }
     }
 
     /*
@@ -1354,14 +1381,18 @@ static int proxy_ftp_handler(request_rec
         /* 502 Command not implemented. */
         /* 530 Not logged in. */
         /* 550 Requested action not taken. */
-        if (rc == -1 || rc == 421) {
+        if (rc == -1) {
+            return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                    "Error reading from remote server");
+        }
+        else if (rc == 421) {
             return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY,
-                                  "Error reading from remote server");
+                    "Error reading from remote server");
         }
-        if (rc == 550) {
+        else if (rc == 550) {
             return ftp_proxyerror(r, backend, HTTP_NOT_FOUND, ftpmessage);
         }
-        if (rc != 250) {
+        else if (rc != 250) {
             return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY, ftpmessage);
         }
 
@@ -1395,11 +1426,15 @@ static int proxy_ftp_handler(request_rec
         /* 501 Syntax error in parameters or arguments. */
         /* 502 Command not implemented. */
         /* 530 Not logged in. */
-        if (rc == -1 || rc == 421) {
+        if (rc == -1) {
+            return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                    "Error reading from remote server");
+        }
+        else if (rc == 421) {
             return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY,
-                                  "Error reading from remote server");
+                    "Error reading from remote server");
         }
-        if (rc != 229 && rc != 500 && rc != 501 && rc != 502) {
+        else if (rc != 229 && rc != 500 && rc != 501 && rc != 502) {
             return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY, ftpmessage);
         }
         else if (rc == 229) {
@@ -1461,8 +1496,10 @@ static int proxy_ftp_handler(request_rec
                     ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01043)
                                   "EPSV attempt to connect to %pI failed - "
                                   "Firewall/NAT?", &epsv_addr);
-                    return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY, apr_psprintf(r->pool,
-                                                                           "EPSV attempt to connect to %pI failed - firewall/NAT?", &epsv_addr));
+                    return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                            apr_psprintf(r->pool,
+                                    "EPSV attempt to connect to %pI failed - firewall/NAT?",
+                                    &epsv_addr));
                 }
                 else {
                     ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
@@ -1484,11 +1521,15 @@ static int proxy_ftp_handler(request_rec
         /* 501 Syntax error in parameters or arguments. */
         /* 502 Command not implemented. */
         /* 530 Not logged in. */
-        if (rc == -1 || rc == 421) {
+        if (rc == -1) {
+            return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                    "Error reading from remote server");
+        }
+        else if (rc == 421) {
             return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY,
-                                  "Error reading from remote server");
+                    "Error reading from remote server");
         }
-        if (rc != 227 && rc != 502) {
+        else if (rc != 227 && rc != 502) {
             return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY, ftpmessage);
         }
         else if (rc == 227) {
@@ -1550,8 +1591,10 @@ static int proxy_ftp_handler(request_rec
                 if (rv != APR_SUCCESS) {
                     ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01048)
                                   "PASV attempt to connect to %pI failed - Firewall/NAT?", pasv_addr);
-                    return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY, apr_psprintf(r->pool,
-                                                                           "PASV attempt to connect to %pI failed - firewall/NAT?", pasv_addr));
+                    return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                            apr_psprintf(r->pool,
+                                    "PASV attempt to connect to %pI failed - firewall/NAT?",
+                                    pasv_addr));
                 }
                 else {
                     connect = 1;
@@ -1621,11 +1664,15 @@ static int proxy_ftp_handler(request_rec
             /* 501 Syntax error in parameters or arguments. */
             /* 502 Command not implemented. */
             /* 530 Not logged in. */
-            if (rc == -1 || rc == 421) {
+            if (rc == -1) {
+                return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                        "Error reading from remote server");
+            }
+            else if (rc == 421) {
                 return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY,
-                                      "Error reading from remote server");
+                        "Error reading from remote server");
             }
-            if (rc != 200) {
+            else if (rc != 200) {
                 return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY, buffer);
             }
 
@@ -1686,9 +1733,13 @@ static int proxy_ftp_handler(request_rec
         rc = proxy_ftp_command(apr_pstrcat(p, "SIZE ",
                            ftp_escape_globbingchars(p, path, fdconf), CRLF, NULL),
                            r, origin, bb, &ftpmessage);
-        if (rc == -1 || rc == 421) {
+        if (rc == -1) {
+            return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                    "Error reading from remote server");
+        }
+        else if (rc == 421) {
             return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY,
-                                  "Error reading from remote server");
+                    "Error reading from remote server");
         }
         else if (rc == 213) {/* Size command ok */
             int j;
@@ -1713,14 +1764,18 @@ static int proxy_ftp_handler(request_rec
             /* 502 Command not implemented. */
             /* 530 Not logged in. */
             /* 550 Requested action not taken. */
-            if (rc == -1 || rc == 421) {
+            if (rc == -1) {
+                return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                        "Error reading from remote server");
+            }
+            else if (rc == 421) {
                 return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY,
-                                      "Error reading from remote server");
+                        "Error reading from remote server");
             }
-            if (rc == 550) {
+            else if (rc == 550) {
                 return ftp_proxyerror(r, backend, HTTP_NOT_FOUND, ftpmessage);
             }
-            if (rc != 250) {
+            else if (rc != 250) {
                 return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY, ftpmessage);
             }
             path = "";
@@ -1827,11 +1882,15 @@ static int proxy_ftp_handler(request_rec
     /* 501 Syntax error in parameters or arguments. */
     /* 530 Not logged in. */
     /* 550 Requested action not taken. */
-    if (rc == -1 || rc == 421) {
+    if (rc == -1) {
+        return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                              "Error reading from remote server");
+    }
+    else if (rc == 421) {
         return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY,
                               "Error reading from remote server");
     }
-    if (rc == 550) {
+    else if (rc == 550) {
         ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, r,
                       "RETR failed, trying LIST instead");
 
@@ -1850,14 +1909,18 @@ static int proxy_ftp_handler(request_rec
         /* 502 Command not implemented. */
         /* 530 Not logged in. */
         /* 550 Requested action not taken. */
-        if (rc == -1 || rc == 421) {
+        if (rc == -1) {
+            return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                    "Error reading from remote server");
+        }
+        else if (rc == 421) {
             return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY,
-                                  "Error reading from remote server");
+                    "Error reading from remote server");
         }
-        if (rc == 550) {
+        else if (rc == 550) {
             return ftp_proxyerror(r, backend, HTTP_NOT_FOUND, ftpmessage);
         }
-        if (rc != 250) {
+        else if (rc != 250) {
             return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY, ftpmessage);
         }
 
@@ -1873,9 +1936,14 @@ static int proxy_ftp_handler(request_rec
                                r, origin, bb, &ftpmessage);
 
         /* rc is an intermediate response for the LIST command (125 transfer starting, 150 opening data connection) */
-        if (rc == -1 || rc == 421)
+        if (rc == -1) {
+            return ftp_proxyerror(r, backend, HTTP_GATEWAY_TIME_OUT,
+                    "Error reading from remote server");
+        }
+        else if (rc == 421) {
             return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY,
-                                  "Error reading from remote server");
+                    "Error reading from remote server");
+        }
     }
     if (rc != 125 && rc != 150 && rc != 226 && rc != 250) {
         return ftp_proxyerror(r, backend, HTTP_BAD_GATEWAY, ftpmessage);
@@ -1947,7 +2015,7 @@ static int proxy_ftp_handler(request_rec
                 ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01053)
                               "failed to accept data connection");
                 proxy_ftp_cleanup(r, backend);
-                return HTTP_BAD_GATEWAY;
+                return HTTP_GATEWAY_TIME_OUT;
             }
         }
     }

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1480058&r1=1480057&r2=1480058&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue May  7 20:27:37 2013
@@ -1374,7 +1374,7 @@ apr_status_t ap_proxy_http_process_respo
                                    " failed.",
                                    backend->hostname, backend->port);
             }
-            return ap_proxyerror(r, HTTP_BAD_GATEWAY,
+            return ap_proxyerror(r, HTTP_GATEWAY_TIME_OUT,
                                  "Error reading from remote server");
         }
         /* XXX: Is this a real headers length send from remote? */
@@ -1722,7 +1722,7 @@ apr_status_t ap_proxy_http_process_respo
                     }
                     else if (rv != APR_SUCCESS) {
                         /* In this case, we are in real trouble because
-                         * our backend bailed on us. Pass along a 502 error
+                         * our backend bailed on us. Pass along a 504 error
                          * error bucket
                          */
                         ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01110)

Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1480058&r1=1480057&r2=1480058&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Tue May  7 20:27:37 2013
@@ -2203,7 +2203,7 @@ ap_proxy_determine_connection(apr_pool_t
     }
 
     if (err != APR_SUCCESS) {
-        return ap_proxyerror(r, HTTP_BAD_GATEWAY,
+        return ap_proxyerror(r, HTTP_GATEWAY_TIME_OUT,
                              apr_pstrcat(p, "DNS lookup failure for: ",
                                          conn->hostname, NULL));
     }
@@ -2761,7 +2761,7 @@ PROXY_DECLARE(void) ap_proxy_backend_bro
      */
     if (r->main)
         r->main->no_cache = 1;
-    e = ap_bucket_error_create(HTTP_BAD_GATEWAY, NULL, c->pool,
+    e = ap_bucket_error_create(HTTP_GATEWAY_TIME_OUT, NULL, c->pool,
                                c->bucket_alloc);
     APR_BRIGADE_INSERT_TAIL(brigade, e);
     e = apr_bucket_eos_create(c->bucket_alloc);
@@ -3281,7 +3281,7 @@ PROXY_DECLARE(int) ap_proxy_pass_brigade
                                      "Error during SSL Handshake with"
                                      " remote server");
             }
-            return APR_STATUS_IS_TIMEUP(status) ? HTTP_GATEWAY_TIME_OUT : HTTP_BAD_GATEWAY;
+            return HTTP_GATEWAY_TIME_OUT;
         }
         else {
             return HTTP_BAD_REQUEST;



Re: svn commit: r1480058 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by Graham Leggett <mi...@sharp.fm>.
On 09 May 2013, at 12:36 AM, Roy T. Fielding <fi...@gbiv.com> wrote:

> Unfortunately, I am at the tail end of a long standards meeting and
> haven't slept much for three days.  Have you checked to see if the
> descriptions changed in HTTPbis p6?  RFC2616 hasn't been relevant
> for a while now.
> 
> http://svn.tools.ietf.org/svn/wg/httpbis/draft-ietf-httpbis/latest/p6-cache.html
> 
> I'll look at it tomorrow when I have slept a bit.

I had done, but I hadn't found anything different to RFC2616.

There is the letter of law, and the spirit of the law, keen to see what the spirit of the law is to work out the right way to handle this.

Regards,
Graham
--


Re: svn commit: r1480058 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, May 17, 2013 at 2:15 PM, Yann Ylavic <yl...@gmail.com> wrote:

> For example ap_http_outerror_filter(), ap_http_chunk_filter() and maybe
> others (grep where HTTP_BAD_GATEWAY is checked in the source tree) should
> be modified accordingly.
>
>
I attached a patch (
https://issues.apache.org/bugzilla/attachment.cgi?id=30849) to bug 55475
regarding this issue.
Currently in trunk, the client connection is kept alive when mod_proxy_http
sends an error bucket through the output filters.

Regards,
> Yann.
>

Re: svn commit: r1480058 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by Yann Ylavic <yl...@gmail.com>.
Sorry to interfere in the debate with a non-RFC argument but there may be
aftermath by changing a long standing mod_proxy 502 error for almost any
non-recoverable problem with the upstream server.

On Wed, May 8, 2013 at 10:06 AM, Graham Leggett <mi...@sharp.fm> wrote:

> Arbitrarily changing a 502 to a 504 on the fly will confuse people, as
> this server isn't the only server that could generate a 502, and 502 might
> be the intended error to be sent to the client.
>
>
Some (third-party-)modules/filters/scripts that rely on the 502 error
(bucket) to take an action regarding the upstream server will also be
confused by the commit.

For example ap_http_outerror_filter(), ap_http_chunk_filter() and maybe
others (grep where HTTP_BAD_GATEWAY is checked in the source tree) should
be modified accordingly.

I guess this argument may not be weighty relative to RFC compliance...

 On Wed, May 8, 2013 at 9:47 AM, Ruediger Pluem <rp...@apache.org> wrote:

> So while changing the response to 504 for failed DNS lookups is always
> correct it is not for other failures.
>

Do you mean 504, like 503, is admissible for idempotent requests replay or
balancer failover ?
Currently this is not the case...

Regards,
Yann.

RE: svn commit: r1480058 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

> -----Original Message-----
> From: Roy T. Fielding 
> Sent: Donnerstag, 9. Mai 2013 00:36
> To: dev@httpd.apache.org
> Subject: Re: svn commit: r1480058 - in /httpd/httpd/trunk: CHANGES
> modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c
> modules/proxy/proxy_util.c
> 
> On May 8, 2013, at 1:11 AM, Ruediger Pluem wrote:
> > Graham Leggett wrote:
> >> On 08 May 2013, at 9:47 AM, Ruediger Pluem <rp...@apache.org> wrote:
> >>
> >>> I don't agree with this. The case you mention is only true if the
> client sends Cache-Control: must-revalidate.
> >>> If this is not the case IMHO 10.5.3 and 10.5.5 apply.
> >>> And only a cache is required to respond with 504 in this case, not a
> gateway or a proxy. So the cache should
> >>> change a 502 to a 504 in case the revalidation fails. Imagine the
> case where you have other backend modules
> >>> as our proxy modules.
> >>> So while changing the response to 504 for failed DNS lookups is
> always correct it is not for other failures.
> >>> 10.5.3: The server, while acting as a gateway or proxy, received an
> invalid response from the upstream server it
> >>> accessed in attempting to fulfill the request.
> >>
> >> Arbitrarily changing a 502 to a 504 on the fly will confuse people,
> as this server isn't the only server that could generate a 502, and 502
> might be the intended error to be sent to the client.
> >>
> >> The way I interpret the RFC is that "received an invalid response"
> described by 10.5.3 occurs when the upstream has said something that the
> proxy doesn't like, while the unfortunately named 10.5.5 504 Gateway
> Timed Out describes a "timely response" which means "at the appropriate
> time", meaning a network error of some kind.
> >>
> >> The change above isolates network errors specifically and returns 504
> in those cases only, while leaving 502 for genuine protocol errors,
> mostly affecting ftp.
> >>
> >> Ultimately Roy is the authority on this?
> >
> > Good idea. He also knows possible clarifications on that that are work
> in progress and that we could already apply.
> > Roy can you please help us here?
> 
> Unfortunately, I am at the tail end of a long standards meeting and
> haven't slept much for three days.  Have you checked to see if the
> descriptions changed in HTTPbis p6?  RFC2616 hasn't been relevant
> for a while now.
> 
> http://svn.tools.ietf.org/svn/wg/httpbis/draft-ietf-httpbis/latest/p6-
> cache.html
> 
> I'll look at it tomorrow when I have slept a bit.

Sorry for being impatient :-)
But any update on this Roy?


Regards

Rüdiger

Re: svn commit: r1480058 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On May 8, 2013, at 1:11 AM, Ruediger Pluem wrote:
> Graham Leggett wrote:
>> On 08 May 2013, at 9:47 AM, Ruediger Pluem <rp...@apache.org> wrote:
>> 
>>> I don't agree with this. The case you mention is only true if the client sends Cache-Control: must-revalidate.
>>> If this is not the case IMHO 10.5.3 and 10.5.5 apply.
>>> And only a cache is required to respond with 504 in this case, not a gateway or a proxy. So the cache should
>>> change a 502 to a 504 in case the revalidation fails. Imagine the case where you have other backend modules
>>> as our proxy modules.
>>> So while changing the response to 504 for failed DNS lookups is always correct it is not for other failures.
>>> 10.5.3: The server, while acting as a gateway or proxy, received an invalid response from the upstream server it
>>> accessed in attempting to fulfill the request.
>> 
>> Arbitrarily changing a 502 to a 504 on the fly will confuse people, as this server isn't the only server that could generate a 502, and 502 might be the intended error to be sent to the client.
>> 
>> The way I interpret the RFC is that "received an invalid response" described by 10.5.3 occurs when the upstream has said something that the proxy doesn't like, while the unfortunately named 10.5.5 504 Gateway Timed Out describes a "timely response" which means "at the appropriate time", meaning a network error of some kind.
>> 
>> The change above isolates network errors specifically and returns 504 in those cases only, while leaving 502 for genuine protocol errors, mostly affecting ftp.
>> 
>> Ultimately Roy is the authority on this?
> 
> Good idea. He also knows possible clarifications on that that are work in progress and that we could already apply.
> Roy can you please help us here?

Unfortunately, I am at the tail end of a long standards meeting and
haven't slept much for three days.  Have you checked to see if the
descriptions changed in HTTPbis p6?  RFC2616 hasn't been relevant
for a while now.

http://svn.tools.ietf.org/svn/wg/httpbis/draft-ietf-httpbis/latest/p6-cache.html

I'll look at it tomorrow when I have slept a bit.

....Roy


Re: svn commit: r1480058 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by Ruediger Pluem <rp...@apache.org>.

Graham Leggett wrote:
> On 08 May 2013, at 9:47 AM, Ruediger Pluem <rp...@apache.org> wrote:
> 
>> I don't agree with this. The case you mention is only true if the client sends Cache-Control: must-revalidate.
>> If this is not the case IMHO 10.5.3 and 10.5.5 apply.
>> And only a cache is required to respond with 504 in this case, not a gateway or a proxy. So the cache should
>> change a 502 to a 504 in case the revalidation fails. Imagine the case where you have other backend modules
>> as our proxy modules.
>> So while changing the response to 504 for failed DNS lookups is always correct it is not for other failures.
>> 10.5.3: The server, while acting as a gateway or proxy, received an invalid response from the upstream server it
>> accessed in attempting to fulfill the request.
> 
> Arbitrarily changing a 502 to a 504 on the fly will confuse people, as this server isn't the only server that could generate a 502, and 502 might be the intended error to be sent to the client.
> 
> The way I interpret the RFC is that "received an invalid response" described by 10.5.3 occurs when the upstream has said something that the proxy doesn't like, while the unfortunately named 10.5.5 504 Gateway Timed Out describes a "timely response" which means "at the appropriate time", meaning a network error of some kind.
> 
> The change above isolates network errors specifically and returns 504 in those cases only, while leaving 502 for genuine protocol errors, mostly affecting ftp.
> 
> Ultimately Roy is the authority on this?

Good idea. He also knows possible clarifications on that that are work in progress and that we could already apply.
Roy can you please help us here?

Regards

Rüdiger


Re: svn commit: r1480058 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by Graham Leggett <mi...@sharp.fm>.
On 08 May 2013, at 9:47 AM, Ruediger Pluem <rp...@apache.org> wrote:

> I don't agree with this. The case you mention is only true if the client sends Cache-Control: must-revalidate.
> If this is not the case IMHO 10.5.3 and 10.5.5 apply.
> And only a cache is required to respond with 504 in this case, not a gateway or a proxy. So the cache should
> change a 502 to a 504 in case the revalidation fails. Imagine the case where you have other backend modules
> as our proxy modules.
> So while changing the response to 504 for failed DNS lookups is always correct it is not for other failures.
> 10.5.3: The server, while acting as a gateway or proxy, received an invalid response from the upstream server it
> accessed in attempting to fulfill the request.

Arbitrarily changing a 502 to a 504 on the fly will confuse people, as this server isn't the only server that could generate a 502, and 502 might be the intended error to be sent to the client.

The way I interpret the RFC is that "received an invalid response" described by 10.5.3 occurs when the upstream has said something that the proxy doesn't like, while the unfortunately named 10.5.5 504 Gateway Timed Out describes a "timely response" which means "at the appropriate time", meaning a network error of some kind.

The change above isolates network errors specifically and returns 504 in those cases only, while leaving 502 for genuine protocol errors, mostly affecting ftp.

Ultimately Roy is the authority on this?

Regards,
Graham
--


Re: svn commit: r1480058 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by Ruediger Pluem <rp...@apache.org>.

minfrin@apache.org wrote:
> Author: minfrin
> Date: Tue May  7 20:27:37 2013
> New Revision: 1480058
> 
> URL: http://svn.apache.org/r1480058
> Log:
> mod_proxy: Ensure network errors detected by the proxy are returned as
> 504 Gateway Timout as opposed to 502 Bad Gateway, in order to be
> compliant with RFC2616 14.9.4 Cache Revalidation and Reload Controls.
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c
>     httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>     httpd/httpd/trunk/modules/proxy/proxy_util.c
> 
>

I don't agree with this. The case you mention is only true if the client sends Cache-Control: must-revalidate.
If this is not the case IMHO 10.5.3 and 10.5.5 apply.
And only a cache is required to respond with 504 in this case, not a gateway or a proxy. So the cache should
change a 502 to a 504 in case the revalidation fails. Imagine the case where you have other backend modules
as our proxy modules.
So while changing the response to 504 for failed DNS lookups is always correct it is not for other failures.
10.5.3: The server, while acting as a gateway or proxy, received an invalid response from the upstream server it
accessed in attempting to fulfill the request.

Regards

Rüdiger