From 0ac3efe07015949ff6f36c69b78bc9e4a0d4308e Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Tue, 15 Nov 2022 17:46:42 +0100 Subject: [PATCH] Fix handling of optional whitespace at the beginning of headers. --- .../java/io/helidon/common/buffers/Bytes.java | 4 + .../io/helidon/common/buffers/LazyString.java | 42 +++++++++++ .../common/buffers/LazyStringTest.java | 74 +++++++++++++++++++ .../helidon/common/http/HeaderValueLazy.java | 6 +- .../common/http/Http1HeadersParser.java | 24 +++--- 5 files changed, 138 insertions(+), 12 deletions(-) create mode 100644 common/buffers/src/test/java/io/helidon/common/buffers/LazyStringTest.java diff --git a/common/buffers/src/main/java/io/helidon/common/buffers/Bytes.java b/common/buffers/src/main/java/io/helidon/common/buffers/Bytes.java index 379d8ef975b..f6ec7a596d1 100644 --- a/common/buffers/src/main/java/io/helidon/common/buffers/Bytes.java +++ b/common/buffers/src/main/java/io/helidon/common/buffers/Bytes.java @@ -64,6 +64,10 @@ public final class Bytes { * {@code %} byte. */ public static final byte PERCENT_BYTE = (byte) '%'; + /** + * Horizontal tabulator byte. + */ + public static final byte TAB_BYTE = (byte) '\t'; private Bytes() { } diff --git a/common/buffers/src/main/java/io/helidon/common/buffers/LazyString.java b/common/buffers/src/main/java/io/helidon/common/buffers/LazyString.java index 418064814cd..911e22b72e7 100644 --- a/common/buffers/src/main/java/io/helidon/common/buffers/LazyString.java +++ b/common/buffers/src/main/java/io/helidon/common/buffers/LazyString.java @@ -28,6 +28,7 @@ public class LazyString { private final Charset charset; private String stringValue; + private String owsLessValue; /** * New instance. @@ -57,6 +58,40 @@ public LazyString(byte[] buffer, Charset charset) { this.charset = charset; } + /** + * Strip optional whitespace(s) from beginning and end of the String. + * Defined by the HTTP specification, OWS is a sequence of zero to n space and/or horizontal tab characters. + * + * @return string without optional leading and trailing whitespaces + */ + public String stripOws() { + if (owsLessValue == null) { + int newOffset = offset; + int newLength = length; + for (int i = offset; i < offset + length; i++) { + if (isOws(buffer[i])) { + newOffset = i + 1; + newLength = length - (newOffset - offset); + } else { + // no more white space, go from the end now + break; + } + } + // now we need to go from the end of the string + for (int i = offset + length - 1; i > newOffset; i--) { + if (isOws(buffer[i])) { + newLength--; + } else { + break; + } + } + newLength = Math.max(newLength, 0); + owsLessValue = new String(buffer, newOffset, newLength, charset); + } + + return owsLessValue; + } + @Override public String toString() { if (stringValue == null) { @@ -64,4 +99,11 @@ public String toString() { } return stringValue; } + + private boolean isOws(byte aByte) { + return switch (aByte) { + case Bytes.SPACE_BYTE, Bytes.TAB_BYTE -> true; + default -> false; + }; + } } diff --git a/common/buffers/src/test/java/io/helidon/common/buffers/LazyStringTest.java b/common/buffers/src/test/java/io/helidon/common/buffers/LazyStringTest.java new file mode 100644 index 00000000000..e54eef6f174 --- /dev/null +++ b/common/buffers/src/test/java/io/helidon/common/buffers/LazyStringTest.java @@ -0,0 +1,74 @@ +/* + * Copyright (c) 2022 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.common.buffers; + +import java.util.stream.Stream; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import static java.nio.charset.StandardCharsets.US_ASCII; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; + +class LazyStringTest { + @ParameterizedTest + @MethodSource("owsData") + void testOwsHandling(OwsTestData data) { + assertThat(data.string().stripOws(), is(data.expected())); + } + + private static Stream owsData() { + return Stream.of( + new OwsTestData(new LazyString("some-value".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString(" some-value".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString("\tsome-value".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString(" some-value".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString("\t\tsome-value".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString(" \tsome-value".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString("some-value ".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString("some-value\t".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString("some-value ".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString("some-value\t\t".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString("some-value \t".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString(" some-value ".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString("\tsome-value\t".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString(" some-value ".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString("\t\tsome-value\t\t".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString(" \tsome-value\t ".getBytes(US_ASCII), US_ASCII), "some-value"), + new OwsTestData(new LazyString(" \t\t ".getBytes(US_ASCII), US_ASCII), ""), + new OwsTestData(new LazyString(" \t\r\t ".getBytes(US_ASCII), US_ASCII), "\r") + ); + } + + record OwsTestData(LazyString string, String expected) { + @Override + public String toString() { + StringBuilder result = new StringBuilder(); + + for (char c : string().toString().toCharArray()) { + switch (c) { + case '\t' -> result.append("\\t"); + case '\r' -> result.append("\\r"); + default -> result.append(c); + } + } + + return "\"" + result + "\""; + } + } +} diff --git a/common/http/src/main/java/io/helidon/common/http/HeaderValueLazy.java b/common/http/src/main/java/io/helidon/common/http/HeaderValueLazy.java index 21a7570618c..a20b989c2f3 100644 --- a/common/http/src/main/java/io/helidon/common/http/HeaderValueLazy.java +++ b/common/http/src/main/java/io/helidon/common/http/HeaderValueLazy.java @@ -35,7 +35,7 @@ class HeaderValueLazy extends HeaderValueBase { public Http.HeaderValueWriteable addValue(String value) { if (values == null) { values = new ArrayList<>(2); - values.add(this.value.toString()); + values.add(this.value.stripOws()); } values.add(value); return this; @@ -43,14 +43,14 @@ public Http.HeaderValueWriteable addValue(String value) { @Override public String value() { - return value.toString(); + return value.stripOws(); } @Override public List allValues() { if (values == null) { values = new ArrayList<>(2); - values.add(value.toString()); + values.add(value.stripOws()); } return values; } diff --git a/common/http/src/main/java/io/helidon/common/http/Http1HeadersParser.java b/common/http/src/main/java/io/helidon/common/http/Http1HeadersParser.java index aa0111271d6..b5e29d7383e 100644 --- a/common/http/src/main/java/io/helidon/common/http/Http1HeadersParser.java +++ b/common/http/src/main/java/io/helidon/common/http/Http1HeadersParser.java @@ -27,10 +27,13 @@ */ public final class Http1HeadersParser { // TODO expand set of fastpath headers - private static final byte[] HD_HOST = (HeaderEnum.HOST.defaultCase() + ": ").getBytes(StandardCharsets.UTF_8); - private static final byte[] HD_ACCEPT = (HeaderEnum.ACCEPT.defaultCase() + ": ").getBytes(StandardCharsets.UTF_8); + private static final byte[] HD_HOST = (HeaderEnum.HOST.defaultCase() + ":").getBytes(StandardCharsets.UTF_8); + private static final byte[] HD_ACCEPT = (HeaderEnum.ACCEPT.defaultCase() + ":").getBytes(StandardCharsets.UTF_8); private static final byte[] HD_CONNECTION = - (HeaderEnum.CONNECTION.defaultCase() + ": ").getBytes(StandardCharsets.UTF_8); + (HeaderEnum.CONNECTION.defaultCase() + ":").getBytes(StandardCharsets.UTF_8); + + private static final byte[] HD_USER_AGENT = + (HeaderEnum.USER_AGENT.defaultCase() + ":").getBytes(StandardCharsets.UTF_8); private Http1HeadersParser() { } @@ -53,7 +56,7 @@ public static WritableHeaders readHeaders(DataReader reader, int maxHeadersSi return headers; } - Http.HeaderName header = readHeaderName(reader, headers, maxLength, validate); + Http.HeaderName header = readHeaderName(reader, maxLength, validate); maxLength -= header.defaultCase().length() + 2; int eol = reader.findNewLine(maxLength); if (eol == maxLength) { @@ -72,7 +75,6 @@ public static WritableHeaders readHeaders(DataReader reader, int maxHeadersSi } private static Http.HeaderName readHeaderName(DataReader reader, - WritableHeaders headers, int maxLength, boolean validate) { switch (reader.lookup()) { @@ -94,6 +96,12 @@ private static Http.HeaderName readHeaderName(DataReader reader, return HeaderEnum.CONNECTION; } } + case (byte) 'U' -> { + if (reader.startsWith(HD_USER_AGENT)) { + reader.skip(HD_USER_AGENT.length); + return HeaderEnum.USER_AGENT; + } + } default -> { } } @@ -109,10 +117,8 @@ private static Http.HeaderName readHeaderName(DataReader reader, HttpToken.validate(headerName); } Http.HeaderName header = Http.Header.create(headerName); - reader.skip(1); - if (Bytes.SPACE_BYTE != reader.read()) { - throw new IllegalArgumentException("Invalid header, space not after colon: " + reader.debugDataHex()); - } + reader.skip(1); // skip the colon character + return header; } }