8195138: The asynchronous Http1HeaderParser doesn't handle all line folds correctly
authordfuchs
Tue, 16 Jan 2018 19:19:03 +0000
changeset 48535 5f9977540ac9
parent 48534 12d9ff9e0a4b
child 48536 d7995ed9627d
8195138: The asynchronous Http1HeaderParser doesn't handle all line folds correctly Reviewed-by: chegar
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http1HeaderParser.java
test/jdk/java/net/httpclient/HeadersTest1.java
test/jdk/java/net/httpclient/whitebox/Http1HeaderParserTestDriver.java
test/jdk/java/net/httpclient/whitebox/jdk.incubator.httpclient/jdk/incubator/http/Http1HeaderParserTest.java
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http1HeaderParser.java	Tue Jan 16 10:48:58 2018 -0500
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http1HeaderParser.java	Tue Jan 16 19:19:03 2018 +0000
@@ -213,9 +213,9 @@
         assert state == State.HEADER_FOUND_CR || state == State.HEADER_FOUND_LF;
         char c = (char)input.get();
         if (c == LF && state == State.HEADER_FOUND_CR) {
-            String headerString = sb.toString();
-            sb = new StringBuilder();
-            addHeaderFromString(headerString);
+            // header value will be flushed by
+            // resumeOrSecondCR if next line does not
+            // begin by SP or HT
             state = State.HEADER_FOUND_CR_LF;
         } else if (c == SP || c == HT) {
             sb.append(SP); // parity with MessageHeaders
@@ -229,11 +229,28 @@
 
     private void resumeOrSecondCR(ByteBuffer input) {
         assert state == State.HEADER_FOUND_CR_LF;
-        assert sb.length() == 0;
         char c = (char)input.get();
         if (c == CR) {
+            if (sb.length() > 0) {
+                // no continuation line - flush
+                // previous header value.
+                String headerString = sb.toString();
+                sb = new StringBuilder();
+                addHeaderFromString(headerString);
+            }
             state = State.HEADER_FOUND_CR_LF_CR;
+        } else if (c == SP || c == HT) {
+            assert sb.length() != 0;
+            sb.append(SP); // continuation line
+            state = State.HEADER;
         } else {
+            if (sb.length() > 0) {
+                // no continuation line - flush
+                // previous header value.
+                String headerString = sb.toString();
+                sb = new StringBuilder();
+                addHeaderFromString(headerString);
+            }
             sb.append(c);
             state = State.HEADER;
         }
--- a/test/jdk/java/net/httpclient/HeadersTest1.java	Tue Jan 16 10:48:58 2018 -0500
+++ b/test/jdk/java/net/httpclient/HeadersTest1.java	Tue Jan 16 19:19:03 2018 +0000
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8153142
+ * @bug 8153142 8195138
  * @modules jdk.incubator.httpclient
  *          jdk.httpserver
  * @run testng/othervm HeadersTest1
@@ -113,8 +113,16 @@
             }
 
             // toString
-            hd.toString().toLowerCase().contains("content-length");
-            hd.toString().toLowerCase().contains("x-foo-response");
+            assertTrue(hd.toString().toLowerCase().contains("content-length"));
+            assertTrue(hd.toString().toLowerCase().contains("x-foo-response"));
+            assertTrue(hd.toString().toLowerCase().contains("x-multi-line-response"));
+
+            // multi-line
+            List<String> multiline = hd.allValues("x-multi-line-response");
+            assertTrue(multiline.get(0).startsWith("Custom "));
+            assertTrue(multiline.get(0).contains(" foo=\"bar\""));
+            assertTrue(multiline.get(0).contains(" bar=\"foo\""));
+            assertTrue(multiline.get(0).contains(" foobar=\"barfoo\""));
         } finally {
             server.stop(0);
             e.shutdownNow();
@@ -138,6 +146,9 @@
             Headers h = he.getResponseHeaders();
             h.add("X-Foo-Response", "resp1");
             h.add("X-Foo-Response", "resp2");
+            h.add("X-multi-line-response", "Custom foo=\"bar\","
+                    + "\r\n    bar=\"foo\","
+                    + "\r\n    foobar=\"barfoo\"");
             he.sendResponseHeaders(200, RESPONSE.length());
             OutputStream os = he.getResponseBody();
             os.write(RESPONSE.getBytes(US_ASCII));
--- a/test/jdk/java/net/httpclient/whitebox/Http1HeaderParserTestDriver.java	Tue Jan 16 10:48:58 2018 -0500
+++ b/test/jdk/java/net/httpclient/whitebox/Http1HeaderParserTestDriver.java	Tue Jan 16 19:19:03 2018 +0000
@@ -23,6 +23,7 @@
 
 /*
  * @test
+ * @bug 8195138
  * @modules jdk.incubator.httpclient
  * @run testng jdk.incubator.httpclient/jdk.incubator.http.Http1HeaderParserTest
  */
--- a/test/jdk/java/net/httpclient/whitebox/jdk.incubator.httpclient/jdk/incubator/http/Http1HeaderParserTest.java	Tue Jan 16 10:48:58 2018 -0500
+++ b/test/jdk/java/net/httpclient/whitebox/jdk.incubator.httpclient/jdk/incubator/http/Http1HeaderParserTest.java	Tue Jan 16 19:19:03 2018 +0000
@@ -175,8 +175,15 @@
               " \t \t charset=UTF-8\r\n" +          // mix of preceding SP and HT
               "Connection: keep-alive\r\n\r\n" +
               "XXYYZZAABBCCDDEEFFGGHHII",
+
+              "HTTP/1.1 401 Unauthorized\r\n" +
+              "WWW-Authenticate: Digest realm=\"wally land\","
+                      +"$NEWLINE    domain=/,"
+                      +"$NEWLINE nonce=\"2B7F3A2B\","
+                      +"$NEWLINE\tqop=\"auth\"\r\n\r\n",
+
            };
-        for (String newLineChar : new String[] { "\n", "\r" }) {
+        for (String newLineChar : new String[] { "\n", "\r", "\r\n" }) {
             for (String template : foldingTemplate)
                 responses.add(template.replace("$NEWLINE", newLineChar));
         }
@@ -331,7 +338,7 @@
                     assertEquals(values.size(), otherValues.size(),
                                  format("%s. Expected list size %d, actual size %s",
                                         msg, values.size(), otherValues.size()));
-                    if (!values.containsAll(otherValues) && otherValues.containsAll(values))
+                    if (!(values.containsAll(otherValues) && otherValues.containsAll(values)))
                         assertTrue(false, format("Lists are unequal [%s] [%s]", values, otherValues));
                     break;
                 }