serf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From i...@apache.org
Subject svn commit: r1808211 - in /serf/branches/1.3.x: ./ STATUS buckets/dechunk_buckets.c test/test_auth.c test/test_buckets.c test/test_context.c
Date Wed, 13 Sep 2017 08:40:39 GMT
Author: ivan
Date: Wed Sep 13 08:40:39 2017
New Revision: 1808211

URL: http://svn.apache.org/viewvc?rev=1808211&view=rev
Log:
Merge:
 * r1804005, r1804008, r1804016
   Return an error for invalid chunk lengths in the dechunk bucket.
   Justification:
     Invalid chunk lengths should cause an error, as otherwise the data
     could be incorrectly interpreted further down the line.
   Branch:
     ^/serf/branches/1.3.x-r1804008-group
   Votes:
     +1: kotkov, ivan, rhuijben

Modified:
    serf/branches/1.3.x/   (props changed)
    serf/branches/1.3.x/STATUS
    serf/branches/1.3.x/buckets/dechunk_buckets.c
    serf/branches/1.3.x/test/test_auth.c
    serf/branches/1.3.x/test/test_buckets.c
    serf/branches/1.3.x/test/test_context.c

Propchange: serf/branches/1.3.x/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Sep 13 08:40:39 2017
@@ -1,4 +1,5 @@
 /serf/branches/1.3.x-fix-outgoing-request-err:1804540-1808208
+/serf/branches/1.3.x-r1804008-group:1805337-1808209
 /serf/branches/multiple_ssl_impls:1699382
 /serf/branches/windows-sspi:1698866-1698877
-/serf/trunk:1699516-1699518,1699520-1699522,1699528,1699530-1699535,1699537,1699539-1699543,1699548-1699549,1699553,1699555-1699556,1699559-1699560,1699563-1699565,1699567-1699570,1699572-1699573,1699578-1699580,1699582-1699597,1699599-1699602,1699607,1699610,1699613,1699615-1699618,1699622-1699623,1699626-1699627,1699633,1699637,1699642,1699645,1699647,1699649-1699650,1699652,1699654-1699655,1699659-1699665,1699671,1699674,1699680-1699683,1699687-1699688,1699690,1699692-1699694,1699698-1699700,1699702,1699707-1699708,1699712-1699716,1699720,1699724,1699728,1699730,1699733,1699762,1699770,1699773,1699777,1699780-1699781,1699791,1699798,1699800-1699801,1699817,1699819,1699838,1699843,1699846,1699850,1699852,1699858-1699859,1699861,1699873,1699881,1699884,1699902-1699903,1699906,1699924,1699926-1699927,1699930,1699932,1699936-1699937,1699941,1699944,1699948-1699950,1699954,1699957,1699964,1699973,1699975,1699985-1699987,1699993-1699994,1700031,1700062,1700128,1700149,1700234,1700236,1
 700246,1700270,1700650,1700830,1702096,1702221,1702264,1703624,1704725,1708849,1709155-1709156,1709296,1748673,1757829,1758190,1758193,1804534
+/serf/trunk:1699516-1699518,1699520-1699522,1699528,1699530-1699535,1699537,1699539-1699543,1699548-1699549,1699553,1699555-1699556,1699559-1699560,1699563-1699565,1699567-1699570,1699572-1699573,1699578-1699580,1699582-1699597,1699599-1699602,1699607,1699610,1699613,1699615-1699618,1699622-1699623,1699626-1699627,1699633,1699637,1699642,1699645,1699647,1699649-1699650,1699652,1699654-1699655,1699659-1699665,1699671,1699674,1699680-1699683,1699687-1699688,1699690,1699692-1699694,1699698-1699700,1699702,1699707-1699708,1699712-1699716,1699720,1699724,1699728,1699730,1699733,1699762,1699770,1699773,1699777,1699780-1699781,1699791,1699798,1699800-1699801,1699817,1699819,1699838,1699843,1699846,1699850,1699852,1699858-1699859,1699861,1699873,1699881,1699884,1699902-1699903,1699906,1699924,1699926-1699927,1699930,1699932,1699936-1699937,1699941,1699944,1699948-1699950,1699954,1699957,1699964,1699973,1699975,1699985-1699987,1699993-1699994,1700031,1700062,1700128,1700149,1700234,1700236,1
 700246,1700270,1700650,1700830,1702096,1702221,1702264,1703624,1704725,1708849,1709155-1709156,1709296,1748673,1757829,1758190,1758193,1804005,1804008,1804016,1804534

