You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2023/02/23 15:22:24 UTC
[tomcat] branch main updated: Validate the scheme pseudo-header in HTTP/2
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 3e19b6947c Validate the scheme pseudo-header in HTTP/2
3e19b6947c is described below
commit 3e19b6947c067c896c978d56f645d5f6f8751bb5
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Feb 23 15:22:10 2023 +0000
Validate the scheme pseudo-header in HTTP/2
---
java/org/apache/coyote/http2/Stream.java | 7 ++
java/org/apache/coyote/http2/StreamProcessor.java | 6 ++
.../apache/tomcat/util/http/parser/HttpParser.java | 51 ++++++++++++++
.../apache/coyote/http2/TestHttp2Section_8_1.java | 80 +++++++++++++++++++++-
webapps/docs/changelog.xml | 8 +++
5 files changed, 151 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
index 4a82c4a239..c5cc5f5f2f 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -149,6 +149,13 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
private void prepareRequest() {
+ if (coyoteRequest.scheme().isNull()) {
+ if (handler.getProtocol().getHttp11Protocol().isSSLEnabled()) {
+ coyoteRequest.scheme().setString("https");
+ } else {
+ coyoteRequest.scheme().setString("http");
+ }
+ }
MessageBytes hostValueMB = coyoteRequest.getMimeHeaders().getUniqueValue("host");
if (hostValueMB == null) {
throw new IllegalArgumentException();
diff --git a/java/org/apache/coyote/http2/StreamProcessor.java b/java/org/apache/coyote/http2/StreamProcessor.java
index 7398ce0de5..affc460a7b 100644
--- a/java/org/apache/coyote/http2/StreamProcessor.java
+++ b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -484,6 +484,12 @@ class StreamProcessor extends AbstractProcessor {
return false;
}
+ // Scheme must adhere to RFC 3986
+ String scheme = request.scheme().toString();
+ if (!HttpParser.isScheme(scheme)) {
+ return false;
+ }
+
// Invalid character in request target
// (other checks such as valid %nn happen later)
ByteChunk bc = request.requestURI().getByteChunk();
diff --git a/java/org/apache/tomcat/util/http/parser/HttpParser.java b/java/org/apache/tomcat/util/http/parser/HttpParser.java
index 0758dd5915..6975e3b53f 100644
--- a/java/org/apache/tomcat/util/http/parser/HttpParser.java
+++ b/java/org/apache/tomcat/util/http/parser/HttpParser.java
@@ -47,6 +47,7 @@ public class HttpParser {
private static final boolean[] IS_HTTP_PROTOCOL = new boolean[ARRAY_SIZE];
private static final boolean[] IS_ALPHA = new boolean[ARRAY_SIZE];
private static final boolean[] IS_NUMERIC = new boolean[ARRAY_SIZE];
+ private static final boolean[] IS_SCHEME = new boolean[ARRAY_SIZE];
private static final boolean[] IS_UNRESERVED = new boolean[ARRAY_SIZE];
private static final boolean[] IS_SUBDELIM = new boolean[ARRAY_SIZE];
private static final boolean[] IS_USERINFO = new boolean[ARRAY_SIZE];
@@ -94,6 +95,10 @@ public class HttpParser {
IS_ALPHA[i] = true;
}
+ if (IS_ALPHA[i] || IS_NUMERIC[i] || i == '+' || i == '-' || i == '.') {
+ IS_SCHEME[i] = true;
+ }
+
if (IS_ALPHA[i] || IS_NUMERIC[i] || i == '-' || i == '.' || i == '_' || i == '~') {
IS_UNRESERVED[i] = true;
}
@@ -319,6 +324,52 @@ public class HttpParser {
}
+ public static boolean isScheme(int c) {
+ // Fast for valid scheme characters, slower for some incorrect
+ // ones
+ try {
+ return IS_SCHEME[c];
+ } catch (ArrayIndexOutOfBoundsException ex) {
+ return false;
+ }
+ }
+
+
+ /**
+ * Is the provided String a scheme as per RFC 3986?
+ * <br>
+ * Note: scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
+ * <br>
+ * Since a scheme requires at least 1 ALPHA, {@code null} and the empty
+ * string ({@code ""}) are not considered to be valid tokens.
+ *
+ * @param s The string to test
+ *
+ * @return {@code true} if the string is a valid scheme, otherwise
+ * {@code false}
+ */
+ public static boolean isScheme(String s) {
+ if (s == null) {
+ return false;
+ }
+ if (s.isEmpty()) {
+ return false;
+ }
+ char[] chars = s.toCharArray();
+ if (!isAlpha(chars[0])) {
+ return false;
+ }
+ if (chars.length > 1) {
+ for (int i = 1; i < chars.length; i++) {
+ if (!isScheme(chars[i])) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
+
+
public static boolean isUserInfo(int c) {
// Fast for valid user info characters, slower for some incorrect
// ones
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
index 8e22bb3429..e76bfd869a 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
@@ -411,6 +411,11 @@ public class TestHttp2Section_8_1 extends Http2TestBase {
private void doInvalidPseudoHeaderTest(List<Header> headers) throws Exception {
+ doInvalidPseudoHeaderTest(headers, "3-RST-[1]\n");
+ }
+
+ private void doInvalidPseudoHeaderTest(List<Header> headers, String expected) throws Exception {
+
byte[] headersFrameHeader = new byte[9];
ByteBuffer headersPayload = ByteBuffer.allocate(128);
@@ -421,6 +426,79 @@ public class TestHttp2Section_8_1 extends Http2TestBase {
parser.readFrame();
- Assert.assertEquals("3-RST-[1]\n", output.getTrace());
+ String trace = output.getTrace();
+ if (trace.length() > expected.length()) {
+ trace = trace.substring(0, expected.length());
+ }
+ Assert.assertEquals(output.getTrace(), expected, trace);
+ }
+
+
+ @Test
+ public void testSchemeHeaderValid() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":scheme", "abcd"));
+ headers.add(new Header(":authority", "localhost:" + getPort()));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header("host", "localhost:" + getPort()));
+
+ 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();
+ Assert.assertTrue(trace, trace.contains("3-Header-[:status]-[200]"));
+ }
+
+
+ @Test
+ public void testSchemeHeaderInvalid01() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":scheme", "ab!cd"));
+ headers.add(new Header(":authority", "localhost:" + getPort()));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header("host", "localhost:" + getPort()));
+
+ doInvalidPseudoHeaderTest(headers, "3-HeadersStart\n3-Header-[:status]-[400]\n");
+ }
+
+
+ @Test
+ public void testSchemeHeaderInvalid02() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":scheme", ""));
+ headers.add(new Header(":authority", "localhost:" + getPort()));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header("host", "localhost:" + getPort()));
+
+ doInvalidPseudoHeaderTest(headers, "3-HeadersStart\n3-Header-[:status]-[400]\n");
+ }
+
+
+ @Test
+ public void testSchemeHeaderMissing() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":authority", "localhost:" + getPort()));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header("host", "localhost:" + getPort()));
+
+ doInvalidPseudoHeaderTest(headers, "0-Goaway-[3]-[1]-");
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ea885486e9..19034a62ba 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -114,6 +114,14 @@
</update>
</changelog>
</subsection>
+ <subsection name="Coyote">
+ <changelog>
+ <add>
+ Add a check for the validity of the scheme pseudo-header in HTTP/2.
+ (markt)
+ </add>
+ </changelog>
+ </subsection>
<subsection name="Jasper">
<changelog>
<add>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org