From 34c7344522363a771a7ede3097c5598c94f2eb29 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 23 Sep 2021 08:16:38 +1000 Subject: [PATCH] Fix #6883 relative welcome redirect (#6886) Fix #6883 relative welcome redirect + make all redirects able to be relative + added test for relative redirection in ResourceService Signed-off-by: Greg Wilkins --- .../jetty/http/HttpFieldsMatchers.java | 6 ++ .../http/matchers/HttpFieldsHeaderValue.java | 60 +++++++++++++++++++ .../eclipse/jetty/server/ResourceService.java | 27 ++++----- .../jetty/servlet/DefaultServletTest.java | 59 +++++++++++++++--- 4 files changed, 129 insertions(+), 23 deletions(-) create mode 100644 jetty-http/src/test/java/org/eclipse/jetty/http/matchers/HttpFieldsHeaderValue.java diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsMatchers.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsMatchers.java index 29c0a1ecc7d7..3685d050170c 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsMatchers.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsMatchers.java @@ -20,6 +20,7 @@ import org.eclipse.jetty.http.matchers.HttpFieldsContainsHeaderKey; import org.eclipse.jetty.http.matchers.HttpFieldsContainsHeaderValue; +import org.eclipse.jetty.http.matchers.HttpFieldsHeaderValue; import org.hamcrest.Matcher; public class HttpFieldsMatchers @@ -34,6 +35,11 @@ public static Matcher containsHeader(HttpHeader header) return new HttpFieldsContainsHeaderKey(header); } + public static Matcher headerValue(String keyName, String value) + { + return new HttpFieldsHeaderValue(keyName, value); + } + public static Matcher containsHeaderValue(String keyName, String value) { return new HttpFieldsContainsHeaderValue(keyName, value); diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/matchers/HttpFieldsHeaderValue.java b/jetty-http/src/test/java/org/eclipse/jetty/http/matchers/HttpFieldsHeaderValue.java new file mode 100644 index 000000000000..047926279987 --- /dev/null +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/matchers/HttpFieldsHeaderValue.java @@ -0,0 +1,60 @@ +// +// ======================================================================== +// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.http.matchers; + +import java.util.Objects; + +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; +import org.hamcrest.Description; +import org.hamcrest.TypeSafeMatcher; + +public class HttpFieldsHeaderValue extends TypeSafeMatcher +{ + private final String keyName; + private final String value; + + public HttpFieldsHeaderValue(String keyName, String value) + { + this.keyName = keyName; + this.value = value; + } + + public HttpFieldsHeaderValue(HttpHeader header, String value) + { + this(header.asString(), value); + } + + @Override + public void describeTo(Description description) + { + description.appendText("expecting http header ").appendValue(keyName).appendText(" with value ").appendValue(value); + } + + @Override + protected boolean matchesSafely(HttpFields fields) + { + HttpField field = fields.getField(this.keyName); + if (field == null) + return false; + + return Objects.equals(this.value, field.getValue()); + } +} diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java index 9a0a2f7bfbee..f51b8274d38e 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java @@ -380,23 +380,20 @@ protected void sendWelcome(HttpContent content, String pathInContext, boolean en // Redirect to directory if (!endsWithSlash) { - StringBuffer buf = request.getRequestURL(); - synchronized (buf) + StringBuilder buf = new StringBuilder(request.getRequestURI()); + int param = buf.lastIndexOf(";"); + if (param < 0 || buf.lastIndexOf("/", param) > 0) + buf.append('/'); + else + buf.insert(param, '/'); + String q = request.getQueryString(); + if (q != null && q.length() != 0) { - int param = buf.lastIndexOf(";"); - if (param < 0) - buf.append('/'); - else - buf.insert(param, '/'); - String q = request.getQueryString(); - if (q != null && q.length() != 0) - { - buf.append('?'); - buf.append(q); - } - response.setContentLength(0); - response.sendRedirect(response.encodeRedirectURL(buf.toString())); + buf.append('?'); + buf.append(q); } + response.setContentLength(0); + response.sendRedirect(response.encodeRedirectURL(buf.toString())); return; } diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java index 85ac1d39fe91..45d5ae95c167 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java @@ -80,6 +80,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.eclipse.jetty.http.HttpFieldsMatchers.containsHeader; import static org.eclipse.jetty.http.HttpFieldsMatchers.containsHeaderValue; +import static org.eclipse.jetty.http.HttpFieldsMatchers.headerValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsString; @@ -897,20 +898,25 @@ public void testWelcomeRedirect() throws Exception rawResponse = connector.getResponse("GET /context/dir/ 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/dir/index.html")); + assertThat(response, headerValue("Location", "http://0.0.0.0/context/dir/index.html")); createFile(inde, "

Hello Inde