Modified: serf/branches/1.3.x/STATUS
URL: http://svn.apache.org/viewvc/serf/branches/1.3.x/STATUS?rev=1808211&r1=1808210&r2=1808211&view=diff
==============================================================================
--- serf/branches/1.3.x/STATUS (original)
+++ serf/branches/1.3.x/STATUS Wed Sep 13 08:40:39 2017
@@ -30,16 +30,6 @@ Veto-blocked changes:
 Approved changes:
 =================
 
- * r1804005, r1804008, r1804016
-   Return an error for invalid chunk lengths in the dechunk bucket.
-   Justification:
-     Invalid chunk lengths should cause an error, as otherwise the data
-     could be incorrectly interpreted further down the line.
-   Branch:
-     ^/serf/branches/1.3.x-r1804008-group
-   Votes:
-     +1: kotkov, ivan, rhuijben
-
  * r1805301
    Fix an endless loop in the deflate bucket with the truncated input.
    Justification:

Modified: serf/branches/1.3.x/buckets/dechunk_buckets.c
URL: http://svn.apache.org/viewvc/serf/branches/1.3.x/buckets/dechunk_buckets.c?rev=1808211&r1=1808210&r2=1808211&view=diff
==============================================================================
--- serf/branches/1.3.x/buckets/dechunk_buckets.c (original)
+++ serf/branches/1.3.x/buckets/dechunk_buckets.c Wed Sep 13 08:40:39 2017
@@ -85,6 +85,7 @@ static apr_status_t serf_dechunk_read(se
 
             /* 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 @@ static apr_status_t serf_dechunk_read(se
                 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. */

Modified: serf/branches/1.3.x/test/test_auth.c
URL: http://svn.apache.org/viewvc/serf/branches/1.3.x/test/test_auth.c?rev=1808211&r1=1808210&r2=1808211&view=diff
==============================================================================
--- serf/branches/1.3.x/test/test_auth.c (original)
+++ serf/branches/1.3.x/test/test_auth.c Wed Sep 13 08:40:39 2017
@@ -53,7 +53,6 @@ static void test_authentication_disabled
             "Transfer-Encoding: chunked" CRLF
             "WWW-Authenticate: Basic realm=""Test Suite""" CRLF
             CRLF
-            "1" CRLF CRLF
             "0" CRLF CRLF},
     };
     apr_status_t status;
@@ -96,7 +95,6 @@ static void test_unsupported_authenticat
             "Transfer-Encoding: chunked" CRLF
             "WWW-Authenticate: NotExistent realm=""Test Suite""" CRLF
             CRLF
-            "1" CRLF CRLF
             "0" CRLF CRLF},
     };
     apr_status_t status;
@@ -195,7 +193,6 @@ static void basic_authentication(CuTest
         "www-Authenticate: bAsIc realm=""Test Suite""" CRLF
         "%s"
         CRLF
-        "1" CRLF CRLF
         "0" CRLF CRLF, resp_hdrs);
     action_list[1].kind = SERVER_RESPOND;
     action_list[1].text = CHUNKED_EMPTY_RESPONSE;
