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 2022/08/08 13:19:47 UTC
[tomcat] branch main updated (9547412dc8 -> 40b5497016)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
from 9547412dc8 Fix typo
new 1a93cf8932 Align host header processing with RFC 9113
new 40b5497016 Fix checkstyle warnings
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
java/org/apache/catalina/users/MemoryGroup.java | 1 -
.../apache/catalina/users/MemoryUserDatabase.java | 12 +--
.../apache/coyote/http2/LocalStrings.properties | 1 +
java/org/apache/coyote/http2/Stream.java | 71 +++++++++++++---
.../apache/coyote/http2/TestHttp2Section_8_1.java | 99 ++++++++++++++++++++++
webapps/docs/changelog.xml | 13 +++
6 files changed, 176 insertions(+), 21 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/02: Align host header processing with RFC 9113
Posted by ma...@apache.org.
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
commit 1a93cf89321e7a86aedcd39206b52cf605fe0806
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Aug 8 13:05:53 2022 +0100
Align host header processing with RFC 9113
---
.../apache/coyote/http2/LocalStrings.properties | 1 +
java/org/apache/coyote/http2/Stream.java | 71 +++++++++++++---
.../apache/coyote/http2/TestHttp2Section_8_1.java | 99 ++++++++++++++++++++++
webapps/docs/changelog.xml | 13 +++
4 files changed, 170 insertions(+), 14 deletions(-)
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties b/java/org/apache/coyote/http2/LocalStrings.properties
index 823ea18a7c..62f3f189bb 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -97,6 +97,7 @@ stream.header.required=Connection [{0}], Stream [{1}], One or more required head
stream.header.te=Connection [{0}], Stream [{1}], HTTP header [te] is not permitted to have the value [{2}] in an HTTP/2 request
stream.header.unexpectedPseudoHeader=Connection [{0}], Stream [{1}], Pseudo header [{2}] received after a regular header
stream.header.unknownPseudoHeader=Connection [{0}], Stream [{1}], Unknown pseudo header [{2}] received
+stream.host.inconsistent=Connection [{0}], Stream [{1}], The header host header [{2}] is inconsistent with previously provided values for host [{3}] and/or port [{4}]
stream.inputBuffer.copy=Copying [{0}] bytes from inBuffer to outBuffer
stream.inputBuffer.dispatch=Data added to inBuffer when read interest is registered. Triggering a read dispatch
stream.inputBuffer.empty=The Stream input buffer is empty. Waiting for more data
diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
index cd15cb0f49..24f68c7e24 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -93,6 +93,7 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
private StreamException headerException = null;
private volatile StringBuilder cookieHeader = null;
+ private volatile boolean hostHeaderSeen = false;
private Object pendingWindowUpdateForStreamLock = new Object();
private int pendingWindowUpdateForStream = 0;
@@ -385,20 +386,7 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
}
case ":authority": {
if (coyoteRequest.serverName().isNull()) {
- int i;
- try {
- i = Host.parse(value);
- } catch (IllegalArgumentException iae) {
- // Host value invalid
- throw new HpackException(sm.getString("stream.header.invalid",
- getConnectionId(), getIdAsString(), ":authority", value));
- }
- if (i > -1) {
- coyoteRequest.serverName().setString(value.substring(0, i));
- coyoteRequest.setServerPort(Integer.parseInt(value.substring(i + 1)));
- } else {
- coyoteRequest.serverName().setString(value);
- }
+ parseAuthority(value, false);
} else {
throw new HpackException(sm.getString("stream.header.duplicate",
getConnectionId(), getIdAsString(), ":authority" ));
@@ -416,6 +404,22 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
cookieHeader.append(value);
break;
}
+ case "host": {
+ if (coyoteRequest.serverName().isNull()) {
+ // No :authority header. This is first host header. Use it.
+ hostHeaderSeen = true;
+ parseAuthority(value, true);
+ } else if (!hostHeaderSeen) {
+ // First host header - must be consistent with :authority
+ hostHeaderSeen = true;
+ compareAuthority(value);
+ } else {
+ // Multiple hosts headers - illegal
+ throw new HpackException(sm.getString("stream.header.duplicate",
+ getConnectionId(), getIdAsString(), "host" ));
+ }
+ break;
+ }
default: {
if (headerState == HEADER_STATE_TRAILER &&
!handler.getProtocol().isTrailerHeaderAllowed(name)) {
@@ -441,6 +445,45 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
}
+ private void parseAuthority(String value, boolean host) throws HpackException {
+ int i;
+ try {
+ i = Host.parse(value);
+ } catch (IllegalArgumentException iae) {
+ // Host value invalid
+ throw new HpackException(sm.getString("stream.header.invalid",
+ getConnectionId(), getIdAsString(), host ? "host" : ":authority", value));
+ }
+ if (i > -1) {
+ coyoteRequest.serverName().setString(value.substring(0, i));
+ coyoteRequest.setServerPort(Integer.parseInt(value.substring(i + 1)));
+ } else {
+ coyoteRequest.serverName().setString(value);
+ }
+ }
+
+
+ private void compareAuthority(String value) throws HpackException {
+ int i;
+ try {
+ i = Host.parse(value);
+ } catch (IllegalArgumentException iae) {
+ // Host value invalid
+ throw new HpackException(sm.getString("stream.header.invalid",
+ getConnectionId(), getIdAsString(), "host", value));
+ }
+ if (i == -1 && value.equals(coyoteRequest.serverName().getString()) ||
+ i > -1 && ((!value.substring(0, i).equals(coyoteRequest.serverName().getString()) ||
+ Integer.parseInt(value.substring(i + 1)) != coyoteRequest.getServerPort()))) {
+ // Host value inconsistent
+ throw new HpackException(sm.getString("stream.host.inconsistent",
+ getConnectionId(), getIdAsString(), value, coyoteRequest.serverName().getString(),
+ Integer.toString(coyoteRequest.getServerPort())));
+ }
+
+ }
+
+
@Override
public void setHeaderException(StreamException streamException) {
if (headerException == null) {
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
index 1fa2b5d15b..3b47dc13ae 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
@@ -245,6 +245,105 @@ public class TestHttp2Section_8_1 extends Http2TestBase {
}
+ @Test
+ public void testHostHeaderValid() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":scheme", "http"));
+ 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(true);
+
+ String trace = output.getTrace();
+ Assert.assertTrue(trace, trace.contains("3-Header-[:status]-[200]"));
+ }
+
+
+ @Test
+ public void testHostHeaderDuplicate() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":scheme", "http"));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header("host", "localhost:" + getPort()));
+ 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(true);
+
+ String trace = output.getTrace();
+ Assert.assertTrue(trace, trace.contains("0-Goaway-[1]-[9]"));
+ }
+
+
+ @Test
+ public void testHostHeaderConsistent() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":scheme", "http"));
+ 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(true);
+
+ String trace = output.getTrace();
+ Assert.assertTrue(trace, trace.contains("3-Header-[:status]-[200]"));
+ }
+
+
+ @Test
+ public void testHostHeaderInconsistent() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":scheme", "http"));
+ headers.add(new Header(":authority", "localhost:" + getPort()));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header("host", "otherhost:" + getPort()));
+
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+
+ buildGetRequest(headersFrameHeader, headersPayload, null, headers , 3);
+
+ writeFrame(headersFrameHeader, headersPayload);
+
+ parser.readFrame(true);
+
+ String trace = output.getTrace();
+ Assert.assertTrue(trace, trace.contains("0-Goaway-[1]-[9]"));
+ }
+
+
private void doInvalidPseudoHeaderTest(List<Header> headers) throws Exception {
http2Connect();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 40e9cbe546..caea8f83bb 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -144,6 +144,19 @@
Ensure HTTP/2 requests that include connection specific headers are
rejected. (markt)
</fix>
+ <fix>
+ When processing HTTP/2 requests, allow a <code>host</code> header to be
+ used in place of an <code>:authority</code> header. (markt)
+ </fix>
+ <fix>
+ When processing HTTP/2 requests, allow a <code>host</code> header and an
+ <code>:authority</code> header to be present providing the are
+ consistent. (markt)
+ </fix>
+ <fix>
+ When processing HTTP/2 requests, reject requests containing multiple
+ <code>host</code> headers. (markt)
+ </fix>
</changelog>
</subsection>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/02: Fix checkstyle warnings
Posted by ma...@apache.org.
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
commit 40b5497016a0fc79fc25d2e112b72ba11a9ac6b6
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Aug 8 13:19:08 2022 +0100
Fix checkstyle warnings
---
java/org/apache/catalina/users/MemoryGroup.java | 1 -
java/org/apache/catalina/users/MemoryUserDatabase.java | 12 ++++++------
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/catalina/users/MemoryGroup.java b/java/org/apache/catalina/users/MemoryGroup.java
index 9de5b959f9..7f5d90eb2b 100644
--- a/java/org/apache/catalina/users/MemoryGroup.java
+++ b/java/org/apache/catalina/users/MemoryGroup.java
@@ -17,7 +17,6 @@
package org.apache.catalina.users;
-import org.apache.catalina.Role;
import org.apache.catalina.UserDatabase;
import org.apache.tomcat.util.buf.StringUtils;
import org.apache.tomcat.util.security.Escape;
diff --git a/java/org/apache/catalina/users/MemoryUserDatabase.java b/java/org/apache/catalina/users/MemoryUserDatabase.java
index b0aff0fdf8..614d855c5f 100644
--- a/java/org/apache/catalina/users/MemoryUserDatabase.java
+++ b/java/org/apache/catalina/users/MemoryUserDatabase.java
@@ -296,7 +296,7 @@ public class MemoryUserDatabase implements UserDatabase {
throw new IllegalArgumentException(msg);
}
- Group group = new GenericGroup(this, groupname, description, null);
+ Group group = new GenericGroup<>(this, groupname, description, null);
readLock.lock();
try {
groups.put(group.getGroupname(), group);
@@ -321,7 +321,7 @@ public class MemoryUserDatabase implements UserDatabase {
throw new IllegalArgumentException(msg);
}
- Role role = new GenericRole(this, rolename, description);
+ Role role = new GenericRole<>(this, rolename, description);
readLock.lock();
try {
roles.put(role.getRolename(), role);
@@ -348,7 +348,7 @@ public class MemoryUserDatabase implements UserDatabase {
throw new IllegalArgumentException(msg);
}
- User user = new GenericUser(this, username, password, fullName, null, null);
+ User user = new GenericUser<>(this, username, password, fullName, null, null);
readLock.lock();
try {
users.put(user.getUsername(), user);
@@ -607,7 +607,7 @@ public class MemoryUserDatabase implements UserDatabase {
writer.print("\"");
}
writer.print(" roles=\"");
- for(Iterator<Role> roles=group.getRoles(); roles.hasNext(); ) {
+ for (Iterator<Role> roles=group.getRoles(); roles.hasNext();) {
Role role = roles.next();
writer.print(Escape.xml(role.getRolename()));
if(roles.hasNext()) {
@@ -631,7 +631,7 @@ public class MemoryUserDatabase implements UserDatabase {
writer.print("\"");
}
writer.print(" groups=\"");
- for(Iterator<Group> groups=user.getGroups(); groups.hasNext(); ) {
+ for (Iterator<Group> groups=user.getGroups(); groups.hasNext();) {
Group group = groups.next();
writer.print(Escape.xml(group.getGroupname()));
if(groups.hasNext()) {
@@ -639,7 +639,7 @@ public class MemoryUserDatabase implements UserDatabase {
}
}
writer.print("\" roles=\"");
- for(Iterator<Role> roles=user.getRoles(); roles.hasNext(); ) {
+ for (Iterator<Role> roles=user.getRoles(); roles.hasNext();) {
Role role = roles.next();
writer.print(Escape.xml(role.getRolename()));
if(roles.hasNext()) {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org