"); + rawResponse = connector.getResponse("GET /context/dir HTTP/1.0\r\n\r\n"); + response = HttpTester.parseResponse(rawResponse); + assertThat(response.toString(), response.getStatus(), is(HttpStatus.MOVED_TEMPORARILY_302)); + assertThat(response, headerValue("Location", "http://0.0.0.0/context/dir/")); + rawResponse = connector.getResponse("GET /context/dir/ 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/dir/index.html")); + assertThat(response, headerValue("Location", "http://0.0.0.0/context/dir/index.html")); if (deleteFile(index)) { rawResponse = connector.getResponse("GET /context/dir/ 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/dir/index.htm")); + assertThat(response, headerValue("Location", "http://0.0.0.0/context/dir/index.htm")); if (deleteFile(inde)) { @@ -921,6 +927,46 @@ public void testWelcomeRedirect() throws Exception } } + @Test + public void testRelativeRedirect() throws Exception + { + Path dir = docRoot.resolve("dir"); + FS.ensureDirExists(dir); + Path index = dir.resolve("index.html"); + createFile(index, "

Hello Index

"); + + context.addAliasCheck((p, r) -> true); + connector.getConnectionFactory(HttpConfiguration.ConnectionFactory.class).getHttpConfiguration().setRelativeRedirectAllowed(true); + + ServletHolder defholder = context.addServlet(DefaultServlet.class, "/"); + defholder.setInitParameter("dirAllowed", "false"); + defholder.setInitParameter("redirectWelcome", "true"); + defholder.setInitParameter("welcomeServlets", "false"); + defholder.setInitParameter("gzip", "false"); + + defholder.setInitParameter("maxCacheSize", "1024000"); + defholder.setInitParameter("maxCachedFileSize", "512000"); + defholder.setInitParameter("maxCachedFiles", "100"); + + String rawResponse; + HttpTester.Response response; + + rawResponse = connector.getResponse("GET /context/dir HTTP/1.0\r\n\r\n"); + response = HttpTester.parseResponse(rawResponse); + assertThat(response.toString(), response.getStatus(), is(HttpStatus.MOVED_TEMPORARILY_302)); + assertThat(response, headerValue("Location", "/context/dir/")); + + rawResponse = connector.getResponse("GET /context/dir/ HTTP/1.0\r\n\r\n"); + response = HttpTester.parseResponse(rawResponse); + assertThat(response.toString(), response.getStatus(), is(HttpStatus.MOVED_TEMPORARILY_302)); + assertThat(response, headerValue("Location", "/context/dir/index.html")); + + rawResponse = connector.getResponse("GET /context/dir/index.html/ HTTP/1.0\r\n\r\n"); + response = HttpTester.parseResponse(rawResponse); + assertThat(response.toString(), response.getStatus(), is(HttpStatus.MOVED_TEMPORARILY_302)); + assertThat(response, headerValue("Location", "/context/dir/index.html")); + } + /** * Ensure that oddball directory names are served with proper escaping */ @@ -1492,8 +1538,7 @@ public void testFiltered() throws Exception assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); body = response.getContent(); assertThat(response, containsHeaderValue(HttpHeader.CONTENT_LENGTH, "" + body.length())); - assertThat(response, containsHeaderValue(HttpHeader.CONTENT_TYPE, "text/plain")); - assertThat(response, containsHeaderValue(HttpHeader.CONTENT_TYPE, "charset=")); + assertThat(response, containsHeaderValue(HttpHeader.CONTENT_TYPE, "text/plain;charset=UTF-8")); assertThat(body, containsString("Extra Info")); rawResponse = connector.getResponse("GET /context/image.jpg HTTP/1.0\r\n\r\n"); @@ -1501,8 +1546,7 @@ public void testFiltered() throws Exception assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); body = response.getContent(); assertThat(response, containsHeaderValue(HttpHeader.CONTENT_LENGTH, "" + body.length())); - assertThat(response, containsHeaderValue(HttpHeader.CONTENT_TYPE, "image/jpeg")); - assertThat(response, containsHeaderValue(HttpHeader.CONTENT_TYPE, "charset=utf-8")); + assertThat(response, containsHeaderValue(HttpHeader.CONTENT_TYPE, "image/jpeg;charset=utf-8")); assertThat(body, containsString("Extra Info")); server.stop(); @@ -1516,7 +1560,6 @@ public void testFiltered() throws Exception assertThat(response.toString(), response.getStatus(), is(HttpStatus.OK_200)); body = response.getContent(); assertThat(response, containsHeaderValue(HttpHeader.CONTENT_TYPE, "text/plain")); - assertThat(response, not(containsHeaderValue(HttpHeader.CONTENT_TYPE, "charset="))); assertThat(body, containsString("Extra Info")); }