Issue #11014 - Improve Relative Redirect Handling in Jetty 10/11 (#11018

)

* Improve relative redirect handling #11014
* Common relative redirect method.
* Use the relative redirect method from redirection rules.

Signed-off-by: gregw <gregw@webtide.com>

jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectUtil.java

@@ -15,7 +15,7 @@
 
 import javax.servlet.http.HttpServletRequest;
 
-import org.eclipse.jetty.util.URIUtil;
+import org.eclipse.jetty.server.Response;
 
 /**
  * Utility for managing redirect based rules
@@ -32,33 +32,6 @@ public final class RedirectUtil
      */
     public static String toRedirectURL(final HttpServletRequest request, String location)
     {
-        if (!URIUtil.hasScheme(location))
-        {
-            StringBuilder url = new StringBuilder(128);
-            URIUtil.appendSchemeHostPort(url, request.getScheme(), request.getServerName(), request.getServerPort());
-
-            if (location.startsWith("/"))
-            {
-                // absolute in context
-                location = URIUtil.canonicalURI(location);
-            }
-            else
-            {
-                // relative to request
-                String path = request.getRequestURI();
-                String parent = (path.endsWith("/")) ? path : URIUtil.parentPath(path);
-                location = URIUtil.canonicalURI(URIUtil.addEncodedPaths(parent, location));
-                if (location != null && !location.startsWith("/"))
-                    url.append('/');
-            }
-
-            if (location == null)
-                throw new IllegalStateException("redirect path cannot be above root");
-            url.append(location);
-
-            location = url.toString();
-        }
-
-        return location;
+        return Response.toRedirectURI(request, location);
     }
 }

jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RedirectPatternRuleTest.java

@@ -17,6 +17,7 @@
 
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.server.HttpConnectionFactory;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -63,4 +64,34 @@ public void testPrefixPattern() throws IOException
         rule.apply("/api/rest?foo=1", _request, _response);
         assertRedirectResponse(HttpStatus.MOVED_PERMANENTLY_301, location);
     }
+
+    @Test
+    public void testRelativeRedirect() throws IOException
+    {
+        _connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setRelativeRedirectAllowed(true);
+        String location = "/foo/bar";
+
+        RedirectPatternRule rule = new RedirectPatternRule();
+        rule.setPattern("/api/*");
+        rule.setLocation(location);
+        rule.setStatusCode(HttpStatus.MOVED_PERMANENTLY_301);
+
+        rule.apply("/api/rest?foo=1", _request, _response);
+        assertRedirectResponse(HttpStatus.MOVED_PERMANENTLY_301, location);
+    }
+
+    @Test
+    public void testAbsoluteRedirect() throws IOException
+    {
+        _connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setRelativeRedirectAllowed(false);
+        String location = "/foo/bar";
+
+        RedirectPatternRule rule = new RedirectPatternRule();
+        rule.setPattern("/api/*");
+        rule.setLocation(location);
+        rule.setStatusCode(HttpStatus.MOVED_PERMANENTLY_301);
+
+        rule.apply("/api/rest?foo=1", _request, _response);
+        assertRedirectResponse(HttpStatus.MOVED_PERMANENTLY_301, "http://0.0.0.0" + location);
+    }
 }

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

@@ -28,6 +28,7 @@
 import javax.servlet.ServletResponse;
 import javax.servlet.ServletResponseWrapper;
 import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpServletResponseWrapper;
 import javax.servlet.http.HttpSession;
@@ -580,35 +581,7 @@ public void sendRedirect(int code, String location, boolean consumeAll) throws I
         if (!isMutable())
             return;
 
-        if (location == null)
-            throw new IllegalArgumentException();
-
-        if (!URIUtil.hasScheme(location))
-        {
-            StringBuilder buf = _channel.getHttpConfiguration().isRelativeRedirectAllowed()
-                ? new StringBuilder()
-                : _channel.getRequest().getRootURL();
-            if (location.startsWith("/"))
-            {
-                // absolute in context
-                location = URIUtil.canonicalURI(location);
-            }
-            else
-            {
-                // relative to request
-                String path = _channel.getRequest().getRequestURI();
-                String parent = (path.endsWith("/")) ? path : URIUtil.parentPath(path);
-                location = URIUtil.canonicalURI(URIUtil.addEncodedPaths(parent, location));
-                if (location != null && !location.startsWith("/"))
-                    buf.append('/');
-            }
-
-            if (location == null)
-                throw new IllegalStateException("path cannot be above root");
-            buf.append(location);
-
-            location = buf.toString();
-        }
+        location = toRedirectURI(_channel.getRequest(), location);
 
         resetBuffer();
         setHeader(HttpHeader.LOCATION, location);
@@ -622,6 +595,49 @@ public void sendRedirect(String location) throws IOException
         sendRedirect(HttpServletResponse.SC_MOVED_TEMPORARILY, location);
     }
 
+    /**
+     * Common point to generate a proper "Location" header for redirects.
+     *
+     * @param request the request the redirect should be based on (needed when relative locations are provided, so that
+     * server name, scheme, port can be built out properly)
+     * @param location the location as an absolute URI or an encoded relative path. A relative path starting with '/'
+     *                 is relative to the root, otherwise it is relative to the request path.
+     * @return the full redirect "Location" URL (including scheme, host, port, path, etc...)
+     */
+    public static String toRedirectURI(final HttpServletRequest request, String location)
+    {
+        // is the URI absolute already?
+        if (!URIUtil.hasScheme(location))
+        {
+            // The location is relative
+
+            // Is it relative to the request?
+            if (!location.startsWith("/"))
+            {
+                String path = request.getRequestURI();
+                String parent = (path.endsWith("/")) ? path : URIUtil.parentPath(path);
+                location = URIUtil.addEncodedPaths(parent, location);
+            }
+
+            // Normalize out any dot dot segments
+            location = URIUtil.canonicalURI(location);
+            if (location == null)
+                throw new IllegalStateException("redirect path cannot be above root");
+
+            // if relative redirects are not allowed?
+            Request baseRequest = Request.getBaseRequest(request);
+            if (baseRequest == null || !baseRequest.getHttpChannel().getHttpConfiguration().isRelativeRedirectAllowed())
+            {
+                // make the location an absolute URI
+                StringBuilder url = new StringBuilder(128);
+                URIUtil.appendSchemeHostPort(url, request.getScheme(), request.getServerName(), request.getServerPort());
+                url.append(location);
+                location = url.toString();
+            }
+        }
+        return location;
+    }
+
     @Override
     public void setDateHeader(String name, long date)
     {