Fix #6883 relative welcome redirect (#6886) (#6894)

* 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 <gregw@webtide.com>

jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java

@@ -389,23 +389,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;
         }
 

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

@@ -74,6 +74,7 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.eclipse.jetty.http.tools.matchers.HttpFieldsMatchers.containsHeader;
 import static org.eclipse.jetty.http.tools.matchers.HttpFieldsMatchers.containsHeaderValue;
+import static org.eclipse.jetty.http.tools.matchers.HttpFieldsMatchers.headerValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.anyOf;
 import static org.hamcrest.Matchers.containsString;
@@ -881,20 +882,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, "<h1>Hello Inde</h1>");
+        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))
             {
@@ -905,6 +911,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, "<h1>Hello Index</h1>");
+
+        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
      */
@@ -1477,17 +1523,15 @@ 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");
         response = HttpTester.parseResponse(rawResponse);
         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();
@@ -1501,7 +1545,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"));
     }
 

tests/jetty-http-tools/src/main/java/org/eclipse/jetty/http/tools/matchers/HttpFieldsHeaderValue.java

@@ -0,0 +1,55 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.http.tools.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<HttpFields>
+{
+    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());
+    }
+}

tests/jetty-http-tools/src/main/java/org/eclipse/jetty/http/tools/matchers/HttpFieldsMatchers.java

@@ -29,6 +29,11 @@ public static Matcher<HttpFields> containsHeader(HttpHeader header)
         return new HttpFieldsContainsHeaderKey(header);
     }
 
+    public static Matcher<HttpFields> headerValue(String keyName, String value)
+    {
+        return new HttpFieldsHeaderValue(keyName, value);
+    }
+
     public static Matcher<HttpFields> containsHeaderValue(String keyName, String value)
     {
         return new HttpFieldsContainsHeaderValue(keyName, value);