From 41e9842921169d4f3b13872776acbd5272e86bc1 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 3 Nov 2022 16:13:46 +0100 Subject: [PATCH] Fixes #8858 - Jetty 12 Review MovedContextHandler. (#8859) * Fixes #8858 - Jetty 12 Review MovedContextHandler. Updated method names to avod references to Servlet concepts. Added test cases. Updated XML files. Signed-off-by: Simone Bordet --- .../server/handler/MovedContextHandler.java | 174 +++++++++----- .../handler/MovedContextHandlerTest.java | 220 ++++++++++++++++++ .../modules/demo.d/ee8-demo-moved-context.xml | 8 +- .../modules/demo.d/demo-moved-context.xml | 8 +- 4 files changed, 339 insertions(+), 71 deletions(-) create mode 100644 jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/MovedContextHandlerTest.java diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/MovedContextHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/MovedContextHandler.java index 83f007439933..57103e85a1da 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/MovedContextHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/MovedContextHandler.java @@ -13,9 +13,11 @@ package org.eclipse.jetty.server.handler; +import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpURI; +import org.eclipse.jetty.http.PreEncodedHttpField; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; @@ -23,122 +25,168 @@ import org.eclipse.jetty.util.URIUtil; /** - * Moved ContextHandler. - * This context can be used to replace a context that has changed - * location. Requests are redirected (either to a fixed URL or to a - * new context base). + *

A {@code ContextHandler} with a child {@code Handler} + * that redirects to a configurable URI.

