# HG changeset patch # User dfuchs # Date 1527157910 -3600 # Node ID e38ce7e04995113dd74624ba2046d2e912fbfc31 # Parent 4c502e3991bf86f28af7b4f9a0874526cd7d77a2 http-client-branch: HttpClient HTTP/1.1 could be more lenient and accept LF as well as CRLF as header separator. 8203427: HttpClient HTTP/1.1 could be more lenient and accept LF as well as CRLF as header separator. Reviewed-by: chegar diff -r 4c502e3991bf -r e38ce7e04995 src/java.net.http/share/classes/jdk/internal/net/http/Http1HeaderParser.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http1HeaderParser.java Wed May 23 16:44:13 2018 +0100 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http1HeaderParser.java Thu May 24 11:31:50 2018 +0100 @@ -51,8 +51,10 @@ enum State { STATUS_LINE, STATUS_LINE_FOUND_CR, + STATUS_LINE_FOUND_LF, STATUS_LINE_END, STATUS_LINE_END_CR, + STATUS_LINE_END_LF, HEADER, HEADER_FOUND_CR, HEADER_FOUND_LF, @@ -69,7 +71,10 @@ int responseCode() { return responseCode; } /** Returns the headers, possibly empty. */ - HttpHeaders headers() { assert state == State.FINISHED; return headers; } + HttpHeaders headers() { + assert state == State.FINISHED : "Unexpected state " + state; + return headers; + } /** * Parses HTTP/1.X status-line and headers from the given bytes. Must be @@ -84,18 +89,22 @@ boolean parse(ByteBuffer input) throws ProtocolException { requireNonNull(input, "null input"); - while (input.hasRemaining() && state != State.FINISHED) { + while (canContinueParsing(input)) { switch (state) { case STATUS_LINE: readResumeStatusLine(input); break; + // fallthrough case STATUS_LINE_FOUND_CR: + case STATUS_LINE_FOUND_LF: readStatusLineFeed(input); break; case STATUS_LINE_END: maybeStartHeaders(input); break; + // fallthrough case STATUS_LINE_END_CR: + case STATUS_LINE_END_LF: maybeEndHeaders(input); break; case HEADER: @@ -121,19 +130,33 @@ return state == State.FINISHED; } + private boolean canContinueParsing(ByteBuffer buffer) { + // some states don't require any input to transition + // to the next state. + switch (state) { + case FINISHED: return false; + case STATUS_LINE_FOUND_LF: return true; + case STATUS_LINE_END_LF: return true; + case HEADER_FOUND_LF: return true; + default: return buffer.hasRemaining(); + } + } + private void readResumeStatusLine(ByteBuffer input) { char c = 0; while (input.hasRemaining() && (c =(char)input.get()) != CR) { + if (c == LF) break; sb.append(c); } - if (c == CR) { state = State.STATUS_LINE_FOUND_CR; + } else if (c == LF) { + state = State.STATUS_LINE_FOUND_LF; } } private void readStatusLineFeed(ByteBuffer input) throws ProtocolException { - char c = (char)input.get(); + char c = state == State.STATUS_LINE_FOUND_LF ? LF : (char)input.get(); if (c != LF) { throw protocolException("Bad trailing char, \"%s\", when parsing status-line, \"%s\"", c, sb.toString()); @@ -158,6 +181,8 @@ char c = (char)input.get(); if (c == CR) { state = State.STATUS_LINE_END_CR; + } else if (c == LF) { + state = State.STATUS_LINE_END_LF; } else { sb.append(c); state = State.HEADER; @@ -165,9 +190,9 @@ } private void maybeEndHeaders(ByteBuffer input) throws ProtocolException { - assert state == State.STATUS_LINE_END_CR; + assert state == State.STATUS_LINE_END_CR || state == State.STATUS_LINE_END_LF; assert sb.length() == 0; - char c = (char)input.get(); + char c = state == State.STATUS_LINE_END_LF ? LF : (char)input.get(); if (c == LF) { headers = ImmutableHeaders.of(privateMap); privateMap = null; @@ -212,8 +237,8 @@ private void resumeOrLF(ByteBuffer input) { assert state == State.HEADER_FOUND_CR || state == State.HEADER_FOUND_LF; - char c = (char)input.get(); - if (c == LF && state == State.HEADER_FOUND_CR) { + char c = state == State.HEADER_FOUND_LF ? LF : (char)input.get(); + if (c == LF) { // header value will be flushed by // resumeOrSecondCR if next line does not // begin by SP or HT @@ -231,7 +256,7 @@ private void resumeOrSecondCR(ByteBuffer input) { assert state == State.HEADER_FOUND_CR_LF; char c = (char)input.get(); - if (c == CR) { + if (c == CR || c == LF) { if (sb.length() > 0) { // no continuation line - flush // previous header value. @@ -239,7 +264,13 @@ sb = new StringBuilder(); addHeaderFromString(headerString); } - state = State.HEADER_FOUND_CR_LF_CR; + if (c == CR) { + state = State.HEADER_FOUND_CR_LF_CR; + } else { + state = State.FINISHED; + headers = ImmutableHeaders.of(privateMap); + privateMap = null; + } } else if (c == SP || c == HT) { assert sb.length() != 0; sb.append(SP); // continuation line diff -r 4c502e3991bf -r e38ce7e04995 test/jdk/java/net/httpclient/whitebox/java.net.http/jdk/internal/net/http/Http1HeaderParserTest.java --- a/test/jdk/java/net/httpclient/whitebox/java.net.http/jdk/internal/net/http/Http1HeaderParserTest.java Wed May 23 16:44:13 2018 +0100 +++ b/test/jdk/java/net/httpclient/whitebox/java.net.http/jdk/internal/net/http/Http1HeaderParserTest.java Thu May 24 11:31:50 2018 +0100 @@ -32,6 +32,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.IntStream; import sun.net.www.MessageHeader; import org.testng.annotations.Test; @@ -132,6 +134,11 @@ "Connection: keep-alive\r\n\r\n" }; Arrays.stream(basic).forEach(responses::add); + // add some tests where some of the CRLF are replaced + // by a single LF + Arrays.stream(basic) + .map(Http1HeaderParserTest::mixedCRLF) + .forEach(responses::add); String[] foldingTemplate = { "HTTP/1.1 200 OK\r\n" + @@ -187,6 +194,12 @@ for (String template : foldingTemplate) responses.add(template.replace("$NEWLINE", newLineChar)); } + // add some tests where some of the CRLF are replaced + // by a single LF + for (String newLineChar : new String[] { "\n", "\r", "\r\n" }) { + for (String template : foldingTemplate) + responses.add(mixedCRLF(template).replace("$NEWLINE", newLineChar)); + } String[] bad = // much of this is to retain parity with legacy MessageHeaders { "HTTP/1.1 200 OK\r\n" + @@ -237,20 +250,77 @@ return responses.stream().map(p -> new Object[] { p }).toArray(Object[][]::new); } + static final AtomicInteger index = new AtomicInteger(); + static final AtomicInteger limit = new AtomicInteger(1); + static final AtomicBoolean useCRLF = new AtomicBoolean(); + // A small method to replace part of the CRLF present in a string + // with simple LF. The method uses a deterministic algorithm based + // on current values of static index/limit/useCRLF counters. + // These counters are used to produce a stream of substitutes that + // looks like this: + // LF CRLF LF LF CRLF CRLF LF LF LF CRLF CRLF CRLF (then repeat from start) + static final String mixedCRLF(String headers) { + int next; + int start = 0; + int last = headers.lastIndexOf("\r\n"); + String prev = ""; + StringBuilder res = new StringBuilder(); + while ((next = headers.indexOf("\r\n", start)) > 0) { + res.append(headers.substring(start, next)); + if ("\n".equals(prev) && next == last) { + // for some reason the legacy MessageHeader parser will + // not consume the final LF if the headers are terminated + // by instead of . It consume + // but leaves the last in the stream. + // Here we just make sure to avoid using + // as that would cause the legacy parser to consume + // 1 byte less than the Http1HeadersParser - which + // does consume the last , as it should. + // if this is the last CRLF and the previous one + // was replaced by LF then use LF. + res.append(prev); + } else { + prev = useCRLF.get() ? "\r\n" : "\n"; + res.append(prev); + } + // skip CRLF + start = next + 2; + + // The idea is to substitute some of the CRLF with LF. + // Rather than doing this randomly, always use the following + // sequence: + // LF CRLF LF LF CRLF CRLF LF LF LF CRLF CRLF CRLF + index.incrementAndGet(); + if (index.get() == limit.get()) { + index.set(0); + if (useCRLF.get()) limit.incrementAndGet(); + if (limit.get() > 3) limit.set(1); + useCRLF.set(!useCRLF.get()); + } + } + res.append(headers.substring(start)); + return res.toString(); + } + + @Test(dataProvider = "responses") public void verifyHeaders(String respString) throws Exception { + System.out.println("\ntesting:\n\t" + respString + .replace("\r\n", "") + .replace("\r", "") + .replace("\n","") + .replace("LF>", "LF>\n\t")); byte[] bytes = respString.getBytes(US_ASCII); ByteArrayInputStream bais = new ByteArrayInputStream(bytes); MessageHeader m = new MessageHeader(bais); Map> messageHeaderMap = m.getHeaders(); - int available = bais.available(); + int availableBytes = bais.available(); Http1HeaderParser decoder = new Http1HeaderParser(); ByteBuffer b = ByteBuffer.wrap(bytes); decoder.parse(b); + System.out.printf("Http1HeaderParser parsed %d bytes out of %d%n", b.position(), bytes.length); Map> decoderMap1 = decoder.headers().map(); - assertEquals(available, b.remaining(), - "stream available not equal to remaining"); // assert status-line String statusLine1 = messageHeaderMap.get(null).get(0); @@ -273,6 +343,9 @@ assertHeadersEqual(messageHeaderMap, decoderMap1, "messageHeaderMap not equal to decoderMap1"); + assertEquals(availableBytes, b.remaining(), + String.format("stream available (%d) not equal to remaining (%d)", + availableBytes, b.remaining())); // byte at a time decoder = new Http1HeaderParser(); List buffers = IntStream.range(0, bytes.length) @@ -280,9 +353,10 @@ .collect(toList()); while (decoder.parse(buffers.remove(0)) != true); Map> decoderMap2 = decoder.headers().map(); - assertEquals(available, buffers.size(), + assertEquals(availableBytes, buffers.size(), "stream available not equals to remaining buffers"); assertEquals(decoderMap1, decoderMap2, "decoder maps not equal"); + } @DataProvider(name = "errors")