On Tuesday, February 14, 2017 4:20:26 PM CET Adam Sampson wrote:
> On Tue, Feb 14, 2017 at 04:03:13PM +0000, Adam Sampson wrote:
> > Your patch compiles, but doesn't fix the broken test. tcpdump traces
> > attached of the test succeeding and failing with the patch applied (it's
> > exactly the same behaviour as before).
>
> If it helps: here's a patch to the test server to add a delay after
> sending the headers -- this makes it always fail (on the second request)
> for me, even on a fast AMD64 machine.

Ok, refreshed my (poor) Python knowledge ;-)

Please try this patch, it works out for me even with your additional timing
(thanks for that !)

It fixes 504 handling in src/url.c and additionally adds Content-Length to
error responses that come with a body.

Regards, Tim
From 3de1306d8e6dacabf2ee101cc1974c93dd7768ac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim Rühsen?= <tim.rueh...@gmx.de>
Date: Tue, 14 Feb 2017 16:20:26 +0100
Subject: [PATCH] Fix 504 status handling

* src/http.c: Move 504 handling to correct place
* testenv/server/http/http_server.py: Add Content-Length header on non-2xx
  status codes with a body

Reported-by: Adam Sampson
---
 src/http.c                         | 20 ++++++++++++++------
 testenv/server/http/http_server.py |  9 +++++----
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/src/http.c b/src/http.c
index 898e1841..788a29ff 100644
--- a/src/http.c
+++ b/src/http.c
@@ -3476,7 +3476,7 @@ gethttp (const struct url *u, struct url *original_url, struct http_stat *hs,

 #ifdef HAVE_METALINK
   /* We need to check for the Metalink data in the very first response
-     we get from the server (before redirectionrs, authorization, etc.).  */
+     we get from the server (before redirections, authorization, etc.).  */
   if (metalink)
     {
       hs->metalink = metalink_from_http (resp, hs, u);
@@ -3496,7 +3496,7 @@ gethttp (const struct url *u, struct url *original_url, struct http_stat *hs,
       uerr_t auth_err = RETROK;
       bool retry;
       /* Normally we are not interested in the response body.
-         But if we are writing a WARC file we are: we like to keep everyting.  */
+         But if we are writing a WARC file we are: we like to keep everything.  */
       if (warc_enabled)
         {
           int _err;
@@ -3556,6 +3556,7 @@ gethttp (const struct url *u, struct url *original_url, struct http_stat *hs,
         pconn.authorized = true;
     }

+/*
   if (statcode == HTTP_STATUS_GATEWAY_TIMEOUT)
     {
       hs->len = 0;
@@ -3568,7 +3569,7 @@ gethttp (const struct url *u, struct url *original_url, struct http_stat *hs,
       retval = GATEWAYTIMEOUT;
       goto cleanup;
     }
-
+*/

   {
     uerr_t ret = check_file_output (u, hs, resp, hdrval, sizeof hdrval);
@@ -3910,8 +3911,8 @@ gethttp (const struct url *u, struct url *original_url, struct http_stat *hs,
               retval = _err;
               goto cleanup;
             }
-          else
-            CLOSE_FINISH (sock);
+
+          CLOSE_FINISH (sock);
         }
       else
         {
@@ -3934,7 +3935,14 @@ gethttp (const struct url *u, struct url *original_url, struct http_stat *hs,
             CLOSE_INVALIDATE (sock);
         }

-      retval = RETRFINISHED;
+      if (statcode == HTTP_STATUS_GATEWAY_TIMEOUT)
+        {
+          /* xfree (hs->message); */
+          retval = GATEWAYTIMEOUT;
+        }
+      else
+        retval = RETRFINISHED;
+
       goto cleanup;
     }

diff --git a/testenv/server/http/http_server.py b/testenv/server/http/http_server.py
index e96f6e82..b222df07 100644
--- a/testenv/server/http/http_server.py
+++ b/testenv/server/http/http_server.py
@@ -204,7 +204,6 @@ class _Handler(BaseHTTPRequestHandler):

     def Response(self, resp_obj):
         self.send_response(resp_obj.response_code)
-        self.finish_headers()
         if resp_obj.response_code == 304:
             raise NoBodyServerError("Conditional get falling to head")
         raise ServerError("Custom Response code sent.")
@@ -329,7 +328,6 @@ class _Handler(BaseHTTPRequestHandler):
         except AuthError as se:
             self.send_response(401, "Authorization Required")
             self.send_challenge(auth_rule.auth_type, auth_rule.auth_parm)
-            self.finish_headers()
             raise se

     def handle_auth(self, auth_rule):
@@ -362,7 +360,6 @@ class _Handler(BaseHTTPRequestHandler):
             if header_recd is None or header_recd != exp_headers[header_line]:
                 self.send_error(400, "Expected Header %s not found" %
                                 header_line)
-                self.finish_headers()
                 raise ServerError("Header " + header_line + " not found")

     def RejectHeader(self, header_obj):
@@ -372,7 +369,6 @@ class _Handler(BaseHTTPRequestHandler):
             if header_recd and header_recd == rej_headers[header_line]:
                 self.send_error(400, 'Blacklisted Header %s received' %
                                 header_line)
-                self.finish_headers()
                 raise ServerError("Header " + header_line + ' received')

     def __log_request(self, method):
@@ -400,6 +396,7 @@ class _Handler(BaseHTTPRequestHandler):

             content = self.server.fileSys.get(path)
             content_length = len(content)
+
             for rule_name in self.rules:
                 try:
                     assert hasattr(self, rule_name)
@@ -410,12 +407,16 @@ class _Handler(BaseHTTPRequestHandler):
                     return(None, None)
                 except AuthError as ae:
                     print(ae.__str__())
+                    self.finish_headers()
                     return(None, None)
                 except NoBodyServerError as nbse:
                     print(nbse.__str__())
+                    self.finish_headers()
                     return(None, None)
                 except ServerError as se:
                     print(se.__str__())
+                    self.add_header("Content-Length", content_length)
+                    self.finish_headers()
                     return(content, None)

             try:
--
2.11.0

Attachment: signature.asc
Description: This is a digitally signed message part.

Reply via email to