Merge pull request #10796 from jetty/fix/12.0.x/redirect-to-context-w…

…ith-query

Issue #10794 - fixing Moved Permanently handling of query strings

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java

@@ -828,11 +828,12 @@ protected boolean handleByContextHandler(String pathInContext, ContextRequest re
 
     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));

jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java

@@ -78,6 +78,7 @@
 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,34 +184,43 @@ public void testSimpleGET() throws Exception
         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();
 
         assertThat(stream.isComplete(), is(true));
         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();
 

jetty-ee9/jetty-ee9-servlet/src/test/java/org/eclipse/jetty/ee9/servlet/DefaultServletTest.java

@@ -745,6 +745,16 @@ public void testWelcomeMultipleBasesBase(WorkDir workDir) throws Exception
         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);