From 7f2859ab49490ffdb1d01619c20f1c7e1830f60f Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 6 Dec 2023 20:12:00 +0000 Subject: [PATCH] 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 --- .../jetty/rewrite/handler/RedirectUtil.java | 31 +------- .../handler/RedirectPatternRuleTest.java | 31 ++++++++ .../org/eclipse/jetty/server/Response.java | 74 +++++++++++-------- 3 files changed, 78 insertions(+), 58 deletions(-) diff --git a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectUtil.java b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectUtil.java index 7ba12413d90..d04113dbe3d 100644 --- a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectUtil.java +++ b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectUtil.java @@ -15,7 +15,7 @@ package org.eclipse.jetty.rewrite.handler; 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); } } diff --git a/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RedirectPatternRuleTest.java b/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RedirectPatternRuleTest.java index ea94bce5095..61078c7ebb4 100644 --- a/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RedirectPatternRuleTest.java +++ b/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RedirectPatternRuleTest.java @@ -17,6 +17,7 @@ import java.io.IOException; 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 class RedirectPatternRuleTest extends AbstractRuleTestCase 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); + } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index 34747a40529..6aefdf29a34 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -28,6 +28,7 @@ import javax.servlet.ServletOutputStream; 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 class Response implements HttpServletResponse 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 class Response implements HttpServletResponse 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) {