Author: kotkov Date: Thu Aug 3 15:06:09 2017 New Revision: 1804008 URL: http://svn.apache.org/viewvc?rev=1804008&view=rev Log: Teach the dechunk bucket to return a proper error in presence of invalid (unparseable) chunk lengths, instead of returning an APR_EOF.
* buckets/dechunk_buckets.c (wait_for_chunk): Check if we were actually able to parse the chunk size with apr_strtoi64(). If not, bail out. * test/test_buckets.c (test_response_body_chunked_invalid_len): New test. (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=1804008&r1=1804007&r2=1804008&view=diff ============================================================================== --- serf/trunk/buckets/dechunk_buckets.c (original) +++ serf/trunk/buckets/dechunk_buckets.c Thu Aug 3 15:06:09 2017 @@ -82,6 +82,7 @@ static apr_status_t wait_for_chunk(serf_ /* if a line was read, then parse it. */ if (ctx->linebuf.state == SERF_LINEBUF_READY) { + char *end; /* Do we have the chunk length? */ if (ctx->linebuf.line[0] == '\0') { @@ -89,10 +90,14 @@ static apr_status_t wait_for_chunk(serf_ } /* Convert from HEX digits. The linebuffer ensures a '\0' */ - ctx->body_left = apr_strtoi64(ctx->linebuf.line, NULL, 16); + ctx->body_left = apr_strtoi64(ctx->linebuf.line, &end, 16); if (errno == ERANGE) { return APR_FROM_OS_ERROR(ERANGE); } + else if (ctx->linebuf.line == end) { + /* Invalid chunk length, bail out. */ + return SERF_ERROR_BAD_HTTP_RESPONSE; + } if (ctx->body_left == 0) { /* Just read the last-chunk marker. We're DONE. */ Modified: serf/trunk/test/test_buckets.c URL: http://svn.apache.org/viewvc/serf/trunk/test/test_buckets.c?rev=1804008&r1=1804007&r2=1804008&view=diff ============================================================================== --- serf/trunk/test/test_buckets.c (original) +++ serf/trunk/test/test_buckets.c Thu Aug 3 15:06:09 2017 @@ -1156,6 +1156,38 @@ static void test_response_body_chunked_t serf_bucket_destroy(bkt); } +static void test_response_body_chunked_invalid_len(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 + "invalid" 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_BAD_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; @@ -3219,6 +3251,7 @@ CuSuite *test_buckets(void) 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_body_chunked_invalid_len); 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);