diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index cdfc7b62c43..3dee12f0c6f 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -828,11 +828,12 @@ public class ContextHandler extends Handler.Wrapper implements Attributes, Alias protected void handleMovedPermanently(Request request, Response response, Callback callback) { + // TODO: should this be a fully qualified URI? (with scheme and host?) String location = _contextPath + "/"; if (request.getHttpURI().getParam() != null) location += ";" + request.getHttpURI().getParam(); if (request.getHttpURI().getQuery() != null) - location += ";" + request.getHttpURI().getQuery(); + location += "?" + request.getHttpURI().getQuery(); response.setStatus(HttpStatus.MOVED_PERMANENTLY_301); response.getHeaders().add(new HttpField(HttpHeader.LOCATION, location)); diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java index 9d6166b5c78..e486d9d06d2 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java @@ -78,6 +78,7 @@ import org.junit.jupiter.api.Disabled; 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.CsvSource; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; @@ -183,18 +184,25 @@ public class ContextHandlerTest assertThat(BufferUtil.toString(stream.getResponseContent()), equalTo(helloHandler.getMessage())); } - @Test - public void testNullPath() throws Exception + @ParameterizedTest + @CsvSource({ + "http://localhost/ctx,/ctx/", + "http://localhost/ctx;a=b,/ctx/;a=b", + "http://localhost/ctx?a=b,/ctx/?a=b", + "http://localhost/ctx?a=b/c/d,/ctx/?a=b/c/d", + }) + public void testContextMovedPermanently(String requestUri, String expectedLocation) throws Exception { HelloHandler helloHandler = new HelloHandler(); _contextHandler.setHandler(helloHandler); _server.start(); + // First request is assuming contextHandler.setAllowNullPathInContext(false) ConnectionMetaData connectionMetaData = new MockConnectionMetaData(new MockConnector(_server)); HttpChannel channel = new HttpChannelState(connectionMetaData); MockHttpStream stream = new MockHttpStream(channel); HttpFields fields = HttpFields.build().add(HttpHeader.HOST, "localhost").asImmutable(); - MetaData.Request request = new MetaData.Request("GET", HttpURI.from("http://localhost/ctx"), HttpVersion.HTTP_1_1, fields, 0); + MetaData.Request request = new MetaData.Request("GET", HttpURI.from(requestUri), HttpVersion.HTTP_1_1, fields, 0); Runnable task = channel.onRequest(request); task.run(); @@ -202,15 +210,17 @@ public class ContextHandlerTest assertThat(stream.getFailure(), nullValue()); assertThat(stream.getResponse(), notNullValue()); assertThat(stream.getResponse().getStatus(), equalTo(301)); - assertThat(stream.getResponseHeaders().get(HttpHeader.LOCATION), equalTo("/ctx/")); + assertThat(stream.getResponseHeaders().get(HttpHeader.LOCATION), equalTo(expectedLocation)); _contextHandler.stop(); + + // Next request is assuming contextHandler.setAllowNullPathInContext(true) _contextHandler.setAllowNullPathInContext(true); _contextHandler.start(); stream = new MockHttpStream(channel); fields = HttpFields.build().add(HttpHeader.HOST, "localhost").asImmutable(); - request = new MetaData.Request("GET", HttpURI.from("http://localhost/ctx"), HttpVersion.HTTP_1_1, fields, 0); + request = new MetaData.Request("GET", HttpURI.from(requestUri), HttpVersion.HTTP_1_1, fields, 0); task = channel.onRequest(request); task.run(); diff --git a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java index 1dd1f70e9d9..483b520670e 100644 --- a/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java +++ b/jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java @@ -745,6 +745,16 @@ public class DefaultServletTest assertThat(response.toString(), response.getStatus(), is(HttpStatus.MOVED_TEMPORARILY_302)); assertThat(response, containsHeaderValue("Location", "http://0.0.0.0/context/other/")); + rawResponse = connector.getResponse("GET /context/other?a=b HTTP/1.0\r\n\r\n"); + response = HttpTester.parseResponse(rawResponse); + assertThat(response.toString(), response.getStatus(), is(HttpStatus.MOVED_TEMPORARILY_302)); + assertThat(response, containsHeaderValue("Location", "http://0.0.0.0/context/other/?a=b")); + + rawResponse = connector.getResponse("GET /context?a=b HTTP/1.0\r\n\r\n"); + response = HttpTester.parseResponse(rawResponse); + assertThat(response.toString(), response.getStatus(), is(HttpStatus.MOVED_PERMANENTLY_301)); + assertThat(response, containsHeaderValue("Location", "/context/?a=b")); + // Test alt default rawResponse = connector.getResponse("GET /context/alt/dir/ HTTP/1.0\r\n\r\n"); response = HttpTester.parseResponse(rawResponse);