Author: kotkov
Date: Thu Aug 3 15:01:21 2017
New Revision: 1804005
URL: http://svn.apache.org/viewvc?rev=1804005&view=rev
Log:
Fix an edge case in the dechunk bucket where it could erroneously report
APR_EOF instead of an error in presence of a truncated response with
CRLF in place of the next chunk length.
* buckets/dechunk_buckets.c
(wait_for_chunk): Check if we got an empty chunk size line before trying
to parse it with apr_strtoi64(). Otherwise this function would report 0,
which we would then incorrectly interpret as the final zero-sized chunk.
* test/test_buckets.c
(test_response_body_chunked_truncated_with_crlf): New test, modelled
after what httpd + mod_dav + mod_dav_svn can actually send when
aborting connection due to an internal error.
(test_linebuf_crlf_split): Update the expectation in this test, as the
response in it didn't include the final zero-sized chunk. That's ill-
formed per RFC7230 3.3.3 and 4.1.
(test_buckets): Add new test.
Modified:
serf/trunk/buckets/dechunk_buckets.c
serf/trunk/test/test_buckets.c
Modified: serf/trunk/buckets/dechunk_buckets.c
URL:
http://svn.apache.org/viewvc/serf/trunk/buckets/dechunk_buckets.c?rev=1804005&r1=1804004&r2=1804005&view=diff
==============================================================================
--- serf/trunk/buckets/dechunk_buckets.c (original)
+++ serf/trunk/buckets/dechunk_buckets.c Thu Aug 3 15:01:21 2017
@@ -83,6 +83,11 @@ static apr_status_t wait_for_chunk(serf_
/* if a line was read, then parse it. */
if (ctx->linebuf.state == SERF_LINEBUF_READY) {
+ /* Do we have the chunk length? */
+ if (ctx->linebuf.line[0] == '\0') {
+ return SERF_ERROR_TRUNCATED_HTTP_RESPONSE;
+ }
+
/* Convert from HEX digits. The linebuffer ensures a '\0' */
ctx->body_left = apr_strtoi64(ctx->linebuf.line, NULL, 16);
if (errno == ERANGE) {
Modified: serf/trunk/test/test_buckets.c
URL:
http://svn.apache.org/viewvc/serf/trunk/test/test_buckets.c?rev=1804005&r1=1804004&r2=1804005&view=diff
==============================================================================
--- serf/trunk/test/test_buckets.c (original)
+++ serf/trunk/test/test_buckets.c Thu Aug 3 15:01:21 2017
@@ -1122,6 +1122,40 @@ static void test_response_body_chunked_g
serf_bucket_destroy(bkt);
}
+/* Test for issue: the server aborts the connection and also sends
+ a bogus CRLF in place of the expected chunk size. Test that we get
+ a decent error code from the response bucket instead of APR_EOF. */
+static void test_response_body_chunked_truncated_with_crlf(CuTest *tc)
+{
+ test_baton_t *tb = tc->testBaton;
+ serf_bucket_t *bkt, *tmp;
+ serf_bucket_alloc_t *alloc = test__create_bucket_allocator(tc, tb->pool);
+
+ tmp = SERF_BUCKET_SIMPLE_STRING("HTTP/1.1 200 OK" CRLF
+ "Content-Type: text/plain" CRLF
+ "Transfer-Encoding: chunked" CRLF
+ CRLF
+ "2" CRLF
+ "AB" CRLF
+ CRLF,
+ alloc);
+
+ bkt = serf_bucket_response_create(tmp, alloc);
+
+ {
+ char buf[1024];
+ apr_size_t len;
+ apr_status_t status;
+
+ status = read_all(bkt, buf, sizeof(buf), &len);
+
+ CuAssertIntEquals(tc, SERF_ERROR_TRUNCATED_HTTP_RESPONSE, status);
+ }
+
+ /* This will also destroy response stream bucket. */
+ serf_bucket_destroy(bkt);
+}
+
static void test_response_bucket_peek_at_headers(CuTest *tc)
{
test_baton_t *tb = tc->testBaton;
@@ -1298,7 +1332,7 @@ static void test_linebuf_crlf_split(CuTe
CRLF, APR_SUCCESS },
{ 1, "6" CR, APR_SUCCESS },
{ 1, "", APR_EAGAIN },
- { 1, LF "blabla" CRLF CRLF, APR_SUCCESS }, };
+ { 1, LF "blabla" CRLF "0" CRLF CRLF, APR_SUCCESS }, };
apr_status_t status;
const char *expected = "blabla";
@@ -3184,6 +3218,7 @@ CuSuite *test_buckets(void)
SUITE_ADD_TEST(suite, test_response_body_chunked_no_crlf);
SUITE_ADD_TEST(suite, test_response_body_chunked_incomplete_crlf);
SUITE_ADD_TEST(suite, test_response_body_chunked_gzip_small);
+ SUITE_ADD_TEST(suite, test_response_body_chunked_truncated_with_crlf);
SUITE_ADD_TEST(suite, test_response_bucket_peek_at_headers);
SUITE_ADD_TEST(suite, test_response_bucket_iis_status_code);
SUITE_ADD_TEST(suite, test_response_bucket_no_reason);
