Skip to content

Commit

Permalink
Allow enabling Jetty's case sensitive header cache
Browse files Browse the repository at this point in the history
See: https://bugs.eclipse.org/bugs/show_bug.cgi?id=414449#c4

Jetty re-writes headers based on a cache. This can result in headers
being sent to clients with a cached value that has a different case
then what was sent from the remote client. In applications that must
validate headers for hash signatures this is a severe problem as the
value that was sent by the client is lost.

Add a binding that makes Jetty's header cache case sensitive. This
is a documented feature of Jetty.
  • Loading branch information
Randgalt authored and wendigo committed Jun 4, 2024
1 parent c6522ea commit 538e1ff
Show file tree
Hide file tree
Showing 17 changed files with 127 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- Support generic list and set injection into configuration classes
- Update Jetty to 12.0.10
- Update HdrHistogram to 2.2.2
- Allow configuring Jetty's header cache case-sensitiveness

248

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* 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.airlift.http.server;

import com.google.inject.BindingAnnotation;

import java.lang.annotation.Retention;
import java.lang.annotation.Target;

import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

@Retention(RUNTIME)
@Target({FIELD, PARAMETER, METHOD})
@BindingAnnotation
public @interface EnableCaseSensitiveHeaderCache
{
}
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ public HttpServer(
Set<Filter> adminFilters,
boolean enableVirtualThreads,
boolean enableLegacyUriCompliance,
boolean enableCaseSensitiveHeaderCache,
ClientCertificate clientCertificate,
MBeanServer mbeanServer,
LoginService loginService,
Expand Down Expand Up @@ -183,6 +184,9 @@ public HttpServer(
baseHttpConfiguration.setResponseHeaderSize(toIntExact(config.getMaxResponseHeaderSize().toBytes()));
}

// see https://bugs.eclipse.org/bugs/show_bug.cgi?id=414449#c4
baseHttpConfiguration.setHeaderCacheCaseSensitive(enableCaseSensitiveHeaderCache);

if (enableLegacyUriCompliance) {
// allow encoded slashes to occur in URI paths
UriCompliance uriCompliance = UriCompliance.from(EnumSet.of(AMBIGUOUS_PATH_SEPARATOR, AMBIGUOUS_PATH_ENCODING, SUSPICIOUS_PATH_CHARACTERS));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ public HttpServerBinder enableVirtualThreads()
return this;
}

public HttpServerBinder enableCaseSensitiveHeaderCache()
{
newOptionalBinder(binder, Key.get(Boolean.class, EnableCaseSensitiveHeaderCache.class)).setBinding().toInstance(true);
return this;
}

/**
* @deprecated this will be removed in the near future and is only intended as a stopgap
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ protected void setup(Binder binder)
newSetBinder(binder, Filter.class, TheAdminServlet.class);
newSetBinder(binder, HttpResourceBinding.class, TheServlet.class);
newOptionalBinder(binder, SslContextFactory.Server.class);
newOptionalBinder(binder, Key.get(Boolean.class, EnableCaseSensitiveHeaderCache.class)).setDefault().toInstance(false);

newExporter(binder).export(RequestStats.class).withGeneratedName();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public class HttpServerProvider
private Map<String, String> adminServletInitParameters = ImmutableMap.of();
private final boolean enableVirtualThreads;
private final boolean enableLegacyUriCompliance;
private final boolean enableCaseSensitiveHeaderCache;
private MBeanServer mbeanServer;
private LoginService loginService;
private final RequestStats stats;
Expand All @@ -78,6 +79,7 @@ public HttpServerProvider(HttpServerInfo httpServerInfo,
@TheAdminServlet Set<Filter> adminFilters,
@EnableVirtualThreads boolean enableVirtualThreads,
@EnableLegacyUriCompliance boolean enableLegacyUriCompliance,
@EnableCaseSensitiveHeaderCache boolean enableCaseSensitiveHeaderCache,
ClientCertificate clientCertificate,
RequestStats stats,
EventClient eventClient,
Expand Down Expand Up @@ -106,6 +108,7 @@ public HttpServerProvider(HttpServerInfo httpServerInfo,
this.adminFilters = ImmutableSet.copyOf(adminFilters);
this.enableVirtualThreads = enableVirtualThreads;
this.enableLegacyUriCompliance = enableLegacyUriCompliance;
this.enableCaseSensitiveHeaderCache = enableCaseSensitiveHeaderCache;
this.clientCertificate = clientCertificate;
this.stats = stats;
this.eventClient = eventClient;
Expand Down Expand Up @@ -166,6 +169,7 @@ public HttpServer get()
adminFilters,
enableVirtualThreads,
enableLegacyUriCompliance,
enableCaseSensitiveHeaderCache,
clientCertificate,
mbeanServer,
loginService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.inject.Inject;
import io.airlift.event.client.NullEventClient;
import io.airlift.http.server.EnableCaseSensitiveHeaderCache;
import io.airlift.http.server.EnableLegacyUriCompliance;
import io.airlift.http.server.EnableVirtualThreads;
import io.airlift.http.server.HttpServer;
Expand Down Expand Up @@ -51,7 +52,7 @@ public TestingHttpServer(
@TheServlet Map<String, String> initParameters)
throws IOException
{
this(httpServerInfo, nodeInfo, config, servlet, initParameters, false, false);
this(httpServerInfo, nodeInfo, config, servlet, initParameters, false, false, false);
}

public TestingHttpServer(
Expand All @@ -61,7 +62,8 @@ public TestingHttpServer(
@TheServlet Servlet servlet,
@TheServlet Map<String, String> initParameters,
boolean enableVirtualThreads,
boolean enableLegacyUriCompliance)
boolean enableLegacyUriCompliance,
boolean enableCaseSensitiveHeaderCache)
throws IOException
{
this(httpServerInfo,
Expand All @@ -74,6 +76,7 @@ public TestingHttpServer(
ImmutableSet.of(),
enableVirtualThreads,
enableLegacyUriCompliance,
enableCaseSensitiveHeaderCache,
ClientCertificate.NONE);
}

Expand All @@ -89,6 +92,7 @@ public TestingHttpServer(
@TheServlet Set<HttpResourceBinding> resources,
@EnableVirtualThreads boolean enableVirtualThreads,
@EnableLegacyUriCompliance boolean enableLegacyUriCompliance,
@EnableCaseSensitiveHeaderCache boolean enableCaseSensitiveHeaderCache,
ClientCertificate clientCertificate)
throws IOException
{
Expand All @@ -105,6 +109,7 @@ public TestingHttpServer(
ImmutableSet.of(),
enableVirtualThreads,
enableLegacyUriCompliance,
enableCaseSensitiveHeaderCache,
clientCertificate,
null,
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.inject.Scopes;
import io.airlift.configuration.AbstractConfigurationAwareModule;
import io.airlift.discovery.client.AnnouncementHttpServerInfo;
import io.airlift.http.server.EnableCaseSensitiveHeaderCache;
import io.airlift.http.server.EnableLegacyUriCompliance;
import io.airlift.http.server.EnableVirtualThreads;
import io.airlift.http.server.HttpServer;
Expand Down Expand Up @@ -69,6 +70,7 @@ protected void setup(Binder binder)
newSetBinder(binder, Filter.class, TheServlet.class);
newSetBinder(binder, HttpResourceBinding.class, TheServlet.class);
binder.bind(AnnouncementHttpServerInfo.class).to(LocalAnnouncementHttpServerInfo.class);
newOptionalBinder(binder, Key.get(Boolean.class, EnableCaseSensitiveHeaderCache.class)).setDefault().toInstance(false);

newOptionalBinder(binder, HttpsConfig.class);
install(conditionalModule(HttpServerConfig.class, HttpServerConfig::isHttpsEnabled, moduleBinder -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ private static HttpServer createServer(HttpServlet servlet, NodeInfo nodeInfo, H
ImmutableSet.of(),
false,
false,
false,
ClientCertificate.NONE,
new RequestStats(),
new NullEventClient(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,7 @@ private void createServer(HttpServlet servlet)
ImmutableSet.of(),
false,
false,
false,
clientCertificate,
new RequestStats(),
new NullEventClient(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ protected void service(HttpServletRequest request, HttpServletResponse response)
ImmutableSet.of(),
false,
false,
false,
ClientCertificate.NONE,
new RequestStats(),
new NullEventClient(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import io.airlift.http.client.HttpClientConfig;
import io.airlift.http.client.HttpStatus;
import io.airlift.http.client.HttpUriBuilder;
import io.airlift.http.client.Request;
import io.airlift.http.client.StatusResponseHandler.StatusResponse;
import io.airlift.http.client.StringResponseHandler;
import io.airlift.http.client.jetty.JettyHttpClient;
Expand Down Expand Up @@ -76,11 +77,13 @@ public abstract class AbstractTestTestingHttpServer
{
private final boolean enableVirtualThreads;
private final boolean enableLegacyUriCompliance;
private final boolean enableCaseSensitiveHeaderCache;

AbstractTestTestingHttpServer(boolean enableVirtualThreads, boolean enableLegacyUriCompliance)
AbstractTestTestingHttpServer(boolean enableVirtualThreads, boolean enableLegacyUriCompliance, boolean enableCaseSensitiveHeaderCache)
{
this.enableVirtualThreads = enableVirtualThreads;
this.enableLegacyUriCompliance = enableLegacyUriCompliance;
this.enableCaseSensitiveHeaderCache = enableCaseSensitiveHeaderCache;
}

@BeforeSuite
Expand All @@ -96,7 +99,7 @@ public void testInitialization()
skipUnlessJdkHasVirtualThreads();
DummyServlet servlet = new DummyServlet();
Map<String, String> params = ImmutableMap.of("sampleInitParameter", "the value");
TestingHttpServer server = createTestingHttpServer(enableVirtualThreads, enableLegacyUriCompliance, servlet, params);
TestingHttpServer server = createTestingHttpServer(enableVirtualThreads, enableLegacyUriCompliance, enableCaseSensitiveHeaderCache, servlet, params);

try {
server.start();
Expand All @@ -114,7 +117,7 @@ public void testRequest()
{
skipUnlessJdkHasVirtualThreads();
DummyServlet servlet = new DummyServlet();
TestingHttpServer server = createTestingHttpServer(enableVirtualThreads, enableLegacyUriCompliance, servlet, ImmutableMap.of());
TestingHttpServer server = createTestingHttpServer(enableVirtualThreads, enableLegacyUriCompliance, enableCaseSensitiveHeaderCache, servlet, ImmutableMap.of());

try {
server.start();
Expand All @@ -138,7 +141,7 @@ public void testFilteredRequest()
skipUnlessJdkHasVirtualThreads();
DummyServlet servlet = new DummyServlet();
DummyFilter filter = new DummyFilter();
TestingHttpServer server = createTestingHttpServerWithFilter(enableVirtualThreads, enableLegacyUriCompliance, servlet, ImmutableMap.of(), filter);
TestingHttpServer server = createTestingHttpServerWithFilter(enableVirtualThreads, enableLegacyUriCompliance, enableCaseSensitiveHeaderCache, servlet, ImmutableMap.of(), filter);

try {
server.start();
Expand Down Expand Up @@ -273,6 +276,47 @@ public void testGuiceInjectionWithResources()
}
}

@SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter")
@Test
public void testHeaderCaseSensitivity()
throws Exception
{
DummyServlet servlet = new DummyServlet();
TestingHttpServer server = createTestingHttpServer(enableVirtualThreads, enableLegacyUriCompliance, enableCaseSensitiveHeaderCache, servlet, ImmutableMap.of());

try {
server.start();

String contentType = "text/plain; charset=UTF-8";
String finalContentType = "text/plain; charset=utf-8";

try (HttpClient client = new JettyHttpClient(new HttpClientConfig().setConnectTimeout(new Duration(1, SECONDS)))) {
// run a few times to prime the Jetty cache
for (int i = 0; i < 3; ++i) {
Request request = prepareGet()
.setUri(server.getBaseUrl())
.setHeader(HttpHeaders.CONTENT_TYPE, (i > 1) ? finalContentType : contentType)
.build();
client.execute(request, createStatusResponseHandler());
}
}

String contentTypeHeader;
synchronized (servlet) {
contentTypeHeader = servlet.contentTypeHeader;
}
if (enableCaseSensitiveHeaderCache) {
assertEquals(contentTypeHeader, finalContentType);
}
else {
assertEquals(contentTypeHeader, contentType);
}
}
finally {
server.stop();
}
}

private void skipUnlessJdkHasVirtualThreads()
{
if (enableVirtualThreads && !VirtualThreads.areSupported()) {
Expand All @@ -290,29 +334,30 @@ private static void assertResource(URI baseUri, HttpClient client, String path,
assertEquals(data.getBody().trim(), contents);
}

private static TestingHttpServer createTestingHttpServer(boolean enableVirtualThreads, boolean enableLegacyUriCompliance, DummyServlet servlet, Map<String, String> params)
private static TestingHttpServer createTestingHttpServer(boolean enableVirtualThreads, boolean enableLegacyUriCompliance, boolean enableCaseSensitiveHeaderCache, DummyServlet servlet, Map<String, String> params)
throws IOException
{
NodeInfo nodeInfo = new NodeInfo("test");
HttpServerConfig config = new HttpServerConfig().setHttpPort(0);
HttpServerInfo httpServerInfo = new HttpServerInfo(config, nodeInfo);
return new TestingHttpServer(httpServerInfo, nodeInfo, config, servlet, params, enableVirtualThreads, enableLegacyUriCompliance);
return new TestingHttpServer(httpServerInfo, nodeInfo, config, servlet, params, enableVirtualThreads, enableLegacyUriCompliance, enableCaseSensitiveHeaderCache);
}

private static TestingHttpServer createTestingHttpServerWithFilter(boolean enableVirtualThreads, boolean enableLegacyUriCompliance, DummyServlet servlet, Map<String, String> params, DummyFilter filter)
private static TestingHttpServer createTestingHttpServerWithFilter(boolean enableVirtualThreads, boolean enableLegacyUriCompliance, boolean enableCaseSensitiveHeaderCache, DummyServlet servlet, Map<String, String> params, DummyFilter filter)
throws IOException
{
NodeInfo nodeInfo = new NodeInfo("test");
HttpServerConfig config = new HttpServerConfig().setHttpPort(0);
HttpServerInfo httpServerInfo = new HttpServerInfo(config, nodeInfo);
return new TestingHttpServer(httpServerInfo, nodeInfo, config, Optional.empty(), servlet, params, ImmutableSet.of(filter), ImmutableSet.of(), enableVirtualThreads, enableLegacyUriCompliance, ClientCertificate.NONE);
return new TestingHttpServer(httpServerInfo, nodeInfo, config, Optional.empty(), servlet, params, ImmutableSet.of(filter), ImmutableSet.of(), enableVirtualThreads, enableLegacyUriCompliance, enableCaseSensitiveHeaderCache, ClientCertificate.NONE);
}

static class DummyServlet
extends HttpServlet
{
private String sampleInitParam;
private int callCount;
private String contentTypeHeader;

@Override
public synchronized void init(ServletConfig config)
Expand All @@ -335,6 +380,7 @@ protected synchronized void doGet(HttpServletRequest req, HttpServletResponse re
{
++callCount;
resp.setStatus(HttpServletResponse.SC_OK);
contentTypeHeader = req.getHeader(HttpHeaders.CONTENT_TYPE);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ public class TestTestingHttpServer
{
TestTestingHttpServer()
{
super(false, false);
super(false, false, false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ public class TestTestingHttpServerWithAllEnabled
{
TestTestingHttpServerWithAllEnabled()
{
super(true, true);
super(true, true, true);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package io.airlift.http.server.testing;

public class TestTestingHttpServerWithCaseSensitiveHeaderCache
extends AbstractTestTestingHttpServer
{
TestTestingHttpServerWithCaseSensitiveHeaderCache()
{
super(false, false, true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ public class TestTestingHttpServerWithLegacyUriCompliance
{
TestTestingHttpServerWithLegacyUriCompliance()
{
super(false, true);
super(false, true, false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ public class TestTestingHttpServerWithVirtualThreads
{
TestTestingHttpServerWithVirtualThreads()
{
super(true, false);
super(true, false, false);
}
}

0 comments on commit 538e1ff

Please sign in to comment.