8195138: The asynchronous Http1HeaderParser doesn't handle all line folds correctly

Reviewed-by: chegar
This commit is contained in:
Daniel Fuchs 2018-01-16 19:19:03 +00:00
parent 99853dbf51
commit 424048c75f
4 changed files with 45 additions and 9 deletions

View file

@ -213,9 +213,9 @@ class Http1HeaderParser {
assert state == State.HEADER_FOUND_CR || state == State.HEADER_FOUND_LF; assert state == State.HEADER_FOUND_CR || state == State.HEADER_FOUND_LF;
char c = (char)input.get(); char c = (char)input.get();
if (c == LF && state == State.HEADER_FOUND_CR) { if (c == LF && state == State.HEADER_FOUND_CR) {
String headerString = sb.toString(); // header value will be flushed by
sb = new StringBuilder(); // resumeOrSecondCR if next line does not
addHeaderFromString(headerString); // begin by SP or HT
state = State.HEADER_FOUND_CR_LF; state = State.HEADER_FOUND_CR_LF;
} else if (c == SP || c == HT) { } else if (c == SP || c == HT) {
sb.append(SP); // parity with MessageHeaders sb.append(SP); // parity with MessageHeaders
@ -229,11 +229,28 @@ class Http1HeaderParser {
private void resumeOrSecondCR(ByteBuffer input) { private void resumeOrSecondCR(ByteBuffer input) {
assert state == State.HEADER_FOUND_CR_LF; assert state == State.HEADER_FOUND_CR_LF;
assert sb.length() == 0;
char c = (char)input.get(); char c = (char)input.get();
if (c == CR) { 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; 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 { } else {
if (sb.length() > 0) {
// no continuation line - flush
// previous header value.
String headerString = sb.toString();
sb = new StringBuilder();
addHeaderFromString(headerString);
}
sb.append(c); sb.append(c);
state = State.HEADER; state = State.HEADER;
} }

View file

@ -23,7 +23,7 @@
/* /*
* @test * @test
* @bug 8153142 * @bug 8153142 8195138
* @modules jdk.incubator.httpclient * @modules jdk.incubator.httpclient
* jdk.httpserver * jdk.httpserver
* @run testng/othervm HeadersTest1 * @run testng/othervm HeadersTest1
@ -113,8 +113,16 @@ public class HeadersTest1 {
} }
// toString // toString
hd.toString().toLowerCase().contains("content-length"); assertTrue(hd.toString().toLowerCase().contains("content-length"));
hd.toString().toLowerCase().contains("x-foo-response"); 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 { } finally {
server.stop(0); server.stop(0);
e.shutdownNow(); e.shutdownNow();
@ -138,6 +146,9 @@ public class HeadersTest1 {
Headers h = he.getResponseHeaders(); Headers h = he.getResponseHeaders();
h.add("X-Foo-Response", "resp1"); h.add("X-Foo-Response", "resp1");
h.add("X-Foo-Response", "resp2"); 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()); he.sendResponseHeaders(200, RESPONSE.length());
OutputStream os = he.getResponseBody(); OutputStream os = he.getResponseBody();
os.write(RESPONSE.getBytes(US_ASCII)); os.write(RESPONSE.getBytes(US_ASCII));

View file

@ -23,6 +23,7 @@
/* /*
* @test * @test
* @bug 8195138
* @modules jdk.incubator.httpclient * @modules jdk.incubator.httpclient
* @run testng jdk.incubator.httpclient/jdk.incubator.http.Http1HeaderParserTest * @run testng jdk.incubator.httpclient/jdk.incubator.http.Http1HeaderParserTest
*/ */

View file

@ -175,8 +175,15 @@ public class Http1HeaderParserTest {
" \t \t charset=UTF-8\r\n" + // mix of preceding SP and HT " \t \t charset=UTF-8\r\n" + // mix of preceding SP and HT
"Connection: keep-alive\r\n\r\n" + "Connection: keep-alive\r\n\r\n" +
"XXYYZZAABBCCDDEEFFGGHHII", "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) for (String template : foldingTemplate)
responses.add(template.replace("$NEWLINE", newLineChar)); responses.add(template.replace("$NEWLINE", newLineChar));
} }
@ -331,7 +338,7 @@ public class Http1HeaderParserTest {
assertEquals(values.size(), otherValues.size(), assertEquals(values.size(), otherValues.size(),
format("%s. Expected list size %d, actual size %s", format("%s. Expected list size %d, actual size %s",
msg, values.size(), otherValues.size())); 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)); assertTrue(false, format("Lists are unequal [%s] [%s]", values, otherValues));
break; break;
} }