Author: rhuijben
Date: Thu Sep 17 13:25:14 2015
New Revision: 1703618

URL: http://svn.apache.org/r1703618
Log:
Merge the r1699985 group from trunk:

 * r1699985, r1699993, r1699994
   Improve status line verifications
   Justification:
     Avoids handling bad data as a real status line, stalling the
     connection until timeout.
   Votes:
     +1: rhuijben, ivan

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

Propchange: serf/branches/1.3.x/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Sep 17 13:25:14 2015
@@ -1,4 +1,4 @@
 /serf/branches/1.3.x:1699925,1699931
 /serf/branches/multiple_ssl_impls:1699382
 /serf/branches/windows-sspi:1698866-1698877
-/serf/trunk:1699516-1699518,1699520-1699522,1699528,1699530-1699535,1699537,1699539-1699541,1699543,1699548-1699549,1699553,1699555-1699556,1699559-1699560,1699563-1699565,1699567-1699570,1699572-1699573,1699578-1699580,1699582-1699597,1699599-1699602,1699607,1699610,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,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,1700062,1700128,1700149,1700234,1700236,1700246,1700270,1700650,1700830,1702096,1702221,1
 702264
+/serf/trunk:1699516-1699518,1699520-1699522,1699528,1699530-1699535,1699537,1699539-1699541,1699543,1699548-1699549,1699553,1699555-1699556,1699559-1699560,1699563-1699565,1699567-1699570,1699572-1699573,1699578-1699580,1699582-1699597,1699599-1699602,1699607,1699610,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,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,1699993-1699994,1700062,1700128,1700149,1700234,1700236,1700246,1700270,1700650,1
 700830,1702096,1702221,1702264

Modified: serf/branches/1.3.x/STATUS
URL: 
http://svn.apache.org/viewvc/serf/branches/1.3.x/STATUS?rev=1703618&r1=1703617&r2=1703618&view=diff
==============================================================================
--- serf/branches/1.3.x/STATUS (original)
+++ serf/branches/1.3.x/STATUS Thu Sep 17 13:25:14 2015
@@ -19,14 +19,6 @@ Candidate changes:
    Votes:
      +1: rhuijben
 
- * r1699985, r1699993, r1699994
-   Improve status line verifications
-   Justification:
-     Avoids handling bad data as a real status line, stalling the
-     connection until timeout.
-   Votes:
-     +1: rhuijben, ivan
-
  * r1699542
    Explicitly use ANSI version of SSPI.
    Justification:

Modified: serf/branches/1.3.x/buckets/response_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/branches/1.3.x/buckets/response_buckets.c?rev=1703618&r1=1703617&r2=1703618&view=diff
==============================================================================
--- serf/branches/1.3.x/buckets/response_buckets.c (original)
+++ serf/branches/1.3.x/buckets/response_buckets.c Thu Sep 17 13:25:14 2015
@@ -133,7 +133,17 @@ static apr_status_t parse_status_line(re
     int res;
     char *reason; /* ### stupid APR interface makes this non-const */
 
-    /* ctx->linebuf.line should be of form: HTTP/1.1 200 OK */
+    /* Ensure a valid length, to avoid overflow on the final '\0' */
+    if (ctx->linebuf.used >= SERF_LINEBUF_LIMIT) {
+       return SERF_ERROR_BAD_HTTP_RESPONSE;
+    }
+
+    /* apr_date_checkmask assumes its arguments are valid C strings */
+    ctx->linebuf.line[ctx->linebuf.used] = '\0';
+
+    /* ctx->linebuf.line should be of form: 'HTTP/1.1 200 OK',
+       but we also explicitly allow the forms 'HTTP/1.1 200' (no reason)
+       and 'HTTP/1.1 401.1 Logon failed' (iis extended error codes) */
     res = apr_date_checkmask(ctx->linebuf.line, "HTTP/#.# ###*");
     if (!res) {
         /* Not an HTTP response?  Well, at least we won't understand it. */

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=1703618&r1=1703617&r2=1703618&view=diff
==============================================================================
--- serf/branches/1.3.x/test/test_buckets.c (original)
+++ serf/branches/1.3.x/test/test_buckets.c Thu Sep 17 13:25:14 2015
@@ -989,6 +989,35 @@ static void test_linebuf_crlf_split(CuTe
     CuAssert(tc, "Read less data than expected.", strlen(expected) == 0);
 }
 
+/* Test handling responses without a reason by response buckets. */
+static void test_response_bucket_no_reason(CuTest *tc)
+{
+    test_baton_t *tb = tc->testBaton;
+    serf_bucket_t *bkt, *tmp;
+    serf_status_line sline;
+    serf_bucket_alloc_t *alloc = serf_bucket_allocator_create(tb->pool, NULL,
+                                                              NULL);
+
+    tmp = SERF_BUCKET_SIMPLE_STRING("HTTP/1.1 401" CRLF
+                                    "Content-Type: text/plain" CRLF
+                                    "Content-Length: 2" CRLF
+                                    CRLF
+                                    "AB",
+                                    alloc);
+
+    bkt = serf_bucket_response_create(tmp, alloc);
+
+    read_and_check_bucket(tc, bkt, "AB");
+
+    serf_bucket_response_status(bkt, &sline);
+    CuAssertTrue(tc, sline.version == SERF_HTTP_11);
+    CuAssertIntEquals(tc, 401, sline.code);
+
+    /* Probably better to have just "Logon failed" as reason. But current
+       behavior is also acceptable.*/
+    CuAssertStrEquals(tc, "", sline.reason);
+}
+
 /* Test that serf can handle lines that don't arrive completely in one go.
    It doesn't really run random, it tries inserting APR_EAGAIN in all possible
    places in the response message, only one currently. */
@@ -1586,6 +1615,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_bucket_peek_at_headers);
+    SUITE_ADD_TEST(suite, test_response_bucket_no_reason);
     SUITE_ADD_TEST(suite, test_bucket_header_set);
     SUITE_ADD_TEST(suite, test_iovec_buckets);
     SUITE_ADD_TEST(suite, test_aggregate_buckets);


Reply via email to