This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/11.0.x by this push:
new e5cef9618c Add HTTP/2 header filtering and associated tests
e5cef9618c is described below
commit e5cef9618c3f4fd31bd6fb1e83f0f18022280dac
Author: Mark Thomas <[email protected]>
AuthorDate: Thu Apr 16 17:11:46 2026 +0100
Add HTTP/2 header filtering and associated tests
Aligns HTTP/2 with HTTP/1.1
Note HTTP/2 does not have the same request smuggling concerns that there
are with HTTP/1.1
---
java/org/apache/coyote/http2/HPackHuffman.java | 75 ++++++++++++-
java/org/apache/coyote/http2/HpackDecoder.java | 35 ++++--
java/org/apache/coyote/http2/HpackEncoder.java | 25 ++++-
java/org/apache/coyote/http2/Http2Parser.java | 2 +
.../apache/coyote/http2/LocalStrings.properties | 7 +-
.../apache/coyote/http2/LocalStrings_fr.properties | 1 -
.../apache/coyote/http2/LocalStrings_ja.properties | 1 -
.../apache/coyote/http2/LocalStrings_ko.properties | 1 -
.../coyote/http2/LocalStrings_zh_CN.properties | 1 -
java/org/apache/coyote/http2/Stream.java | 10 +-
java/org/apache/coyote/http2/StreamException.java | 6 +
.../apache/tomcat/util/http/parser/HttpParser.java | 40 ++++++-
test/org/apache/coyote/http2/Http2TestBase.java | 7 +-
test/org/apache/coyote/http2/TestHPackHuffman.java | 2 +-
.../apache/coyote/http2/TestHttp2Section_8_2.java | 121 +++++++++++++++++++++
.../apache/coyote/http2/TestStreamProcessor.java | 5 +-
webapps/docs/changelog.xml | 9 +-
17 files changed, 301 insertions(+), 47 deletions(-)
diff --git a/java/org/apache/coyote/http2/HPackHuffman.java
b/java/org/apache/coyote/http2/HPackHuffman.java
index 0b669a7590..3ef6cb392f 100644
--- a/java/org/apache/coyote/http2/HPackHuffman.java
+++ b/java/org/apache/coyote/http2/HPackHuffman.java
@@ -22,6 +22,7 @@ import java.util.HashSet;
import java.util.Locale;
import java.util.Set;
+import org.apache.tomcat.util.http.parser.HttpParser;
import org.apache.tomcat.util.res.StringManager;
public class HPackHuffman {
@@ -372,12 +373,35 @@ public class HPackHuffman {
* @param target The target for the decompressed data
*
* @throws HpackException If the Huffman encoded value in HPACK headers
did not end with EOS padding
+ *
+ * @deprecated Will be removed in Tomcat 12. Use {@link
#decode(ByteBuffer, int, StringBuilder, boolean)}
*/
+ @Deprecated
public static void decode(ByteBuffer data, int length, StringBuilder
target) throws HpackException {
+ decode(data, length, target, false);
+ }
+
+ /**
+ * Decodes a huffman encoded string into the target StringBuilder. There
must be enough space left in the buffer for
+ * this method to succeed.
+ *
+ * @param data The byte buffer
+ * @param length The length of data from the buffer to decode
+ * @param target The target for the decompressed data
+ * @param isFieldName {@code true} if a field name is being decoded (names
have a more restrictive set of allowed
+ * characters than field values)
+ *
+ * @throws HpackException If the Huffman encoded value in HPACK headers
did not end with EOS padding
+ */
+ public static void decode(ByteBuffer data, int length, StringBuilder
target, boolean isFieldName)
+ throws HpackException {
+
assert data.remaining() >= length;
int treePos = 0;
boolean eosBits = true;
int eosBitCount = 0;
+ boolean firstChar = true;
+ char c = 'a';
for (int i = 0; i < length; ++i) {
byte b = data.get();
int bitPos = 7;
@@ -391,7 +415,27 @@ public class HPackHuffman {
// Found a zero, can't be counting EOS bits
eosBitCount = 0;
} else {
- target.append((char) (val & LOW_MASK));
+ c = (char) (val & LOW_MASK);
+ if (isFieldName) {
+ if (!HttpParser.isToken(c) ||
Character.isUpperCase(c)) {
+ throw new IllegalArgumentException(
+
sm.getString("hpackhuffman.decode.illegalCharacterName"));
+ }
+ } else {
+ if (firstChar) {
+ if (!HttpParser.isFieldVChar(c)) {
+ throw new IllegalArgumentException(
+
sm.getString("hpackhuffman.decode.illegalCharacterValue.start"));
+ }
+ firstChar = false;
+ } else {
+ if (!HttpParser.isFieldContent(c)) {
+ throw new IllegalArgumentException(
+
sm.getString("hpackhuffman.decode.illegalCharacterValue"));
+ }
+ }
+ }
+ target.append(c);
treePos = 0;
eosBits = true;
// Output a character, reset eosBitCount
@@ -410,7 +454,27 @@ public class HPackHuffman {
// as an error
throw new
HpackException(sm.getString("hpackhuffman.stringLiteralEOS"));
}
- target.append((char) ((val >> 16) & LOW_MASK));
+ c = (char) ((val >> 16) & LOW_MASK);
+ if (isFieldName) {
+ if (!HttpParser.isToken(c) ||
Character.isUpperCase(c)) {
+ throw new IllegalArgumentException(
+
sm.getString("hpackhuffman.decode.illegalCharacterName"));
+ }
+ } else {
+ if (firstChar) {
+ if (!HttpParser.isFieldVChar(c)) {
+ throw new IllegalArgumentException(
+
sm.getString("hpackhuffman.decode.illegalCharacterValue.start"));
+ }
+ firstChar = false;
+ } else {
+ if (!HttpParser.isFieldContent(c)) {
+ throw new IllegalArgumentException(
+
sm.getString("hpackhuffman.decode.illegalCharacterValue"));
+ }
+ }
+ }
+ target.append(c);
treePos = 0;
eosBits = true;
// Output a character, reset eosBitCount
@@ -426,6 +490,9 @@ public class HPackHuffman {
if (!eosBits) {
throw new
HpackException(sm.getString("hpackhuffman.huffmanEncodedHpackValueDidNotEndWithEOS"));
}
+ if (!HttpParser.isFieldVChar(c)) {
+ throw new
IllegalArgumentException(sm.getString("hpackhuffman.decode.illegalCharacterValue.end"));
+ }
}
@@ -455,8 +522,8 @@ public class HPackHuffman {
* Encodes the given string into the buffer. If there is not enough space
in the buffer, or the encoded version is
* bigger than the original it will return false and not modify the
buffers position.
*
- * @param buffer The buffer to encode into
- * @param toEncode The string to encode
+ * @param buffer The buffer to encode into
+ * @param toEncode The string to encode
*
* @return true if encoding succeeded
*/
diff --git a/java/org/apache/coyote/http2/HpackDecoder.java
b/java/org/apache/coyote/http2/HpackDecoder.java
index d9f2ec62e4..791d1ff4c6 100644
--- a/java/org/apache/coyote/http2/HpackDecoder.java
+++ b/java/org/apache/coyote/http2/HpackDecoder.java
@@ -20,6 +20,7 @@ import java.nio.ByteBuffer;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.http.parser.HttpParser;
import org.apache.tomcat.util.res.StringManager;
/**
@@ -114,7 +115,7 @@ public class HpackDecoder {
buffer.position(originalPos);
return;
}
- String headerValue = readHpackString(buffer);
+ String headerValue = readHpackString(buffer, false);
if (headerValue == null) {
buffer.position(originalPos);
return;
@@ -128,7 +129,7 @@ public class HpackDecoder {
buffer.position(originalPos);
return;
}
- String headerValue = readHpackString(buffer);
+ String headerValue = readHpackString(buffer, false);
if (headerValue == null) {
buffer.position(originalPos);
return;
@@ -141,7 +142,7 @@ public class HpackDecoder {
buffer.position(originalPos);
return;
}
- String headerValue = readHpackString(buffer);
+ String headerValue = readHpackString(buffer, false);
if (headerValue == null) {
buffer.position(originalPos);
return;
@@ -202,11 +203,11 @@ public class HpackDecoder {
} else if (index != 0) {
return handleIndexedHeaderName(index);
} else {
- return readHpackString(buffer);
+ return readHpackString(buffer, true);
}
}
- private String readHpackString(ByteBuffer buffer) throws HpackException {
+ private String readHpackString(ByteBuffer buffer, boolean isFieldName)
throws HpackException {
if (!buffer.hasRemaining()) {
return null;
}
@@ -218,18 +219,34 @@ public class HpackDecoder {
}
boolean huffman = (data & 0b10000000) != 0;
if (huffman) {
- return readHuffmanString(length, buffer);
+ return readHuffmanString(length, buffer, isFieldName);
}
StringBuilder stringBuilder = new StringBuilder(length);
for (int i = 0; i < length; ++i) {
- stringBuilder.append((char) buffer.get());
+ char c = (char) (buffer.get() & 0xFF);
+ if (isFieldName) {
+ if (HttpParser.isToken(c) && !Character.isUpperCase(c)) {
+ stringBuilder.append(c);
+ } else {
+ throw new IllegalArgumentException(
+ sm.getString("hpackdecoder.illegalCharacterName",
Character.toString(c)));
+ }
+ } else {
+ if ((i == 0 || i == length - 1) && HttpParser.isFieldVChar(c)
||
+ i > 0 && i < length - 1 &&
HttpParser.isFieldContent(c)) {
+ stringBuilder.append(c);
+ } else {
+ throw new IllegalArgumentException(
+ sm.getString("hpackdecoder.illegalCharacterValue",
Character.toString(c)));
+ }
+ }
}
return stringBuilder.toString();
}
- private String readHuffmanString(int length, ByteBuffer buffer) throws
HpackException {
+ private String readHuffmanString(int length, ByteBuffer buffer, boolean
isFieldName) throws HpackException {
StringBuilder stringBuilder = new StringBuilder(length);
- HPackHuffman.decode(buffer, length, stringBuilder);
+ HPackHuffman.decode(buffer, length, stringBuilder, isFieldName);
return stringBuilder.toString();
}
diff --git a/java/org/apache/coyote/http2/HpackEncoder.java
b/java/org/apache/coyote/http2/HpackEncoder.java
index 2096c04db0..97aeace111 100644
--- a/java/org/apache/coyote/http2/HpackEncoder.java
+++ b/java/org/apache/coyote/http2/HpackEncoder.java
@@ -121,6 +121,22 @@ class HpackEncoder {
* @return The state of the encoding process
*/
State encode(MimeHeaders headers, ByteBuffer target) {
+ return encode(headers, target, true);
+ }
+
+ /**
+ * Encodes the headers into a buffer.
+ *
+ * @param headers The headers to encode
+ * @param target The buffer to which to write the encoded headers
+ * @param forceLowerCase Normally {@code true} to ensure that header field
names are lower case as required for
+ * HTTP/2 but some tests may deliberately allow
upper case characters to test Tomcat's
+ * handling of such invalid field header names.
+ *
+ * @return The state of the encoding process
+ */
+ State encode(MimeHeaders headers, ByteBuffer target, boolean
forceLowerCase) {
+
int it = headersIterator;
if (headersIterator == -1) {
handleTableSizeChange(target);
@@ -133,11 +149,10 @@ class HpackEncoder {
}
}
while (it < currentHeaders.size()) {
- /*
- * Need to ensure header names are lower case from this point
onwards as table lookups etc. are
- * case-sensitive.
- */
- String headerName =
headers.getName(it).toString().toLowerCase(Locale.ENGLISH);
+ String headerName = headers.getName(it).toString();
+ if (forceLowerCase) {
+ headerName = headerName.toLowerCase(Locale.US);
+ }
boolean skip = false;
if (firstPass) {
if (headerName.charAt(0) != ':') {
diff --git a/java/org/apache/coyote/http2/Http2Parser.java
b/java/org/apache/coyote/http2/Http2Parser.java
index e6db6d6b02..524925577f 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -531,6 +531,8 @@ class Http2Parser {
} catch (HpackException hpe) {
throw new
ConnectionException(sm.getString("http2Parser.processFrameHeaders.decodingFailed"),
Http2Error.COMPRESSION_ERROR, hpe);
+ } catch (IllegalArgumentException iae) {
+ throw new StreamException("Invalid headers",
Http2Error.PROTOCOL_ERROR, streamId, iae);
}
// switches to write mode
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties
b/java/org/apache/coyote/http2/LocalStrings.properties
index 31c61d76f2..ded1e4794e 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -46,6 +46,8 @@ hpackdecoder.addDynamic=Adding header to index [{0}] of
dynamic table with name
hpackdecoder.clearDynamic=Emptying dynamic table
hpackdecoder.emitHeader=Emitting header with name [{0}] and value [{1}]
hpackdecoder.headerTableIndexInvalid=The header table index [{0}] is not valid
as there are [{1}] static entries and [{2}] dynamic entries
+hpackdecoder.illegalCharacterName=The illegal [{0}] character was found when
decoding an HTTP/2 header field name
+hpackdecoder.illegalCharacterValue=The illegal [{0}] character was found when
decoding an HTTP/2 header field value
hpackdecoder.maxMemorySizeExceeded=The header table size [{0}] exceeds the
maximum size [{1}]
hpackdecoder.notImplemented=Not yet implemented
hpackdecoder.nullHeader=Null header at index [{0}]
@@ -54,6 +56,10 @@ hpackdecoder.useDynamic=Using header from index [{0}] of
dynamic table
hpackdecoder.useStatic=Using header from index [{0}] of static table
hpackdecoder.zeroNotValidHeaderTableIndex=Zero is not a valid header table
index
+hpackhuffman.decode.illegalCharacterName=The illegal [{0}] character was found
when decoding an HTTP/2 header field name
+hpackhuffman.decode.illegalCharacterValue=The illegal [{0}] character was
found when decoding an HTTP/2 header field value
+hpackhuffman.decode.illegalCharacterValue.end=The illegal [{0}] character was
found when decoding the final character in an HTTP/2 header field value
+hpackhuffman.decode.illegalCharacterValue.start=The illegal [{0}] character
was found when decoding the first character in an HTTP/2 header field value
hpackhuffman.huffmanEncodedHpackValueDidNotEndWithEOS=Huffman encoded value in
HPACK headers did not end with EOS padding
hpackhuffman.stringLiteralEOS=Huffman encoded value in HPACK headers contained
the EOS symbol
hpackhuffman.stringLiteralTooMuchPadding=More than 7 bits of EOS padding were
provided at the end of an Huffman encoded string literal
@@ -92,7 +98,6 @@ pingManager.roundTripTime=Connection [{0}] Round trip time
measured as [{1}]ns
stream.clientResetRequest=Client reset the stream before the request was fully
read
stream.closed=Connection [{0}], Stream [{1}], Unable to write to stream once
it has been closed
-stream.header.case=Connection [{0}], Stream [{1}], HTTP header name [{2}] must
be in lower case
stream.header.connection=Connection [{0}], Stream [{1}], HTTP header [{2}] is
not permitted in an HTTP/2 request
stream.header.contentLength=Connection [{0}], Stream [{1}], The content length
header value [{2}] does not agree with the size of the data received [{3}]
stream.header.debug=Connection [{0}], Stream [{1}], HTTP header [{2}], Value
[{3}]
diff --git a/java/org/apache/coyote/http2/LocalStrings_fr.properties
b/java/org/apache/coyote/http2/LocalStrings_fr.properties
index 87d520781f..709c97c425 100644
--- a/java/org/apache/coyote/http2/LocalStrings_fr.properties
+++ b/java/org/apache/coyote/http2/LocalStrings_fr.properties
@@ -92,7 +92,6 @@ pingManager.roundTripTime=Connection [{0}] Le temps d''aller
retour est de [{1}]
stream.clientResetRequest=Le client a réinitialisé la stream avant qu'elle ait
été complètement lue
stream.closed=Connection [{0}], Flux [{1}], Impossible d''écrire sur un flux
après sa fermeture
-stream.header.case=Connection [{0}], Flux [{1}], Le nom d''en-tête HTTP [{2}]
doit être en miniscules
stream.header.connection=Connection [{0}], Flux [{1}], L''en-tête HTTP [{2}]
n''est pas autorisé dans une requête HTTP/2
stream.header.contentLength=Connection [{0}], Flux [{1}], La valeur de
l''en-tête content-length [{2}] ne correspond pas à la taille des données reçue
[{3}]
stream.header.debug=Connection [{0}], Flux [{1}], en-tête HTTP [{2}], valeur
[{3}]
diff --git a/java/org/apache/coyote/http2/LocalStrings_ja.properties
b/java/org/apache/coyote/http2/LocalStrings_ja.properties
index 4727137e2c..955081c472 100644
--- a/java/org/apache/coyote/http2/LocalStrings_ja.properties
+++ b/java/org/apache/coyote/http2/LocalStrings_ja.properties
@@ -92,7 +92,6 @@ pingManager.roundTripTime=コネクション [{0}] の往復時間は [{1}] ns
stream.clientResetRequest=リクエストが完全に読み取られる前にクライアントがストリームをリセットしました
stream.closed=コネクション [{0}]、ストリーム [{1}]、切断したストリームには書き込みできません
-stream.header.case=コネクション [{0}]、ストリーム [{1}]、HTTP ヘッダー名 [{2}] は小文字でなければなりません。
stream.header.connection=コネクション [{0}]、ストリーム [{1}]、HTTP/2 のリクエストには HTTP ヘッダー
[{2}] を指定することはできません。
stream.header.contentLength=コネクション [{0}]、ストリーム [{1}]、content length ヘッダーの値
[{2}] と受信したデータ長 [{3}] は一致しません。
stream.header.debug=コネクション [{0}]、ストリーム [{1}]、HTTP ヘッダー [{2}]、値は [{3}]
diff --git a/java/org/apache/coyote/http2/LocalStrings_ko.properties
b/java/org/apache/coyote/http2/LocalStrings_ko.properties
index 6a48b00b1d..9ecb85e49c 100644
--- a/java/org/apache/coyote/http2/LocalStrings_ko.properties
+++ b/java/org/apache/coyote/http2/LocalStrings_ko.properties
@@ -86,7 +86,6 @@ http2Protocol.jmxRegistration.fail=HTTP/2 프로토콜을 JMX에 등록하지
pingManager.roundTripTime=연결 [{0}]: 라운드 트립 시간이 [{1}] 나노초(ns)로 측정되었습니다.
stream.closed=연결 [{0}], 스트림 [{1}], 한번 닫힌 스트림에 쓰기를 할 수 없습니다.
-stream.header.case=연결 [{0}], 스트림 [{1}], HTTP 헤더 이름 [{2}]은(는) 반드시 소문자여야 합니다.
stream.header.connection=연결 [{0}], 스트림 [{1}], HTTP 헤더 [{2}]은 HTTP/2 요청에서 허용되지
않습니다.
stream.header.contentLength=연결 [{0}], 스트림 [{1}], 해당 Content-Length 헤더 값
[{2}]은(는) 수신된 데이터의 크기 [{3}]와(과) 일치하지 않습니다.
stream.header.debug=연결 [{0}], 스트림 [{1}], HTTP 헤더: [{2}], 값: [{3}]
diff --git a/java/org/apache/coyote/http2/LocalStrings_zh_CN.properties
b/java/org/apache/coyote/http2/LocalStrings_zh_CN.properties
index bcc5a2396c..d2e5dbe9d8 100644
--- a/java/org/apache/coyote/http2/LocalStrings_zh_CN.properties
+++ b/java/org/apache/coyote/http2/LocalStrings_zh_CN.properties
@@ -87,7 +87,6 @@ http2Protocol.jmxRegistration.fail=HTTP/2协议注册JMX失败
pingManager.roundTripTime=连接[{0}]往返时间测量为[{1}]ns
stream.closed=连接[{0}]，流[{1}]，一旦关闭就无法写入流
-stream.header.case=连接[{0}]，流[{1}]，HTTP标头名称[{2}]必须小写
stream.header.connection=HTTP/2请求中不允许连接[{0}]、流[{1}]、HTTP头[{2}]
stream.header.contentLength=连接[{0}]，流[{1}]，内容长度头值[{2}]与接收的数据大小[{3}]不一致
stream.header.debug=连接[{0}]，流[{1}]，HTTP标头[{2}]，值[{3}]
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 687b859539..c4a23d9900 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -22,7 +22,6 @@ import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.HashSet;
-import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -322,14 +321,7 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
log.trace(sm.getString("stream.header.debug", getConnectionId(),
getIdAsString(), name, value));
}
- // Header names must be lowercase
- if (!name.toLowerCase(Locale.US).equals(name)) {
- headerException =
- new StreamException(sm.getString("stream.header.case",
getConnectionId(), getIdAsString(), name),
- Http2Error.PROTOCOL_ERROR, getIdAsInt());
- // No need for further processing. The stream will be reset.
- return;
- }
+ // Field header names being all lower case is enforced in HpackDecoder.
if (HTTP_CONNECTION_SPECIFIC_HEADERS.contains(name)) {
headerException = new StreamException(
diff --git a/java/org/apache/coyote/http2/StreamException.java
b/java/org/apache/coyote/http2/StreamException.java
index 0056144082..894d0e5f90 100644
--- a/java/org/apache/coyote/http2/StreamException.java
+++ b/java/org/apache/coyote/http2/StreamException.java
@@ -34,6 +34,12 @@ class StreamException extends Http2Exception {
}
+ StreamException(String msg, Http2Error error, int streamId, Throwable
cause) {
+ super(msg, error, cause);
+ this.streamId = streamId;
+ }
+
+
int getStreamId() {
return streamId;
}
diff --git a/java/org/apache/tomcat/util/http/parser/HttpParser.java
b/java/org/apache/tomcat/util/http/parser/HttpParser.java
index 8b41c52140..e85e47214d 100644
--- a/java/org/apache/tomcat/util/http/parser/HttpParser.java
+++ b/java/org/apache/tomcat/util/http/parser/HttpParser.java
@@ -33,7 +33,7 @@ public class HttpParser {
private static final StringManager sm =
StringManager.getManager(HttpParser.class);
- private static final int ARRAY_SIZE = 128;
+ private static final int ARRAY_SIZE = 256;
private static final boolean[] IS_CONTROL = new boolean[ARRAY_SIZE];
private static final boolean[] IS_SEPARATOR = new boolean[ARRAY_SIZE];
@@ -47,6 +47,8 @@ public class HttpParser {
private static final boolean[] IS_SUBDELIM = new boolean[ARRAY_SIZE];
private static final boolean[] IS_USERINFO = new boolean[ARRAY_SIZE];
private static final boolean[] IS_RELAXABLE = new boolean[ARRAY_SIZE];
+ private static final boolean[] IS_FIELD_VCHAR = new boolean[ARRAY_SIZE];
+ private static final boolean[] IS_FIELD_CONTENT = new boolean[ARRAY_SIZE];
private static final HttpParser DEFAULT;
@@ -66,7 +68,7 @@ public class HttpParser {
}
// Token: Anything 0-127 that is not a control and not a separator
- if (!IS_CONTROL[i] && !IS_SEPARATOR[i]) {
+ if (!IS_CONTROL[i] && !IS_SEPARATOR[i] && i < 128) {
IS_TOKEN[i] = true;
}
@@ -75,7 +77,6 @@ public class HttpParser {
IS_HEX[i] = true;
}
- // Not valid for HTTP protocol
// "HTTP/" DIGIT "." DIGIT
if (i == 'H' || i == 'T' || i == 'P' || i == '/' || i == '.' || (i
>= '0' && i <= '9')) {
IS_HTTP_PROTOCOL[i] = true;
@@ -114,6 +115,16 @@ public class HttpParser {
i == '{' || i == '|' || i == '}') {
IS_RELAXABLE[i] = true;
}
+
+ // field-vchar is VCHAR / obs-text
+ if (i > 20 && i < 127 || i > 127) {
+ IS_FIELD_VCHAR[i] = true;
+ }
+
+ // field-content = field-vchar [ 1*( SP / HTAB / field-vchar )
field-vchar ]
+ if (IS_FIELD_VCHAR[i] || i == '\t' || i == ' ') {
+ IS_FIELD_CONTENT[i] = true;
+ }
}
DEFAULT = new HttpParser(null, null);
@@ -393,8 +404,7 @@ public class HttpParser {
public static boolean isControl(int c) {
- // Fast for valid control characters, slower for some incorrect
- // ones
+ // Fast for valid control characters, slower for some incorrect ones
try {
return IS_CONTROL[c];
} catch (ArrayIndexOutOfBoundsException ex) {
@@ -403,6 +413,26 @@ public class HttpParser {
}
+ public static boolean isFieldVChar(int c) {
+ // Fast for valid field-vchar characters, slower for some incorrect
ones
+ try {
+ return IS_FIELD_VCHAR[c];
+ } catch (ArrayIndexOutOfBoundsException ex) {
+ return false;
+ }
+ }
+
+
+ public static boolean isFieldContent(int c) {
+ // Fast for valid field-content characters, slower for some incorrect
ones
+ try {
+ return IS_FIELD_CONTENT[c];
+ } catch (ArrayIndexOutOfBoundsException ex) {
+ return false;
+ }
+ }
+
+
/*
* Skip any whitespace and position to read the next character. The next
character is returned as being able to
* 'peek()' it allows a small optimisation in some cases.
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java
b/test/org/apache/coyote/http2/Http2TestBase.java
index 5b01366f9b..979ea511e2 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -247,7 +247,8 @@ public abstract class Http2TestBase extends TomcatBaseTest {
for (Header header : headers) {
mimeHeaders.addValue(header.getName()).setString(header.getValue());
}
- hpackEncoder.encode(mimeHeaders, headersPayload);
+ // Don't force lower case to allow testing with upper case field names
+ hpackEncoder.encode(mimeHeaders, headersPayload, false);
if (padding != null) {
headersPayload.put(padding);
}
@@ -1200,7 +1201,7 @@ public abstract class Http2TestBase extends
TomcatBaseTest {
public void headersEnd(int streamId, boolean endOfStream) {
trace.append(streamId + "-HeadersEnd\n");
if (endOfStream) {
- receivedEndOfStream(streamId) ;
+ receivedEndOfStream(streamId);
}
}
@@ -1464,7 +1465,7 @@ public abstract class Http2TestBase extends
TomcatBaseTest {
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse
resp) throws ServletException, IOException {
- Map<String, String[]> params = req.getParameterMap();
+ Map<String,String[]> params = req.getParameterMap();
resp.setContentType("text/plain");
resp.setCharacterEncoding("UTF-8");
diff --git a/test/org/apache/coyote/http2/TestHPackHuffman.java
b/test/org/apache/coyote/http2/TestHPackHuffman.java
index 5aaf6ac8f9..1dc457c506 100644
--- a/test/org/apache/coyote/http2/TestHPackHuffman.java
+++ b/test/org/apache/coyote/http2/TestHPackHuffman.java
@@ -38,7 +38,7 @@ public class TestHPackHuffman {
buf.get();
StringBuilder target = new StringBuilder();
- HPackHuffman.decode(buf, buf.remaining(), target);
+ HPackHuffman.decode(buf, buf.remaining(), target, false);
Assert.assertEquals("Value changed after encode/decode roundtrip",
data, target.toString());
}
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_8_2.java
b/test/org/apache/coyote/http2/TestHttp2Section_8_2.java
new file mode 100644
index 0000000000..e15875e9c2
--- /dev/null
+++ b/test/org/apache/coyote/http2/TestHttp2Section_8_2.java
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.coyote.http2;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+
+import org.apache.tomcat.util.http.Method;
+import org.apache.tomcat.util.http.parser.HttpParser;
+
+
+/**
+ * Unit tests for Section 8.2 of <a
href="https://tools.ietf.org/html/rfc7540";>RFC 7540</a>.
+ */
+@RunWith(Parameterized.class)
+public class TestHttp2Section_8_2 extends Http2TestBase {
+
+ @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4}")
+ public static Collection<Object[]> parameters() {
+ Collection<Object[]> baseData = data();
+
+ List<Object[]> parameterSets = new ArrayList<>();
+ for (Object[] base : baseData) {
+ parameterSets.add(new Object[] { base[0], base[1], "x-test",
"x-value", Boolean.TRUE });
+ // Strings longer than 5 characters will be huffman encoded
+ for (char c = 0; c < 256; c++) {
+ // HTTP/2 field names must be tokens and must be lower case
+ boolean valid = HttpParser.isToken(c) &&
!Character.isUpperCase(c);
+ // Non-Huffman field names
+ parameterSets.add(new Object[] { base[0], base[1], "x-" + c +
"t", "x-value", Boolean.valueOf(valid) });
+ parameterSets.add(new Object[] { base[0], base[1], c + "x-t",
"x-value", Boolean.valueOf(valid) });
+ parameterSets.add(new Object[] { base[0], base[1], "x-t" + c,
"x-value", Boolean.valueOf(valid) });
+ // Huffman field names
+ parameterSets
+ .add(new Object[] { base[0], base[1], "x-te" + c +
"st", "x-value", Boolean.valueOf(valid) });
+ parameterSets.add(new Object[] { base[0], base[1], c +
"x-test", "x-value", Boolean.valueOf(valid) });
+ parameterSets.add(new Object[] { base[0], base[1], "x-test" +
c, "x-value", Boolean.valueOf(valid) });
+
+ // HTTP/2 field values have same criteria as HTTP/1.1
+ // Non-Huffman field values
+ parameterSets.add(new Object[] { base[0], base[1], "x-test",
"x-" + c + "v",
+ Boolean.valueOf(HttpParser.isFieldContent(c)) });
+ parameterSets.add(new Object[] { base[0], base[1], "x-test", c
+ "x-v",
+ Boolean.valueOf(HttpParser.isFieldVChar(c)) });
+ parameterSets.add(new Object[] { base[0], base[1], "x-test",
"x-v" + c,
+ Boolean.valueOf(HttpParser.isFieldVChar(c)) });
+ parameterSets.add(new Object[] { base[0], base[1], "x-test",
"" + c,
+ Boolean.valueOf(HttpParser.isFieldVChar(c)) });
+ // Huffman field values
+ parameterSets.add(new Object[] { base[0], base[1], "x-test",
"x-va" + c + "lue",
+ Boolean.valueOf(HttpParser.isFieldContent(c)) });
+ parameterSets.add(new Object[] { base[0], base[1], "x-test", c
+ "x-value",
+ Boolean.valueOf(HttpParser.isFieldVChar(c)) });
+ parameterSets.add(new Object[] { base[0], base[1], "x-test",
"x-value" + c,
+ Boolean.valueOf(HttpParser.isFieldVChar(c)) });
+ }
+ }
+ return parameterSets;
+ }
+
+
+ @Parameter(2)
+ public String fieldName;
+
+ @Parameter(3)
+ public String fieldValue;
+
+ @Parameter(4)
+ public boolean valid;
+
+
+ @Test
+ public void testFieldNameAndValue() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", Method.GET));
+ headers.add(new Header(":scheme", "http"));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header(":authority", "localhost:" + getPort()));
+ headers.add(new Header(fieldName, fieldValue));
+
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+
+ buildGetRequest(headersFrameHeader, headersPayload, null, headers, 3);
+
+ writeFrame(headersFrameHeader, headersPayload);
+
+ parser.readFrame();
+
+ String trace = output.getTrace();
+ if (valid) {
+ Assert.assertTrue(trace,
trace.contains("3-Header-[:status]-[200]"));
+ } else {
+ Assert.assertTrue(trace, trace.contains("3-RST-[1]"));
+ }
+ }
+}
diff --git a/test/org/apache/coyote/http2/TestStreamProcessor.java
b/test/org/apache/coyote/http2/TestStreamProcessor.java
index 24777ca9ac..25a40b1283 100644
--- a/test/org/apache/coyote/http2/TestStreamProcessor.java
+++ b/test/org/apache/coyote/http2/TestStreamProcessor.java
@@ -296,10 +296,7 @@ public class TestStreamProcessor extends Http2TestBase {
parser.readFrame();
StringBuilder expected = new StringBuilder();
- expected.append("3-HeadersStart\n");
- expected.append("3-Header-[:status]-[400]\n");
- expected.append("3-Header-[date]-[Wed, 11 Nov 2015 19:18:42 GMT]\n");
- expected.append("3-HeadersEnd\n");
+ expected.append("3-RST-[1]\n");
Assert.assertEquals(expected.toString(), output.getTrace());
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 344b7df8b1..2696c0da8e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -207,8 +207,13 @@
connection close. (markt)
</fix>
<fix>
- Refactor HTTP/2 HPACK encoding so field names are only converted to
- lower case once during the encoding process. (markt)
+ Refactor HTTP/2 HPACK encoding so header field names are only converted
+ to lower case once during the encoding process. (markt)
+ </fix>
+ <fix>
+ Refactor HTTP/2 header field validation so it occurs earlier. Extend
+ validation to check for disallowed characters as well as upper case
+ characters. (markt)
</fix>
<fix>
Add TLS 1.3 groups added in OpenSSL 4.0. (remm)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
