Re: [PATCH] Serf crash on spurious data between responses

Philip Martin <philip.martin_at_wandisco.com> writes:

> Philip Martin <philip.martin_at_wandisco.com> writes:
>
>> While debugging a problem reported on users I accidentally sent an extra
>> byte to the client: I sent Content-Length of N and then sent N+1 bytes.
>> The first N bytes made a valid response, so serf was happy at that
>> stage. When processing the next request the extra byte causes serf to
>> attempt to handle the spurious data before any request handler is setup.
>> This is with serf 1.3.x_at_2440.
>
> Here's a patch that causes serf to return an error if this sort of
> spurious data is received:
>
> Index: outgoing.c
> ===================================================================
> --- outgoing.c (revision 2445)
> +++ outgoing.c (working copy)
> @@ -1109,7 +1109,11 @@ static apr_status_t read_from_connection(serf_conn
> goto error;
> }
>
> - /* Unexpected response from the server */
> + /* Unexpected response from the server. This can happen if
> + * a buggy server sends more than Content-Length data.
> + */
> + status = SERF_ERROR_BAD_HTTP_RESPONSE;
> + goto error;
>
> }

That breaks HTTPS negotiation. This works:

Index: outgoing.c
===================================================================
--- outgoing.c (revision 2445)
+++ outgoing.c (working copy)
@@ -1109,8 +1109,14 @@
                 goto error;
             }
 
- /* Unexpected response from the server */
-
+ if (!request->req_bkt) {
+ /* Unexpected response from the server. This can happen if
+ * a buggy server sends more than Content-Length data.
+ */
+ status = SERF_ERROR_BAD_HTTP_RESPONSE;
+ goto error;
+ }
+
         }
 
         /* If the request doesn't have a response bucket, then call the

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*