[PATCH] ra_serf infinite loop with mod_deflate

Philip Martin <philip.martin_at_wandisco.com> writes:

> Philip Martin <philip_at_codematters.co.uk> writes:
>
>> Philip Martin <philip.martin_at_wandisco.com> writes:
>>
>>> handle_fetch (request=0x7ffff3e56038, response=0x7ffff3e918b8,
>>> handler_baton=0x7ffff3e611e0, pool=0x7ffff3e6d028)
>>> at ../src/subversion/libsvn_ra_serf/update.c:1152
>>> 1152 if (SERF_BUCKET_READ_ERROR(status))
>>> (gdb) p status
>>> $13 = 0
>>
>> So that's what happens when I interrupt the infinite loop, i.e. long
>> after the first failure to read from the socket. If I catch the first
>> socket read error then serf does supply an error to ra_serf:
>
> I can reproduce over plain HTTP without mod_ssl -
>
> - the closed socket causes apr_socket_recv() to returns APR_EOF
> - APR_EOF gets returned to serf_deflate_read()
> - APR_EOF gets stored in deflate_context_t->stream_status
> - serf_deflate_read() returns APR_SUCCESS
> - serf_response_read() returns APR_SUCCESS

I'm using "apachectl -k stop" to stop apache and this causes the output
to end between the chunks of a chunked response. So the state machine
in serf_dechunk_read is in state STATE_SIZE and the EOF looks normal.

In deflate_buckets.c:serf_deflate_read the APR_EOF is returned by the
serf_bucket_read call at line 255 (1.3.x branch). At this point the
ctx->stream_status is APR_EOF and private_len is zero and we can use
these conditions to return SERF_ERROR_TRUNCATED_HTTP_RESPONSE:

Index: buckets/deflate_buckets.c
===================================================================
--- buckets/deflate_buckets.c (revision 2445)
+++ buckets/deflate_buckets.c (working copy)
@@ -261,6 +261,11 @@
                     return ctx->stream_status;
                 }
 
+ if (!private_len && APR_STATUS_IS_EOF(ctx->stream_status)) {
+ *len = 0;
+ return SERF_ERROR_TRUNCATED_HTTP_RESPONSE;
+ }
+
                 if (!private_len && APR_STATUS_IS_EAGAIN(ctx->stream_status)) {
                     *len = 0;
                     status = ctx->stream_status;

With that patch the infinte loop is avoided:

A wc/A
../src2/subversion/svn/checkout-cmd.c:175,
../src2/subversion/libsvn_client/checkout.c:226,
../src2/subversion/libsvn_client/checkout.c:193,
../src2/subversion/libsvn_client/update.c:668,
../src2/subversion/libsvn_client/update.c:508,
../src2/subversion/libsvn_wc/adm_crawler.c:856,
../src2/subversion/libsvn_ra_serf/update.c:2693,
../src2/subversion/libsvn_ra_serf/update.c:2683,
../src2/subversion/libsvn_ra_serf/update.c:2613,
../src2/subversion/libsvn_ra_serf/util.c:895,
../src2/subversion/libsvn_ra_serf/util.c:1422,
../src2/subversion/libsvn_ra_serf/util.c:1403,
../src2/subversion/libsvn_ra_serf/update.c:1154: (apr_err=120106)
svn: E120106: ra_serf: The server sent a truncated HTTP response body.
[Inferior 1 (process 28858) exited with code 01]

The problem also occurs with serf trunk and the same patch applies.

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