Skip to content

Commit

Permalink
Issue #12104 - tests for null connection response header.
Browse files Browse the repository at this point in the history
  • Loading branch information
joakime committed Aug 1, 2024
1 parent 58cfe77 commit 11e8faf
Showing 1 changed file with 226 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.List;
import java.util.function.Consumer;
import java.util.stream.Stream;

import jakarta.servlet.DispatcherType;
import jakarta.servlet.FilterChain;
Expand All @@ -31,6 +35,7 @@
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpServletResponseWrapper;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.UriCompliance;
Expand All @@ -39,12 +44,15 @@
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.toolchain.test.MavenPaths;
import org.eclipse.jetty.util.StringUtil;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class ResponseHeadersTest
Expand Down Expand Up @@ -150,11 +158,24 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t
}
}

private static Server server;
private static LocalConnector connector;
private Server server;
private LocalConnector connector;

@BeforeAll
public static void startServer() throws Exception
public void startServer() throws Exception
{
startServer((context) ->
{
context.addServlet(new ServletHolder(new DefaultServlet()), "/default/*");
context.addFilter(new FilterHolder(new WrappingFilter()), "/default/*", EnumSet.allOf(DispatcherType.class));
context.addServlet(new ServletHolder(new SimulateUpgradeServlet()), "/ws/*");
context.addServlet(new ServletHolder(new MultilineResponseValueServlet()), "/multiline/*");
context.addServlet(CharsetResetToJsonMimeTypeServlet.class, "/charset/json-reset/*");
context.addServlet(CharsetChangeToJsonMimeTypeServlet.class, "/charset/json-change/*");
context.addServlet(CharsetChangeToJsonMimeTypeSetCharsetToNullServlet.class, "/charset/json-change-null/*");
});
}

public void startServer(Consumer<ServletContextHandler> configureServletContext) throws Exception
{
Path staticContentPath = MavenPaths.findTestResourceDir("contextResources");
server = new Server();
Expand All @@ -165,21 +186,15 @@ public static void startServer() throws Exception
context.setContextPath("/");
context.setBaseResourceAsPath(staticContentPath);
context.setInitParameter("org.eclipse.jetty.servlet.Default.pathInfoOnly", "TRUE");
server.setHandler(context);

context.addServlet(new ServletHolder(new DefaultServlet()), "/default/*");
context.addFilter(new FilterHolder(new WrappingFilter()), "/default/*", EnumSet.allOf(DispatcherType.class));
context.addServlet(new ServletHolder(new SimulateUpgradeServlet()), "/ws/*");
context.addServlet(new ServletHolder(new MultilineResponseValueServlet()), "/multiline/*");
context.addServlet(CharsetResetToJsonMimeTypeServlet.class, "/charset/json-reset/*");
context.addServlet(CharsetChangeToJsonMimeTypeServlet.class, "/charset/json-change/*");
context.addServlet(CharsetChangeToJsonMimeTypeSetCharsetToNullServlet.class, "/charset/json-change-null/*");
configureServletContext.accept(context);

server.setHandler(context);
server.start();
}