@@ -315,7 +312,6 @@ static void digest_authentication(CuTest
         "algorithm=\"MD5\",qop-options=\"auth\"" CRLF
         "%s"
         CRLF
-        "1" CRLF CRLF
         "0" CRLF CRLF, resp_hdrs);
     /* If the resp_hdrs includes "Connection: close", serf will automatically
        reset the connection from the client side, no need to use 
@@ -457,7 +453,6 @@ static void authentication_switch_realms
         "Transfer-Encoding: chunked" CRLF
         "WWW-Authenticate: %s realm=""Test Suite""%s" CRLF
         CRLF
-        "1" CRLF CRLF
         "0" CRLF CRLF, scheme, authn_attr);
     action_list[1].kind = SERVER_RESPOND;
     action_list[1].text = CHUNKED_EMPTY_RESPONSE;
@@ -469,7 +464,6 @@ static void authentication_switch_realms
         "Transfer-Encoding: chunked" CRLF
         "WWW-Authenticate: %s realm=""New Realm""%s" CRLF
         CRLF
-        "1" CRLF CRLF
         "0" CRLF CRLF, scheme, authn_attr);
     action_list[4].kind = SERVER_RESPOND;
     action_list[4].text = CHUNKED_EMPTY_RESPONSE;

Modified: serf/branches/1.3.x/test/test_buckets.c
URL: http://svn.apache.org/viewvc/serf/branches/1.3.x/test/test_buckets.c?rev=1808211&r1=1808210&r2=1808211&view=diff
==============================================================================
--- serf/branches/1.3.x/test/test_buckets.c (original)
+++ serf/branches/1.3.x/test/test_buckets.c Wed Sep 13 08:40:39 2017
@@ -833,6 +833,74 @@ static void test_response_body_chunked_g
     }
 }
 
+/* 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 @@ 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";
@@ -1614,6 +1682,8 @@ 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_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);

Modified: serf/branches/1.3.x/test/test_context.c
URL: http://svn.apache.org/viewvc/serf/branches/1.3.x/test/test_context.c?rev=1808211&r1=1808210&r2=1808211&view=diff
==============================================================================
--- serf/branches/1.3.x/test/test_context.c (original)
+++ serf/branches/1.3.x/test/test_context.c Wed Sep 13 08:40:39 2017
@@ -1918,7 +1918,6 @@ static void test_ssltunnel_no_creds_cb(C
             "Transfer-Encoding: chunked" CRLF
             "Proxy-Authenticate: Basic realm=""Test Suite Proxy""" CRLF
             CRLF
-            "1" CRLF CRLF
             "0" CRLF CRLF},
     };
 
@@ -2034,7 +2033,6 @@ static void ssltunnel_basic_auth(CuTest
         "Proxy-Authenticate: Basic realm=""Test Suite Proxy""" CRLF
         "%s"
         CRLF
-        "1" CRLF CRLF
         "0" CRLF CRLF, proxy_407_resp_hdrs);
     action_list_proxy[1].kind = SERVER_RESPOND;
     action_list_proxy[1].text = apr_psprintf(test_pool,
@@ -2043,7 +2041,6 @@ static void ssltunnel_basic_auth(CuTest
         "Proxy-Authenticate: Basic realm=""Test Suite Proxy""" CRLF
         "%s"
         CRLF
-        "1" CRLF CRLF
         "0" CRLF CRLF, proxy_407_resp_hdrs);
 
     action_list_proxy[2].kind = SERVER_RESPOND;
@@ -2053,7 +2050,6 @@ static void ssltunnel_basic_auth(CuTest
         "Proxy-Authenticate: Basic realm=""Test Suite Proxy""" CRLF
         "%s"
         CRLF
-        "1" CRLF CRLF
         "0" CRLF CRLF, proxy_407_resp_hdrs);
 
     action_list_proxy[3].kind = SERVER_RESPOND;
@@ -2099,7 +2095,6 @@ static void ssltunnel_basic_auth(CuTest
         "WWW-Authenticate: Basic realm=""Test Suite""" CRLF
         "%s"
         CRLF
-        "1" CRLF CRLF
         "0" CRLF CRLF, server_resp_hdrs);
     action_list_server[1].kind = SERVER_RESPOND;
     action_list_server[1].text = CHUNKED_EMPTY_RESPONSE;
@@ -2244,7 +2239,6 @@ static void test_ssltunnel_digest_auth(C
             "nonce=\"ABCDEF1234567890\",opaque=\"myopaque\","
             "algorithm=\"MD5\",qop-options=\"auth\"" CRLF
             CRLF
-            "1" CRLF CRLF
             "0" CRLF CRLF},
         {SERVER_RESPOND, CHUNKED_EMPTY_RESPONSE},
         /* Forward the remainder of the data to the server without validation */



Mime
View raw message