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

Issue #10794 - fixing Moved Permanently handling of query strings
This commit is contained in:
Joakim Erdfelt 2023-10-26 13:32:12 -05:00 committed by GitHub
commit 3608d67f05
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 27 additions and 6 deletions

View File

@ -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));

View File

@ -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();

View File

@ -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);