Re: [PATCH] Fix variable setting in the local interface

On 12 Nov 2012, at 15:47, Gabriel Kerneis wrote:
> This patch fixes two bugs in httpSpecialClientSideHandler (local.c).
> 
> Setting variables in the local interface returns "503 Post aborted" when the
> POST request cannot be read at once.  This function httpSpecialClientSideHandler
> is called to handle remaining data, but it assumes incorrectly that the object
> flag is OBJECT_INPROGRESS (and therefore fails); the flag is in fact
> OBJECT_INITIAL.
> 
> Another bug, hidden by the previous one, is that httpSpecialClientSideHandler
> always return "502 Incomplete client request", even when the request is complete
> and handled correctly by httpSpecialDoSide.
> ---
> local.c |    4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/local.c b/local.c
> index 82106c4..9be0321 100644
> --- a/local.c
> +++ b/local.c
> @@ -361,7 +361,7 @@ httpSpecialClientSideHandler(int status,
>     int push;
> 
>     if((request->object->flags & OBJECT_ABORTED) || 
> -       !(request->object->flags & OBJECT_INPROGRESS)) {
> +       !(request->object->flags & OBJECT_INITIAL)) {
>         httpClientDiscardBody(connection);
>         httpClientError(request, 503, internAtom("Post aborted"));
>         return 1;
> @@ -380,7 +380,7 @@ httpSpecialClientSideHandler(int status,
>                connection->bodylen - connection->reqoffset);
>     if(push > 0) {
>         connection->reqlen += push;
> -        httpSpecialDoSide(request);
> +        return httpSpecialDoSide(request);
>     }
> 
>     do_log(L_ERROR, "Incomplete client request.\n");
> -- 
> 1.7.10.4


I can't comment on the code changes, but here are the effects on my test cases:

+ Safari now works correctly, every time.

+ Firefox continues to work correctly, every time.

+ wget --post-data=clientTimeout=3m http://localhost:8123/polipo/config -O-
  now works correctly, every time. It used to fail intermittently.

+ curl -F "clientTimeout=3m" http://localhost:8123/polipo/config
  now hangs waiting for a response, every time. Before this patch it used
  to receive the "503 Post aborted", every time.
  I don't know if this is caused by your change, or whether the bug you
  fixed was previously masking this problem.

With curl, I *once* got the following error from polipo when I killed the
curl process:

    Incomplete client request.
    Assertion failed: (!object->chunks[i].locked), function destroyObject, file object.c, line 661.
    Abort trap: 6           ./polipo

Below is some debug output from polipo & curl when curl hangs waiting for
a response.

Thanks,
Dave.

--

$ ./polipo logLevel=0xFFFFCFFF 2>&1 | sed 's/^/polipo: /' &
polipo: Disabling disk cache: No such file or directory
polipo: Disabling local tree: No such file or directory
polipo: Established listening socket on port 8123.

$ curl -F "clientTimeout=3m" http://localhost:8123/polipo/config --trace -
== Info: About to connect() to localhost port 8123 (#0)
== Info:   Trying ::1...
== Info: Connection refused
== Info:   Trying 127.0.0.1...
== Info: connected
== Info: Connected to localhost (127.0.0.1) port 8123 (#0)
polipo: Accepted client connection 0x10ea03b80
=> Send header, 221 bytes (0xdd)
0000: 50 4f 53 54 20 2f 70 6f 6c 69 70 6f 2f 63 6f 6e POST /polipo/con
0010: 66 69 67 20 48 54 54 50 2f 31 2e 31 0d 0a 55 73 fig HTTP/1.1..Us
0020: 65 72 2d 41 67 65 6e 74 3a 20 63 75 72 6c 2f 37 er-Agent: curl/7
0030: 2e 32 38 2e 30 0d 0a 48 6f 73 74 3a 20 6c 6f 63 .28.0..Host: loc
0040: 61 6c 68 6f 73 74 3a 38 31 32 33 0d 0a 41 63 63 alhost:8123..Acc
0050: 65 70 74 3a 20 2a 2f 2a 0d 0a 43 6f 6e 74 65 6e ept: */*..Conten
0060: 74 2d 4c 65 6e 67 74 68 3a 20 31 35 30 0d 0a 45 t-Length: 150..E
0070: 78 70 65 63 74 3a 20 31 30 30 2d 63 6f 6e 74 69 xpect: 100-conti
0080: 6e 75 65 0d 0a 43 6f 6e 74 65 6e 74 2d 54 79 70 nue..Content-Typ
0090: 65 3a 20 6d 75 6c 74 69 70 61 72 74 2f 66 6f 72 e: multipart/for
00a0: 6d 2d 64 61 74 61 3b 20 62 6f 75 6e 64 61 72 79 m-data; boundary
00b0: 3d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d =---------------
00c0: 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 32 61 30 -------------2a0
polipo: Client request: POST /polipo/config HTTP/1.1
00d0: 64 39 38 61 64 39 66 35 32 0d 0a 0d 0a          d98ad9f52....
== Info: Done waiting for 100-continue
=> Send data, 150 bytes (0x96)
0000: 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d ----------------
0010: 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 32 61 --------------2a
0020: 30 64 39 38 61 64 39 66 35 32 0d 0a 43 6f 6e 74 0d98ad9f52..Cont
0030: 65 6e 74 2d 44 69 73 70 6f 73 69 74 69 6f 6e 3a ent-Disposition:
0040: 20 66 6f 72 6d 2d 64 61 74 61 3b 20 6e 61 6d 65  form-data; name
0050: 3d 22 63 6c 69 65 6e 74 54 69 6d 65 6f 75 74 22 ="clientTimeout"
0060: 0d 0a 0d 0a 33 6d 0d 0a 2d 2d 2d 2d 2d 2d 2d 2d ....3m..--------
0070: 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d ----------------
0080: 2d 2d 2d 2d 2d 2d 32 61 30 64 39 38 61 64 39 66 ------2a0d98ad9f
0090: 35 32 2d 2d 0d 0a                               52--..
^C



------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_nov