*/ public class MovedContextHandler extends ContextHandler { - final Redirector _redirector; - String _newContextURL; - boolean _discardPathInfo; - boolean _discardQuery; - boolean _permanent; - String _expires; + private int _statusCode = HttpStatus.SEE_OTHER_303; + private String _redirectURI; + private boolean _discardPathInContext = true; + private boolean _discardQuery = true; + private HttpField _cacheControl; public MovedContextHandler() { - _redirector = new Redirector(); - setHandler(_redirector); + setHandler(new Redirector()); setAllowNullPathInContext(true); } - public MovedContextHandler(Handler.Collection parent, String contextPath, String newContextURL) + public MovedContextHandler(Handler.Collection parent, String contextPath, String redirectURI) { - super(contextPath); parent.addHandler(this); - _newContextURL = newContextURL; - _redirector = new Redirector(); - setHandler(_redirector); + setContextPath(contextPath); + setRedirectURI(redirectURI); } - public boolean isDiscardPathInfo() + /** + * @return the redirect status code, by default 303 + */ + public int getStatusCode() { - return _discardPathInfo; + return _statusCode; } - public void setDiscardPathInfo(boolean discardPathInfo) + /** + * @param statusCode the redirect status code + * @throws IllegalArgumentException if the status code is not of type redirect (3xx) + */ + public void setStatusCode(int statusCode) { - _discardPathInfo = discardPathInfo; + if (!HttpStatus.isRedirection(statusCode)) + throw new IllegalArgumentException("Invalid HTTP redirection status code: " + statusCode); + _statusCode = statusCode; } - public String getNewContextURL() + /** + * @return the URI to redirect to + */ + public String getRedirectURI() { - return _newContextURL; + return _redirectURI; } - public void setNewContextURL(String newContextURL) + /** + *

Sets the URI to redirect to.

+ *

If the redirect URI is not absolute, the original request scheme + * and authority will be used to build the redirect URI.

+ *

The original request {@link Request#getPathInContext(Request) pathInContext} + * will be appended to the redirect URI path, unless {@link #isDiscardPathInContext()}.

+ *

The original request query will be preserved in the redirect URI, unless + * {@link #isDiscardQuery()}.

+ * + * @param redirectURI the URI to redirect to + */ + public void setRedirectURI(String redirectURI) { - _newContextURL = newContextURL; + _redirectURI = redirectURI; } - public boolean isPermanent() + /** + * @return whether the original request {@code pathInContext} is discarded + */ + public boolean isDiscardPathInContext() { - return _permanent; + return _discardPathInContext; } - public void setPermanent(boolean permanent) + /** + *

Whether to discard the original request {@link Request#getPathInContext(Request) pathInContext} + * when building the redirect URI.

+ * + * @param discardPathInContext whether the original request {@code pathInContext} is discarded + * @see #setRedirectURI(String) + */ + public void setDiscardPathInContext(boolean discardPathInContext) { - _permanent = permanent; + _discardPathInContext = discardPathInContext; } + /** + * @return whether the original request query is discarded + */ public boolean isDiscardQuery() { return _discardQuery; } + /** + *

Whether to discard the original request query + * when building the redirect URI.

+ * + * @param discardQuery whether the original request query is discarded + */ public void setDiscardQuery(boolean discardQuery) { _discardQuery = discardQuery; } + /** + * @return the {@code Cache-Control} header value or {@code null} + */ + public String getCacheControl() + { + return _cacheControl == null ? null : _cacheControl.getValue(); + } + + /** + * @param cacheControl the {@code Cache-Control} header value or {@code null} + */ + public void setCacheControl(String cacheControl) + { + _cacheControl = cacheControl == null ? null : new PreEncodedHttpField(HttpHeader.CACHE_CONTROL, cacheControl); + } + private class Redirector extends Handler.Processor { @Override public void process(Request request, Response response, Callback callback) throws Exception { - if (_newContextURL == null) - return; + String redirectURI = getRedirectURI(); + if (redirectURI == null) + redirectURI = "/"; - String path = _newContextURL; - String pathInContext = Request.getPathInContext(request); - if (!_discardPathInfo && pathInContext != null) - path = URIUtil.addPaths(path, pathInContext); + HttpURI.Mutable redirectHttpURI = HttpURI.build(redirectURI); + if (redirectHttpURI.getScheme() == null) + { + HttpURI httpURI = request.getHttpURI(); + redirectHttpURI = redirectHttpURI.scheme(httpURI.getScheme()) + .authority(httpURI.getAuthority()); + } - HttpURI uri = request.getHttpURI(); - StringBuilder location = new StringBuilder(); - location.append(uri.getScheme()).append("://").append(uri.getAuthority()).append(path); + if (!isDiscardPathInContext()) + { + String pathInContext = Request.getPathInContext(request); + String newPath = redirectHttpURI.getPath(); + redirectHttpURI.path(URIUtil.addPaths(newPath, pathInContext)); + } - if (!_discardQuery && uri.getQuery() != null) + if (!isDiscardQuery()) { - location.append('?'); - String q = uri.getQuery(); - q = q.replaceAll("\r\n?&=", "!"); - location.append(q); + String query = request.getHttpURI().getQuery(); + String newQuery = redirectHttpURI.getQuery(); + redirectHttpURI.query(URIUtil.addQueries(query, newQuery)); } - response.getHeaders().put(HttpHeader.LOCATION, location.toString()); - if (_expires != null) - response.getHeaders().put(HttpHeader.EXPIRES, _expires); - response.setStatus(_permanent ? HttpStatus.MOVED_PERMANENTLY_301 : HttpStatus.FOUND_302); - callback.succeeded(); - } - } + response.setStatus(getStatusCode()); - /** - * @return the expires header value or null if no expires header - */ - public String getExpires() - { - return _expires; - } + response.getHeaders().put(HttpHeader.LOCATION, redirectHttpURI.asString()); - /** - * @param expires the expires header value or null if no expires header - */ - public void setExpires(String expires) - { - _expires = expires; + HttpField cacheControl = _cacheControl; + if (cacheControl != null) + response.getHeaders().put(cacheControl); + + callback.succeeded(); + } } } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/MovedContextHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/MovedContextHandlerTest.java new file mode 100644 index 000000000000..f0f6cb818876 --- /dev/null +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/MovedContextHandlerTest.java @@ -0,0 +1,220 @@ +// +// ======================================================================== +// Copyright (c) 1995-2022 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.server.handler; + +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.server.LocalConnector; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.util.component.LifeCycle; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.endsWith; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +public class MovedContextHandlerTest +{ + private Server server; + private LocalConnector connector; + + private void start(MovedContextHandler handler) throws Exception + { + server = new Server(); + connector = new LocalConnector(server); + server.addConnector(connector); + + server.setHandler(handler); + + server.start(); + } + + @AfterEach + public void dispose() + { + LifeCycle.stop(server); + } + + @Test + public void testRelativeURIDiscardPathInContextDiscardQuery() throws Exception + { + MovedContextHandler handler = new MovedContextHandler(); + handler.setContextPath("/ctx"); + handler.setRedirectURI("/moved"); + handler.setDiscardPathInContext(true); + handler.setDiscardQuery(true); + start(handler); + + HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(""" + GET /ctx/path?query HTTP/1.1 + Host: localhost + + """)); + + assertEquals(HttpStatus.SEE_OTHER_303, response.getStatus()); + String location = response.get(HttpHeader.LOCATION); + assertNotNull(location); + assertThat(location, endsWith("/moved")); + } + + @Test + public void testRelativeURIPreservePathInContextDiscardQuery() throws Exception + { + MovedContextHandler handler = new MovedContextHandler(); + handler.setContextPath("/ctx"); + handler.setRedirectURI("/moved"); + handler.setDiscardPathInContext(false); + handler.setDiscardQuery(true); + start(handler); + + HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(""" + GET /ctx/path?query HTTP/1.1 + Host: localhost + + """)); + + assertEquals(HttpStatus.SEE_OTHER_303, response.getStatus()); + String location = response.get(HttpHeader.LOCATION); + assertNotNull(location); + assertThat(location, endsWith("/moved/path")); + } + + @Test + public void testRelativeURIPreservePathInContextPreserveQuery() throws Exception + { + MovedContextHandler handler = new MovedContextHandler(); + handler.setContextPath("/ctx"); + handler.setRedirectURI("/moved"); + handler.setDiscardPathInContext(false); + handler.setDiscardQuery(false); + start(handler); + + HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(""" + GET /ctx/path?query HTTP/1.1 + Host: localhost + + """)); + + assertEquals(HttpStatus.SEE_OTHER_303, response.getStatus()); + String location = response.get(HttpHeader.LOCATION); + assertNotNull(location); + assertThat(location, endsWith("/moved/path?query")); + } + + @Test + public void testRelativeURIPreservePathInContextCoalesceQuery() throws Exception + { + MovedContextHandler handler = new MovedContextHandler(); + handler.setContextPath("/ctx"); + handler.setRedirectURI("/moved?a=b"); + handler.setDiscardPathInContext(false); + handler.setDiscardQuery(false); + start(handler); + + HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(""" + GET /ctx/path?query HTTP/1.1 + Host: localhost + + """)); + + assertEquals(HttpStatus.SEE_OTHER_303, response.getStatus()); + String location = response.get(HttpHeader.LOCATION); + assertNotNull(location); + assertThat(location, endsWith("/moved/path?query&a=b")); + } + + @Test + public void testAbsoluteURIPreservePathInContextPreserveQuery() throws Exception + { + MovedContextHandler handler = new MovedContextHandler(); + handler.setContextPath("/ctx"); + handler.setRedirectURI("https://host/moved-path"); + handler.setDiscardPathInContext(false); + handler.setDiscardQuery(false); + start(handler); + + HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(""" + GET /ctx/path?query HTTP/1.1 + Host: localhost + + """)); + + assertEquals(HttpStatus.SEE_OTHER_303, response.getStatus()); + String location = response.get(HttpHeader.LOCATION); + assertNotNull(location); + assertThat(location, endsWith("https://host/moved-path/path?query")); + } + + @Test + public void testCacheControl() throws Exception + { + MovedContextHandler handler = new MovedContextHandler(); + handler.setContextPath("/ctx"); + handler.setRedirectURI("/moved"); + start(handler); + + HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(""" + GET /ctx/path?query HTTP/1.1 + Host: localhost + + """)); + + assertEquals(HttpStatus.SEE_OTHER_303, response.getStatus()); + String location = response.get(HttpHeader.LOCATION); + assertNotNull(location); + assertThat(location, endsWith("/moved")); + assertFalse(response.contains(HttpHeader.CACHE_CONTROL)); + + handler.setCacheControl("max-age=5"); + + response = HttpTester.parseResponse(connector.getResponse(""" + GET /ctx/path?query HTTP/1.1 + Host: localhost + + """)); + + assertEquals(HttpStatus.SEE_OTHER_303, response.getStatus()); + location = response.get(HttpHeader.LOCATION); + assertNotNull(location); + assertThat(location, endsWith("/moved")); + String cacheControl = response.get(HttpHeader.CACHE_CONTROL); + assertNotNull(cacheControl); + assertEquals("max-age=5", cacheControl); + } + + @Test + public void testStatusCode() throws Exception + { + MovedContextHandler handler = new MovedContextHandler(); + handler.setContextPath("/ctx"); + handler.setRedirectURI("/moved"); + handler.setStatusCode(HttpStatus.MOVED_PERMANENTLY_301); + start(handler); + + HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(""" + GET /ctx/path?query HTTP/1.1 + Host: localhost + + """)); + + assertEquals(HttpStatus.MOVED_PERMANENTLY_301, response.getStatus()); + String location = response.get(HttpHeader.LOCATION); + assertNotNull(location); + assertThat(location, endsWith("/moved")); + } +} diff --git a/jetty-ee8/jetty-ee8-demos/jetty-ee8-demo-jetty-webapp/src/main/config/modules/demo.d/ee8-demo-moved-context.xml b/jetty-ee8/jetty-ee8-demos/jetty-ee8-demo-jetty-webapp/src/main/config/modules/demo.d/ee8-demo-moved-context.xml index c9595b71ce36..3d243a27d018 100644 --- a/jetty-ee8/jetty-ee8-demos/jetty-ee8-demo-jetty-webapp/src/main/config/modules/demo.d/ee8-demo-moved-context.xml +++ b/jetty-ee8/jetty-ee8-demos/jetty-ee8-demo-jetty-webapp/src/main/config/modules/demo.d/ee8-demo-moved-context.xml @@ -4,9 +4,9 @@ /oldContextPath - /test/dump/moved - false - false + /test/dump/moved + 302 + false false - -1 + no-cache diff --git a/jetty-home/src/main/resources/modules/demo.d/demo-moved-context.xml b/jetty-home/src/main/resources/modules/demo.d/demo-moved-context.xml index e9b46c12db7e..ed67b61ae093 100644 --- a/jetty-home/src/main/resources/modules/demo.d/demo-moved-context.xml +++ b/jetty-home/src/main/resources/modules/demo.d/demo-moved-context.xml @@ -4,9 +4,9 @@ /oldContextPath - /ee10-test/dump/moved - false - false + /ee10-test/dump/moved + 302 + false false - -1 + no-cache