From 92e909a43108c728fa2b5cf50492ef6fcfacc05b Mon Sep 17 00:00:00 2001 From: Dhiraj Suri Date: Fri, 31 Oct 2025 15:58:51 -0700 Subject: [PATCH] Allow suspicious URI characters 8.1.x --- .../io/confluent/rest/ApplicationServer.java | 32 +- .../java/io/confluent/rest/RestConfig.java | 11 + .../java/io/confluent/rest/Http2Test.java | 288 +++++++++++++++++- 3 files changed, 320 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/io/confluent/rest/ApplicationServer.java b/core/src/main/java/io/confluent/rest/ApplicationServer.java index 2911b01b3a..0b57194410 100644 --- a/core/src/main/java/io/confluent/rest/ApplicationServer.java +++ b/core/src/main/java/io/confluent/rest/ApplicationServer.java @@ -27,8 +27,8 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; +import java.util.Objects; import java.util.StringTokenizer; import java.util.concurrent.BlockingQueue; @@ -272,6 +272,28 @@ public Map getSslContextFactories() { return sslContextFactories; } + private void setJettyUriCompliance(HttpConfiguration httpConfiguration) { + // Refer to https://github.com/jetty/jetty.project/issues/11890#issuecomment-2156449534 + // In Jetty 12, using Servlet 6 and ee10+, ambiguous path separators are not allowed + // We must set a URI compliance to allow for this violation so that client + // requests are not automatically rejected + if (serverConfig.getBoolean(RestConfig.JETTY_LEGACY_URI_COMPLIANCE)) { + // If we want to run Jetty 12 with backward-compliance to Jetty 9, + // we should allow the following violations. Otherwise, some existing URI + // paths will encounter errors + UriCompliance backwardCompatibility = UriCompliance.LEGACY.with( + "backward-compatibility", + UriCompliance.Violation.ILLEGAL_PATH_CHARACTERS, + UriCompliance.Violation.SUSPICIOUS_PATH_CHARACTERS + ); + httpConfiguration.setUriCompliance(backwardCompatibility); + } else { + httpConfiguration.setUriCompliance(UriCompliance.from(Set.of( + UriCompliance.Violation.AMBIGUOUS_PATH_SEPARATOR, + UriCompliance.Violation.AMBIGUOUS_PATH_ENCODING))); + } + } + private void configureConnectors() { for (NamedURI listener : listeners) { RestConfig connectorConfig = serverConfig.getListenerScopedConfig(listener); @@ -295,13 +317,7 @@ private void configureConnectors() { httpConfiguration.addCustomizer(new ForwardedRequestCustomizer()); } - // Refer to https://github.com/jetty/jetty.project/issues/11890#issuecomment-2156449534 - // In Jetty 12, using Servlet 6 and ee10+, ambiguous path separators are not allowed - // We must set a URI compliance to allow for this violation so that client - // requests are not automatically rejected - httpConfiguration.setUriCompliance(UriCompliance.from(Set.of( - UriCompliance.Violation.AMBIGUOUS_PATH_SEPARATOR, - UriCompliance.Violation.AMBIGUOUS_PATH_ENCODING))); + setJettyUriCompliance(httpConfiguration); final HttpConnectionFactory httpConnectionFactory = new HttpConnectionFactory(httpConfiguration); diff --git a/core/src/main/java/io/confluent/rest/RestConfig.java b/core/src/main/java/io/confluent/rest/RestConfig.java index a878c076d1..928bcf34a4 100644 --- a/core/src/main/java/io/confluent/rest/RestConfig.java +++ b/core/src/main/java/io/confluent/rest/RestConfig.java @@ -382,6 +382,11 @@ public class RestConfig extends AbstractConfig { "The capacity of request queue for each thread pool."; public static final int REQUEST_QUEUE_CAPACITY_DEFAULT = Integer.MAX_VALUE; + public static final String JETTY_LEGACY_URI_COMPLIANCE = "jetty.legacy.uri.compliance"; + public static final String JETTY_LEGACY_URI_COMPLIANCE_DOC = + "Enable legacy URI Compliance in Jetty."; + public static final boolean JETTY_LEGACY_URI_COMPLIANCE_DEFAULT = false; + public static final String REQUEST_QUEUE_CAPACITY_INITIAL_CONFIG = "request.queue.capacity.init"; public static final String REQUEST_QUEUE_CAPACITY_INITIAL_DOC = "The initial capacity of request queue for each thread pool."; @@ -1031,6 +1036,12 @@ private static ConfigDef incompleteBaseConfigDef() { REQUEST_QUEUE_CAPACITY_DEFAULT, Importance.LOW, REQUEST_QUEUE_CAPACITY_DOC + ).define( + JETTY_LEGACY_URI_COMPLIANCE, + Type.BOOLEAN, + JETTY_LEGACY_URI_COMPLIANCE_DEFAULT, + Importance.LOW, + JETTY_LEGACY_URI_COMPLIANCE_DOC ).define( REQUEST_QUEUE_CAPACITY_GROWBY_CONFIG, Type.INT, diff --git a/core/src/test/java/io/confluent/rest/Http2Test.java b/core/src/test/java/io/confluent/rest/Http2Test.java index d61271939f..59517423e7 100644 --- a/core/src/test/java/io/confluent/rest/Http2Test.java +++ b/core/src/test/java/io/confluent/rest/Http2Test.java @@ -79,6 +79,9 @@ public class Http2Test { private static final String HTTPS_URI = "https://localhost:8081"; private static final String SSL_PASSWORD = "test1234"; private static final String EXPECTED_200_MSG = "Response status must be 200."; + private static final String EXPECTED_400_MSG = "Response status must be 400."; + private static final String UNENCODED_BACKSLASH_PATH = "/test\\unencoded-backslash"; + private static final String URL_ENCODED_BACKSLASH_PATH = "/test%5Cencoded-backslash"; @BeforeEach public void setUp() throws Exception { @@ -113,11 +116,15 @@ private void configServerKeystore(Properties props) { } private TestRestConfig buildTestConfig(boolean enableHttp2) { - return buildTestConfig(enableHttp2, null, null); + return buildTestConfig(enableHttp2, null, null, false); + } + + private TestRestConfig buildTestConfig(boolean enableHttp2, boolean enableLegacyURI) { + return buildTestConfig(enableHttp2, null, null, enableLegacyURI); } private TestRestConfig buildTestConfig(boolean enableHttp2, String sslProtocol, - String sslProvider) { + String sslProvider, boolean enableLegacyURI) { Properties props = new Properties(); props.put(RestConfig.LISTENERS_CONFIG, HTTP_URI + "," + HTTPS_URI); props.put(RestConfig.METRICS_REPORTER_CLASSES_CONFIG, "io.confluent.rest.TestMetricsReporter"); @@ -130,6 +137,9 @@ private TestRestConfig buildTestConfig(boolean enableHttp2, String sslProtocol, if (sslProvider != null) { props.put(RestConfig.SSL_PROVIDER_CONFIG, sslProvider); } + if (enableLegacyURI) { + props.put(RestConfig.JETTY_LEGACY_URI_COMPLIANCE, true); + } configServerKeystore(props); return new TestRestConfig(props); } @@ -166,7 +176,7 @@ public void testHttp2() throws Exception { @Test public void testHttp2WithConscrypt() throws Exception { - TestRestConfig config = buildTestConfig(true, "TLSv1.3", SslConfig.TLS_CONSCRYPT); + TestRestConfig config = buildTestConfig(true, "TLSv1.3", SslConfig.TLS_CONSCRYPT, false); Http2TestApplication app = new Http2TestApplication(config); try { app.start(); @@ -224,6 +234,242 @@ public void testHttp2AmbiguousSegment() throws Exception { } } + @Test + public void testHttp2EncodedBackslashLegacyCompatibility() throws Exception { + // This test is ensuring that URI-encoded / characters work in URIs in all variants + // With Jetty 12, additional changes to configuring the servlet and URI handling were needed + // to allow HTTP requests with ambiguous segments to be handled properly. + TestRestConfig config = buildTestConfig(true, true); + Http2TestApplication app = new Http2TestApplication(config); + try { + app.start(); + + int statusCode; + + // Just skip HTTP/2 for earlier than Java 11 + if (ApplicationServer.isJava11Compatible()) { + statusCode = makeGetRequestHttp2(HTTP_URI + URL_ENCODED_BACKSLASH_PATH); + assertEquals(200, statusCode, EXPECTED_200_MSG); + statusCode = makeGetRequestHttp2(HTTPS_URI + URL_ENCODED_BACKSLASH_PATH, + clientKeystore.getAbsolutePath(), SSL_PASSWORD, SSL_PASSWORD); + assertEquals(200, statusCode, EXPECTED_200_MSG); + } + + // HTTP/1.1 should work whether HTTP/2 is available or not + statusCode = makeGetRequestHttp(HTTP_URI + URL_ENCODED_BACKSLASH_PATH); + assertEquals(200, statusCode, EXPECTED_200_MSG); + statusCode = makeGetRequestHttp(HTTPS_URI + URL_ENCODED_BACKSLASH_PATH, + clientKeystore.getAbsolutePath(), SSL_PASSWORD, SSL_PASSWORD); + assertEquals(200, statusCode, EXPECTED_200_MSG); + assertMetricsCollected(); + } finally { + app.stop(); + } + } + + @Test + public void testHttp2EncodedBackslash() throws Exception { + // This test is ensuring that URI-encoded / characters work in URIs in all variants + // With Jetty 12, additional changes to configuring the servlet and URI handling were needed + // to allow HTTP requests with ambiguous segments to be handled properly. + TestRestConfig config = buildTestConfig(true); + Http2TestApplication app = new Http2TestApplication(config); + try { + app.start(); + + int statusCode; + + // Just skip HTTP/2 for earlier than Java 11 + if (ApplicationServer.isJava11Compatible()) { + statusCode = makeGetRequestHttp2(HTTP_URI + URL_ENCODED_BACKSLASH_PATH); + assertEquals(400, statusCode, EXPECTED_400_MSG); + statusCode = makeGetRequestHttp2(HTTPS_URI + URL_ENCODED_BACKSLASH_PATH, + clientKeystore.getAbsolutePath(), SSL_PASSWORD, SSL_PASSWORD); + assertEquals(400, statusCode, EXPECTED_400_MSG); + } + + // HTTP/1.1 should work whether HTTP/2 is available or not + statusCode = makeGetRequestHttp(HTTP_URI + URL_ENCODED_BACKSLASH_PATH); + assertEquals(400, statusCode, EXPECTED_400_MSG); + statusCode = makeGetRequestHttp(HTTPS_URI + URL_ENCODED_BACKSLASH_PATH, + clientKeystore.getAbsolutePath(), SSL_PASSWORD, SSL_PASSWORD); + assertEquals(400, statusCode, EXPECTED_400_MSG); + assertMetricsCollected(); + } finally { + app.stop(); + } + } + + @Test + public void testHttp2UnencodedBackslashLegacyCompatibility() throws Exception { + // This test is ensuring that Jetty 12 URI Compliance enforcement is + // compatible with the legacy Jetty 9 enforcement. + TestRestConfig config = buildTestConfig(true, true); + Http2TestApplication app = new Http2TestApplication(config); + try { + app.start(); + + int statusCode; + + // Just skip HTTP/2 for earlier than Java 11 + if (ApplicationServer.isJava11Compatible()) { + HTTP2Client http2Client = new HTTP2Client(); + HttpClient http2ClientHttp = new HttpClient(new HttpClientTransportOverHTTP2(http2Client)); + http2ClientHttp.start(); + try { + statusCode = http2ClientHttp + .newRequest(HTTP_URI) + .path(UNENCODED_BACKSLASH_PATH) + .send() + .getStatus(); + assertEquals(200, statusCode, EXPECTED_200_MSG); + } finally { + http2ClientHttp.stop(); + } + + // HTTPS over HTTP/2 with client keystore, preserving raw backslash + SslContextFactory.Client sslContextFactoryH2 = buildSslContextFactory( + clientKeystore.getAbsolutePath(), SSL_PASSWORD, SSL_PASSWORD); + ClientConnector h2TlsConnector = new ClientConnector(); + h2TlsConnector.setSslContextFactory(sslContextFactoryH2); + HTTP2Client http2TlsClient = new HTTP2Client(h2TlsConnector); + HttpClient https2Client = new HttpClient(new HttpClientTransportOverHTTP2(http2TlsClient)); + https2Client.start(); + try { + statusCode = https2Client + .newRequest(HTTPS_URI) + .path(UNENCODED_BACKSLASH_PATH) + .send() + .getStatus(); + assertEquals(200, statusCode, EXPECTED_200_MSG); + } finally { + https2Client.stop(); + } + } + + // HTTP/1.1 should behave consistently as well (raw backslash) + HttpClient http11Client = new HttpClient(); + http11Client.start(); + try { + statusCode = http11Client + .newRequest(HTTP_URI) + .path(UNENCODED_BACKSLASH_PATH) + .send() + .getStatus(); + assertEquals(200, statusCode, EXPECTED_200_MSG); + } finally { + http11Client.stop(); + } + + // HTTPS over HTTP/1.1 with client keystore, preserving raw backslash + SslContextFactory.Client sslContextFactory = buildSslContextFactory( + clientKeystore.getAbsolutePath(), SSL_PASSWORD, SSL_PASSWORD); + ClientConnector clientConnector = new ClientConnector(); + clientConnector.setSslContextFactory(sslContextFactory); + HttpClient https11Client = new HttpClient(new HttpClientTransportDynamic(clientConnector)); + https11Client.start(); + try { + statusCode = https11Client + .newRequest(HTTPS_URI) + .path(UNENCODED_BACKSLASH_PATH) + .send() + .getStatus(); + assertEquals(200, statusCode, EXPECTED_200_MSG); + } finally { + https11Client.stop(); + } + + assertMetricsCollected(); + } finally { + app.stop(); + } + } + + @Test + public void testHttp2UnencodedBackslash() throws Exception { + // This test is ensuring that Jetty 12 URI Compliance enforcement is + // compatible with the legacy Jetty 9 enforcement. + TestRestConfig config = buildTestConfig(true); + Http2TestApplication app = new Http2TestApplication(config); + try { + app.start(); + + int statusCode; + + // Just skip HTTP/2 for earlier than Java 11 + if (ApplicationServer.isJava11Compatible()) { + HTTP2Client http2Client = new HTTP2Client(); + HttpClient http2ClientHttp = new HttpClient(new HttpClientTransportOverHTTP2(http2Client)); + http2ClientHttp.start(); + try { + statusCode = http2ClientHttp + .newRequest(HTTP_URI) + .path(UNENCODED_BACKSLASH_PATH) + .send() + .getStatus(); + assertEquals(400, statusCode, EXPECTED_400_MSG); + } finally { + http2ClientHttp.stop(); + } + + // HTTPS over HTTP/2 with client keystore, preserving raw backslash + SslContextFactory.Client sslContextFactoryH2 = buildSslContextFactory( + clientKeystore.getAbsolutePath(), SSL_PASSWORD, SSL_PASSWORD); + ClientConnector h2TlsConnector = new ClientConnector(); + h2TlsConnector.setSslContextFactory(sslContextFactoryH2); + HTTP2Client http2TlsClient = new HTTP2Client(h2TlsConnector); + HttpClient https2Client = new HttpClient(new HttpClientTransportOverHTTP2(http2TlsClient)); + https2Client.start(); + try { + statusCode = https2Client + .newRequest(HTTPS_URI) + .path(UNENCODED_BACKSLASH_PATH) + .send() + .getStatus(); + assertEquals(400, statusCode, EXPECTED_400_MSG); + } finally { + https2Client.stop(); + } + } + + // HTTP/1.1 should behave consistently as well (raw backslash) + HttpClient http11Client = new HttpClient(); + http11Client.start(); + try { + statusCode = http11Client + .newRequest(HTTP_URI) + .path(UNENCODED_BACKSLASH_PATH) + .send() + .getStatus(); + assertEquals(400, statusCode, EXPECTED_400_MSG); + } finally { + http11Client.stop(); + } + + // HTTPS over HTTP/1.1 with client keystore, preserving raw backslash + SslContextFactory.Client sslContextFactory = buildSslContextFactory( + clientKeystore.getAbsolutePath(), SSL_PASSWORD, SSL_PASSWORD); + ClientConnector clientConnector = new ClientConnector(); + clientConnector.setSslContextFactory(sslContextFactory); + HttpClient https11Client = new HttpClient(new HttpClientTransportDynamic(clientConnector)); + https11Client.start(); + try { + statusCode = https11Client + .newRequest(HTTPS_URI) + .path(UNENCODED_BACKSLASH_PATH) + .send() + .getStatus(); + assertEquals(400, statusCode, EXPECTED_400_MSG); + } finally { + https11Client.stop(); + } + + assertMetricsCollected(); + } finally { + app.stop(); + } + } + @Test public void testHttp2CNotEnabled() throws Exception { TestRestConfig config = buildTestConfig(false); @@ -381,6 +627,8 @@ public Http2TestApplication(TestRestConfig props) { public void setupResources(Configurable config, TestRestConfig appConfig) { config.register(new Http2TestResource()); config.register(new Http2TestAmbiguousSegmentResource()); + config.register(new Http2EncodedBackslashResource()); + config.register(new Http2UnencodedBackslashResource()); } @Override @@ -408,6 +656,40 @@ public Http2TestResponse hello() { } } + @Path(URL_ENCODED_BACKSLASH_PATH) + @Produces("application/test.v1+json") + public static class Http2UnencodedBackslashResource { + public static class Http2UnencodedBackslashResponse { + @JsonProperty + public String getMessage() { + return "unencoded-backslash"; + } + } + + @GET + @PerformanceMetric("test") + public Http2UnencodedBackslashResponse hello() { + return new Http2UnencodedBackslashResponse(); + } + } + + @Path(UNENCODED_BACKSLASH_PATH) + @Produces("application/test.v1+json") + public static class Http2EncodedBackslashResource { + public static class Http2EncodedBackslashResponse { + @JsonProperty + public String getMessage() { + return "encoded-backslash"; + } + } + + @GET + @PerformanceMetric("test") + public Http2EncodedBackslashResponse hello() { + return new Http2EncodedBackslashResponse(); + } + } + @Path("/test%2Fambiguous%2Fsegment") @Produces("application/test.v1+json") public static class Http2TestAmbiguousSegmentResource {