@AfterAll
public static void stopServer()
@AfterEach
public void stopServer()
{
try
{
Expand Down Expand Up @@ -208,6 +223,7 @@ public void testWrappedResponseWithStaticContent() throws Exception
@Test
public void testResponseWebSocketHeaderFormat() throws Exception
{
startServer();
HttpTester.Request request = new HttpTester.Request();
request.setMethod("GET");
request.setURI("/ws/");
Expand All @@ -226,6 +242,7 @@ public void testResponseWebSocketHeaderFormat() throws Exception
@Test
public void testMultilineResponseHeaderValue() throws Exception
{
startServer();
connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE);
String actualPathInfo = "%0a%20Content-Type%3a%20image/png%0a%20Content-Length%3a%208%0A%20%0A%20yuck<!--";

Expand Down Expand Up @@ -254,6 +271,7 @@ public void testMultilineResponseHeaderValue() throws Exception
@Test
public void testCharsetResetToJsonMimeType() throws Exception
{
startServer();
HttpTester.Request request = new HttpTester.Request();
request.setMethod("GET");
request.setURI("/charset/json-reset/");
Expand All @@ -274,6 +292,7 @@ public void testCharsetResetToJsonMimeType() throws Exception
@Test
public void testCharsetChangeToJsonMimeType() throws Exception
{
startServer();
HttpTester.Request request = new HttpTester.Request();
request.setMethod("GET");
request.setURI("/charset/json-change/");
Expand All @@ -294,6 +313,7 @@ public void testCharsetChangeToJsonMimeType() throws Exception
@Test
public void testCharsetChangeToJsonMimeTypeSetCharsetToNull() throws Exception
{
startServer();
HttpTester.Request request = new HttpTester.Request();
request.setMethod("GET");
request.setURI("/charset/json-change-null/");
Expand All @@ -310,4 +330,194 @@ public void testCharsetChangeToJsonMimeTypeSetCharsetToNull() throws Exception
// The Content-Type should not have a charset= portion
assertThat("Response Header Content-Type", response.get("Content-Type"), is("application/json"));
}

public static Stream<Arguments> http10ConnectionBehavior()
{
List<Arguments> cases = new ArrayList<>();
String request;

// --- VALID HTTP/1.0 SPEC REQUESTS ---

// Request does not send connection header.
request = null;

cases.add(Arguments.of(request, null, "close"));
cases.add(Arguments.of(request, "close", "close"));
cases.add(Arguments.of(request, "keep-alive", "close"));

// Request sends "keep-alive"
request = "keep-alive";

cases.add(Arguments.of(request, null, "keep-alive"));
cases.add(Arguments.of(request, "close", "close"));
cases.add(Arguments.of(request, "keep-alive", "keep-alive"));

// --- INVALID HTTP/1.0 SPEC REQUESTS ---

// Request sends invalid value "close" (on HTTP/1.0)
request = "close";

cases.add(Arguments.of(request, null, "close"));
cases.add(Arguments.of(request, "close", "close"));
cases.add(Arguments.of(request, "keep-alive", "close"));

// --- INVALID HTTP RESPONSE HEADERS ---
// this is when the servlet is setting headers that are in conflict with
// the spec and each other.

// Request does not set "connection" header.
request = null;

cases.add(Arguments.of(request, "close, keep-alive", "close"));
cases.add(Arguments.of(request, "keep-alive, close", "close"));

// Request sends invalid value "close" (on HTTP/1.0)
request = "close";

// keep-alive is forbidden when not persistent
cases.add(Arguments.of(request, "close, keep-alive", "close"));
cases.add(Arguments.of(request, "keep-alive, close", "close"));

// Request sends "keep-alive"
request = "keep-alive";

// connection lists are preserved per hop-by-hop rules
cases.add(Arguments.of(request, "close, keep-alive", "close, keep-alive"));
cases.add(Arguments.of(request, "keep-alive, close", "keep-alive, close"));

return cases.stream();
}

@ParameterizedTest
@MethodSource("http10ConnectionBehavior")
public void testHTTP10ConnectionBehavior(String requestConnectionHeader, String responseConnectionHeader, String expectedConnectionHeader) throws Exception
{
startServer((context) ->
{
HttpServlet servlet = new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp)
{
if (responseConnectionHeader != null)
resp.setHeader("Connection", responseConnectionHeader);
}
};
ServletHolder servletHolder = new ServletHolder(servlet);
context.addServlet(servletHolder, "/nothing");
});

StringBuilder rawRequest = new StringBuilder();
rawRequest.append("GET /nothing HTTP/1.0\r\n");
rawRequest.append("Host: test\r\n");
if (requestConnectionHeader != null)
{
rawRequest.append("Connection: ").append(requestConnectionHeader).append("\r\n");
}
rawRequest.append("\r\n");

String rawResponse = connector.getResponse(rawRequest.toString());
HttpTester.Response response = HttpTester.parseResponse(rawResponse);

assertThat(response.getStatus(), is(200));
assertThat(response.getVersion(), is(HttpVersion.HTTP_1_1));
if (expectedConnectionHeader == null)
assertThat(response.get(HttpHeader.CONNECTION), nullValue());
else
assertThat(response.get(HttpHeader.CONNECTION), is(expectedConnectionHeader));
}

public static Stream<Arguments> http11ConnectionBehavior()
{
List<Arguments> cases = new ArrayList<>();
String request;

// --- VALID HTTP/1.1 SPEC REQUESTS ---

// Request does not send connection header.
request = null;

cases.add(Arguments.of(request, null, null));
cases.add(Arguments.of(request, "close", "close"));
cases.add(Arguments.of(request, "keep-alive", null));

// Request sends value "close"
request = "close";

cases.add(Arguments.of(request, null, "close"));
cases.add(Arguments.of(request, "close", "close"));
cases.add(Arguments.of(request, "keep-alive", "close"));

// --- INVALID HTTP/1.0 SPEC REQUESTS ---

// Request sends invalid "keep-alive" header value (on HTTP/1.1)
request = "keep-alive";

cases.add(Arguments.of(request, null, null));
cases.add(Arguments.of(request, "close", "close"));
cases.add(Arguments.of(request, "keep-alive", null));

// --- INVALID HTTP RESPONSE HEADERS ---
// this is when the servlet is setting headers that are in conflict with
// the spec and each other.

// Request does not set "connection" header.
request = null;

cases.add(Arguments.of(request, "close, keep-alive", "close"));
cases.add(Arguments.of(request, "keep-alive, close", "close"));

// Request sends value "close"
request = "close";

cases.add(Arguments.of(request, "close, keep-alive", "close"));
cases.add(Arguments.of(request, "keep-alive, close", "close"));

// Request sends invalid "keep-alive" header value (on HTTP/1.1)
request = "keep-alive";

cases.add(Arguments.of(request, "close, keep-alive", "close"));
cases.add(Arguments.of(request, "keep-alive, close", "close"));

return cases.stream();
}

@ParameterizedTest
@MethodSource("http11ConnectionBehavior")
public void testHTTP11ConnectionBehavior(String requestConnectionHeader, String responseConnectionHeader, String expectedConnectionHeader) throws Exception
{
startServer((context) ->
{
HttpServlet servlet = new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp)
{
if (responseConnectionHeader != null)
resp.setHeader("Connection", responseConnectionHeader);
}
};
ServletHolder servletHolder = new ServletHolder(servlet);
context.addServlet(servletHolder, "/nothing");
});

StringBuilder rawRequest = new StringBuilder();
rawRequest.append("GET /nothing HTTP/1.1\r\n");
rawRequest.append("Host: test\r\n");
if (requestConnectionHeader != null)
{
rawRequest.append("Connection: ").append(requestConnectionHeader).append("\r\n");
}
rawRequest.append("\r\n");

String rawResponse = connector.getResponse(rawRequest.toString());
HttpTester.Response response = HttpTester.parseResponse(rawResponse);

assertThat(response.getStatus(), is(200));
assertThat(response.getVersion(), is(HttpVersion.HTTP_1_1));
if (expectedConnectionHeader == null)
assertThat(response.get(HttpHeader.CONNECTION), nullValue());
else
assertThat(response.get(HttpHeader.CONNECTION), is(expectedConnectionHeader));
}
}

0 comments on commit 11e8faf

Please sign in to comment.