On the '1.3.x-r1804008-group' branch: Merge r1804005, r1804008 and
r1804016 from trunk.
The shortlog of these changes is:
r1804005: Fix an edge case in the dechunk bucket where it could erroneously
report APR_EOF instead of an error.
r1804008: Teach the dechunk bucket to return a proper error in presence
of invalid (unparseable) chunk lengths, instead of returning
an APR_EOF.
r1804016: Following up on r1804005, don't return the "response is
truncated" error code for an empty chunk size line.
git-svn-id: https://svn.apache.org/repos/asf/serf/branches/1.3.x-r1804008-group@1805339 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/buckets/dechunk_buckets.c b/buckets/dechunk_buckets.c
index 1a27720..a62ea78 100644
--- a/buckets/dechunk_buckets.c
+++ b/buckets/dechunk_buckets.c
@@ -85,6 +85,7 @@
/* if a line was read, then parse it. */
if (ctx->linebuf.state == SERF_LINEBUF_READY) {
+ char *end;
/* NUL-terminate the line. if it filled the entire buffer,
then just assume the thing is too large. */
if (ctx->linebuf.used == sizeof(ctx->linebuf.line))
@@ -92,10 +93,14 @@
ctx->linebuf.line[ctx->linebuf.used] = '\0';
/* convert from HEX digits. */
- 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. */
diff --git a/test/test_buckets.c b/test/test_buckets.c
index 7d9d473..9d690a9 100644
--- a/test/test_buckets.c
+++ b/test/test_buckets.c
@@ -833,6 +833,74 @@
}
}
+/* 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_bogus_crlf(CuTest *tc)
+{
+ test_baton_t *tb = tc->testBaton;
+ serf_bucket_t *bkt, *tmp;
+ serf_bucket_alloc_t *alloc = serf_bucket_allocator_create(tb->pool, NULL,
+ NULL);
+
+ 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_BAD_HTTP_RESPONSE, status);
+ }
+
+ /* This will also destroy response stream bucket. */
+ 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 = serf_bucket_allocator_create(tb->pool, NULL,
+ NULL);
+
+ 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)
{
apr_pool_t *test_pool = tc->testBaton;
@@ -959,7 +1027,7 @@
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";
@@ -1614,6 +1682,8 @@
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_bogus_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_no_reason);
SUITE_ADD_TEST(suite, test_bucket_header_set);
