This is an automated email from the ASF dual-hosted git repository. bonampak pushed a commit to branch feature/jakarta-jetty-upgrade in repository https://gitbox.apache.org/repos/asf/knox.git
commit 20bd04515676991bf6aec9dd67cb5fe02f22bd12 Author: bonampak <[email protected]> AuthorDate: Tue Apr 21 18:20:11 2026 +0200 KNOX-3238: refactor RequestUpdateHandler and PortMappingHelperHandler to Jetty 12 --- .../gateway/filter/PortMappingHelperHandler.java | 130 +++++++++++---------- .../knox/gateway/filter/RequestUpdateHandler.java | 102 ++++------------ 2 files changed, 90 insertions(+), 142 deletions(-) diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/filter/PortMappingHelperHandler.java b/gateway-server/src/main/java/org/apache/knox/gateway/filter/PortMappingHelperHandler.java index b838e8e9a..d0fd83ca9 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/filter/PortMappingHelperHandler.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/filter/PortMappingHelperHandler.java @@ -21,13 +21,12 @@ import static org.apache.knox.gateway.filter.AbstractGatewayFilter.DEFAULT_TOPOL import org.apache.knox.gateway.GatewayMessages; import org.apache.knox.gateway.config.GatewayConfig; import org.apache.knox.gateway.i18n.messages.MessagesFactory; +import org.eclipse.jetty.http.HttpURI; +import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.handler.HandlerWrapper; +import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.util.Callback; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import java.io.IOException; import java.util.Map; /** @@ -42,7 +41,7 @@ import java.util.Map; * Basically Topology Port Mapping for standard port. * Backwards compatible to Default Topology Feature. */ -public class PortMappingHelperHandler extends HandlerWrapper { +public class PortMappingHelperHandler extends Handler.Wrapper { private static final GatewayMessages LOG = MessagesFactory.get(GatewayMessages.class); private final GatewayConfig config; private final String defaultTopologyRedirectContext; @@ -53,65 +52,61 @@ public class PortMappingHelperHandler extends HandlerWrapper { } /** - * Set up context for default topology feature. + * Set up context for the default topology feature. * @param config GatewayConfig object to read from - * @return default topology redirect context as a string + * @return default topology redirect context as a string (or {@code null}) */ private String getDefaultTopologyRedirectContext(final GatewayConfig config) { final String defaultTopologyName = config.getDefaultTopologyName(); - // default topology feature can also be enabled using port mapping feature + // The default topology feature can also be enabled using port mapping feature // config e.g. gateway.port.mapping.{defaultTopologyName} - String defaultTopologyRedirectContext = null; - if(defaultTopologyName == null && - config.getGatewayPortMappings().containsValue(config.getGatewayPort())) { - for(final Map.Entry<String, Integer> entry: config.getGatewayPortMappings().entrySet()) { - if(entry.getValue().equals(config.getGatewayPort())) { - defaultTopologyRedirectContext = "/" + config.getGatewayPath() + "/" + entry.getKey(); + String redirectContext = null; + if (defaultTopologyName == null + && config.getGatewayPortMappings().containsValue(config.getGatewayPort())) { + for (final Map.Entry<String, Integer> entry : config.getGatewayPortMappings().entrySet()) { + if (entry.getValue().equals(config.getGatewayPort())) { + redirectContext = "/" + config.getGatewayPath() + "/" + entry.getKey(); break; } } } if (defaultTopologyName != null) { - defaultTopologyRedirectContext = config.getDefaultAppRedirectPath(); - if (defaultTopologyRedirectContext != null - && defaultTopologyRedirectContext.trim().isEmpty()) { - defaultTopologyRedirectContext = null; + redirectContext = config.getDefaultAppRedirectPath(); + if (redirectContext != null && redirectContext.trim().isEmpty()) { + redirectContext = null; } } - if (defaultTopologyRedirectContext != null) { - LOG.defaultTopologySetup(defaultTopologyName, defaultTopologyRedirectContext); + if (redirectContext != null) { + LOG.defaultTopologySetup(defaultTopologyName, redirectContext); } - return defaultTopologyRedirectContext; + return redirectContext; } @Override - public void handle(final String target, final Request baseRequest, - final HttpServletRequest request, final HttpServletResponse response) - throws IOException, ServletException { - final String baseURI = baseRequest.getRequestURI(); - final int port = baseRequest.getLocalPort(); + public boolean handle(final Request request, final Response response, final Callback callback) + throws Exception { + final String requestPath = request.getHttpURI().getPath(); + final int port = Request.getLocalPort(request); if (config.isGatewayPortMappingEnabled() - && config.getGatewayPortMappings().containsValue(port)) { + && config.getGatewayPortMappings().containsValue(port)) { // If Port Mapping feature enabled - handlePortMapping(target, baseRequest, request, response, port); - } else if (defaultTopologyRedirectContext != null && - !baseURI.startsWith("/" + config.getGatewayPath())) { + return handlePortMapping(request, response, callback, port); + } else if (defaultTopologyRedirectContext != null + && !requestPath.startsWith("/" + config.getGatewayPath())) { //Backwards compatibility for default topology feature - handleDefaultTopologyMapping(target, baseRequest, request, response); + return handleDefaultTopologyMapping(request, response, callback); } else { // case where topology port mapping is not enabled (or improperly configured) // and no default topology is configured - super.handle(target, baseRequest, request, response); + return super.handle(request, response, callback); } } - private void handlePortMapping(final String target, final Request baseRequest, - final HttpServletRequest request, - final HttpServletResponse response, final int port) - throws IOException, ServletException { + private boolean handlePortMapping(final Request request, final Response response, + final Callback callback, final int port) throws Exception { final String topologyName = config.getGatewayPortMappings().entrySet() .stream() .filter(e -> e.getValue().equals(port)) @@ -119,37 +114,46 @@ public class PortMappingHelperHandler extends HandlerWrapper { .findFirst() .orElse(null); final String gatewayTopologyContext = "/" + config.getGatewayPath() + "/" + topologyName; - String newTarget = target; + final String requestPath = request.getHttpURI().getPath(); - if(!target.contains(gatewayTopologyContext)) { - newTarget = gatewayTopologyContext + target; + // If the request URI does not already contain /{gatewayPath}/{topologyName}, + // wrap the request to prepend it. + if (!requestPath.contains(gatewayTopologyContext)) { + final String newPath = gatewayTopologyContext + requestPath; + LOG.topologyPortMappingUpdateRequest(requestPath, newPath); + + Request rewritten = rewritePath(request, newPath); + return super.handle(rewritten, response, callback); } - // if the request does not contain /{gatewayName}/{topologyName} - if(!baseRequest.getRequestURI().contains(gatewayTopologyContext)) { - RequestUpdateHandler.ForwardedRequest newRequest = new RequestUpdateHandler.ForwardedRequest( - request, gatewayTopologyContext); + return super.handle(request, response, callback); + } - baseRequest.setPathInfo(gatewayTopologyContext + baseRequest.getPathInfo()); - baseRequest.setURIPathQuery(gatewayTopologyContext + baseRequest.getRequestURI()); + private boolean handleDefaultTopologyMapping(final Request request, final Response response, + final Callback callback) throws Exception { + final String requestPath = request.getHttpURI().getPath(); + final String newPath = defaultTopologyRedirectContext + requestPath; + LOG.defaultTopologyForward(requestPath, newPath); - LOG.topologyPortMappingUpdateRequest(target, newTarget); - super.handle(newTarget, baseRequest, newRequest, response); - } else { - super.handle(newTarget, baseRequest, request, response); - } + Request rewritten = rewritePath(request, newPath); + rewritten.setAttribute(DEFAULT_TOPOLOGY_FORWARD_ATTRIBUTE_NAME, "true"); + return super.handle(rewritten, response, callback); } - private void handleDefaultTopologyMapping(final String target, final Request baseRequest, - final HttpServletRequest request, - final HttpServletResponse response) - throws IOException, ServletException { - RequestUpdateHandler.ForwardedRequest newRequest = new RequestUpdateHandler.ForwardedRequest( - request, defaultTopologyRedirectContext); - - final String newTarget = defaultTopologyRedirectContext + target; - LOG.defaultTopologyForward(target, newTarget); - request.setAttribute(DEFAULT_TOPOLOGY_FORWARD_ATTRIBUTE_NAME, "true"); - super.handle(newTarget, baseRequest, newRequest, response); + /** + * Wraps the given request so that its {@link HttpURI} reports the supplied + * path instead of the original one. All other URI components (scheme, + * authority, query, fragment) are preserved. + */ + private static Request rewritePath(final Request request, final String newPath) { + final HttpURI original = request.getHttpURI(); + final HttpURI rewritten = HttpURI.build(original).path(newPath).asImmutable(); + + return new Request.Wrapper(request) { + @Override + public HttpURI getHttpURI() { + return rewritten; + } + }; } -} +} \ No newline at end of file diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/filter/RequestUpdateHandler.java b/gateway-server/src/main/java/org/apache/knox/gateway/filter/RequestUpdateHandler.java index 9d1020204..c8f5e24aa 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/filter/RequestUpdateHandler.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/filter/RequestUpdateHandler.java @@ -16,112 +16,56 @@ */ package org.apache.knox.gateway.filter; -import org.apache.commons.lang3.StringUtils; -import org.apache.knox.gateway.GatewayMessages; -import org.apache.knox.gateway.config.GatewayConfig; -import org.apache.knox.gateway.i18n.messages.MessagesFactory; -import org.apache.knox.gateway.services.GatewayServices; -import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.handler.ScopedHandler; - -import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletRequestWrapper; -import jakarta.servlet.http.HttpServletResponse; -import java.io.IOException; import java.util.Locale; /** - * This handler will be ONLY registered with a specific connector listening on a - * port that is configured through a property gateway.port.mapping.{topologyName} - * in gateway-site.xml - * <p> - * The function of this connector is to append the right context path and - * forward the request to the default port. + * Container for the {@link ForwardedRequest} wrapper used by the port-mapping / + * default-topology request rewriting logic (see KNOX-928). * <p> - * See KNOX-928 - * + * The former Jetty {@code ScopedHandler} implementation was removed as part of + * the Jetty 12 migration because {@code ScopedHandler} no longer exists and the + * rewriting logic now lives directly inside {@link PortMappingHelperHandler}. + * Only the request wrapper is retained here for backwards compatibility with + * existing call sites and tests. */ -public class RequestUpdateHandler extends ScopedHandler { - private static final GatewayMessages LOG = MessagesFactory.get(GatewayMessages.class); - - private String redirectContext; - - public RequestUpdateHandler(final GatewayConfig config, - final String topologyName, final GatewayServices services) { - super(); - - if (config == null) { - throw new IllegalArgumentException("config==null"); - } - if (services == null) { - throw new IllegalArgumentException("services==null"); - } - if (topologyName == null) { - throw new IllegalArgumentException("topologyName==null"); - } +public final class RequestUpdateHandler { - redirectContext = "/" + config.getGatewayPath() + "/" + topologyName; - } - - @Override - public void doScope(final String target, final Request baseRequest, - final HttpServletRequest request, final HttpServletResponse response) - throws IOException, ServletException { - nextScope(target, baseRequest, request, response); - } - - @Override - public void doHandle(final String target, final Request baseRequest, - final HttpServletRequest request, final HttpServletResponse response) - throws IOException, ServletException { - - RequestUpdateHandler.ForwardedRequest newRequest = new RequestUpdateHandler.ForwardedRequest( - request, redirectContext); - - // if the request already has the /{gatewaypath}/{topology} part then skip - if (!StringUtils.startsWithIgnoreCase(target, redirectContext)) { - baseRequest.setPathInfo(redirectContext + baseRequest.getPathInfo()); - baseRequest.setURIPathQuery(redirectContext + baseRequest.getRequestURI()); - - final String newTarget = redirectContext + target; - LOG.topologyPortMappingUpdateRequest(target, newTarget); - nextHandle(newTarget, baseRequest, newRequest, response); - } else { - nextHandle(target, baseRequest, newRequest, response); - } + private RequestUpdateHandler() { + // utility holder for the ForwardedRequest wrapper } /** - * A request wrapper class that wraps a request and adds the context path if - * needed. + * A request wrapper class that wraps a request and prepends a context path + * to the request URI. */ - static class ForwardedRequest extends HttpServletRequestWrapper { + public static class ForwardedRequest extends HttpServletRequestWrapper { private final String contextPath; private final String requestURL; - ForwardedRequest(final HttpServletRequest request, final String contextPath) { + public ForwardedRequest(final HttpServletRequest request, final String contextPath) { super(request); this.contextPath = contextPath; this.requestURL = generateRequestURL(); } /** - * Handle the case where getServerPort returns -1 + * Handle the case where getServerPort returns -1. * @return requestURL */ private String generateRequestURL() { if (getRequest().getServerPort() != -1) { return String.format(Locale.ROOT, "%s://%s:%s%s", - getRequest().getScheme(), - getRequest().getServerName(), - getRequest().getServerPort(), - getRequestURI()); + getRequest().getScheme(), + getRequest().getServerName(), + getRequest().getServerPort(), + getRequestURI()); } else { return String.format(Locale.ROOT, "%s://%s%s", - getRequest().getScheme(), - getRequest().getServerName(), - getRequestURI()); + getRequest().getScheme(), + getRequest().getServerName(), + getRequestURI()); } } @@ -140,4 +84,4 @@ public class RequestUpdateHandler extends ScopedHandler { return this.contextPath; } } -} +} \ No newline at end of file
