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 cc2859c79860501a2cc7effdd39478f0195d4b48 Author: bonampak <[email protected]> AuthorDate: Wed Aug 5 17:22:10 2026 +0200 KNOX-3238: cleanup some comments. --- .../java/org/apache/knox/gateway/GatewayServer.java | 2 +- .../apache/knox/gateway/trace/AccessHandler.java | 6 +++--- .../org/apache/knox/gateway/trace/TraceRequest.java | 1 - .../gateway/webshell/WebshellWebSocketAdapter.java | 2 -- .../gateway/websockets/GatewayWebsocketHandler.java | 21 ++------------------- .../gateway/websockets/KnoxWebSocketCreator.java | 6 +----- .../knox/gateway/websockets/MessageFailureTest.java | 2 +- .../WebsocketServerInitiatedMessageTest.java | 3 --- .../WebsocketServerInitiatedPingTest.java | 6 ------ 9 files changed, 8 insertions(+), 41 deletions(-) diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java index 881e42620..83148d701 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServer.java @@ -902,7 +902,7 @@ public class GatewayServer { context.setInitParameter("org.eclipse.jetty.ee10.servlet.Default.dirAllowed", "false"); ClassLoader jspClassLoader = new URLClassLoader(new URL[0], this.getClass().getClassLoader()); context.setClassLoader(jspClassLoader); - // NOTE: In Jetty 12 the max form content size and max form keys are + // In Jetty 12 the max form content size and max form keys are // configured server-wide via FormFields.MAX_LENGTH_ATTRIBUTE and // FormFields.MAX_FIELDS_ATTRIBUTE (see createJetty()). The per-context // setters were removed; server-level settings apply to all WebApps. diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/trace/AccessHandler.java b/gateway-server/src/main/java/org/apache/knox/gateway/trace/AccessHandler.java index 47aa5c056..81e8eeeb9 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/trace/AccessHandler.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/trace/AccessHandler.java @@ -36,17 +36,17 @@ public class AccessHandler extends AbstractLifeCycle implements RequestLog { TraceUtil.appendCorrelationContext(sb); long durationMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - request.getBeginNanoTime()); sb.append('|') - .append(Request.getRemoteAddr(request)) // Static helper or request.getConnectionMetaData().getRemoteSocketAddress() + .append(Request.getRemoteAddr(request)) .append('|') .append(request.getMethod()) .append('|') .append(request.getHttpURI().toString()) .append('|') - .append(request.getLength()) // .getContentLength() is now .getLength() + .append(request.getLength()) .append('|') .append(response.getStatus()) .append('|') - .append(Response.getContentBytesWritten(response)) // Use static helper for bytes written + .append(Response.getContentBytesWritten(response)) .append('|') .append(durationMillis); log.trace(sb); diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceRequest.java b/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceRequest.java index 8dbfe2743..6ef86c559 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceRequest.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/trace/TraceRequest.java @@ -52,7 +52,6 @@ class TraceRequest extends Request.Wrapper { Content.Chunk chunk = super.read(); if (chunk != null && log.isTraceEnabled()) { if (Content.Chunk.isFailure(chunk)) { - // Log that the request failed if you want return chunk; } ByteBuffer data = chunk.getByteBuffer(); diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/webshell/WebshellWebSocketAdapter.java b/gateway-server/src/main/java/org/apache/knox/gateway/webshell/WebshellWebSocketAdapter.java index 7ec9e4ac9..5fb439c1c 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/webshell/WebshellWebSocketAdapter.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/webshell/WebshellWebSocketAdapter.java @@ -119,8 +119,6 @@ public class WebshellWebSocketAdapter extends ProxyWebSocketAdapter { cleanup(); return; } - // In Jetty 12 the send is asynchronous: report send failures via the Callback, - // preserving the old IOException-based error handling semantics. session.sendText(message, Callback.from( () -> { /* success: nothing to do */ }, t -> { diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/websockets/GatewayWebsocketHandler.java b/gateway-server/src/main/java/org/apache/knox/gateway/websockets/GatewayWebsocketHandler.java index 14e983a52..d826865a7 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/websockets/GatewayWebsocketHandler.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/websockets/GatewayWebsocketHandler.java @@ -57,18 +57,15 @@ public class GatewayWebsocketHandler extends Handler.Wrapper { throw new IllegalStateException("GatewayWebsocketHandler must be attached to a Server before starting"); } - // 1. Get or create the global ServerWebSocketContainer (no ContextHandler needed) ServerWebSocketContainer container = ServerWebSocketContainer.ensure(server, null); configureServerWebSocketContainer(container); - // 3. Create the UpgradeHandler. this.wsHandler = new WebSocketUpgradeHandler(container); - // 4. PRESERVE THE CHAIN: This handler currently wraps PortMappingHelperHandler. + // Preserve the chain: this handler currently wraps PortMappingHelperHandler. // We must ensure the internal wsHandler also wraps it, so HTTP traffic flows downwards. this.wsHandler.setHandler(getHandler()); - // Start the internal handler this.wsHandler.start(); super.doStart(); @@ -97,23 +94,9 @@ public class GatewayWebsocketHandler extends Handler.Wrapper { container.setIdleTimeout(Duration.ofMillis(config.getWebsocketIdleTimeout())); - // 2. Map ALL incoming requests to our custom Knox routing creator + // Map all incoming requests to our custom Knox routing creator. // "regex|^/.*" acts as a catch-all interceptor. container.addMapping("regex|^/.*", new KnoxWebSocketCreator(config, services)); - - //removed in Jetty 12 container.setMaxBinaryMessageBufferSize(config.getWebsocketMaxBinaryMessageBufferSize()); - //removed in Jetty 12 container.setMaxTextMessageBufferSize(config.getWebsocketMaxTextMessageBufferSize()); - - //removed in Jetty 12 container.setAsyncWriteTimeout(config.getWebsocketAsyncWriteTimeout()); - // handled by the core HTTP connection idle timeouts and - // one can apply it directly to the asynchronous write execution - // (e.g., using CompletableFuture.orTimeout(duration, TimeUnit) when we call session.sendText(...)). - - // removed, idle timeout is used or specified in send() methods: - // container.setAsyncSendTimeout(config.getWebsocketAsyncWriteTimeout()); - - //same as setIdleTimeout: container.setDefaultMaxSessionIdleTimeout(config.getWebsocketIdleTimeout()); - } } diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/websockets/KnoxWebSocketCreator.java b/gateway-server/src/main/java/org/apache/knox/gateway/websockets/KnoxWebSocketCreator.java index a73db079c..02dacb159 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/websockets/KnoxWebSocketCreator.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/websockets/KnoxWebSocketCreator.java @@ -101,12 +101,10 @@ public class KnoxWebSocketCreator implements WebSocketCreator { @Override public Object createWebSocket(ServerUpgradeRequest req, ServerUpgradeResponse resp, Callback callback) throws Exception { try { - // 1. Get the raw HTTP URI from the Jetty 12 Request and convert it to ws URI final URI requestURI = WSURI.toWebsocket(req.getHttpURI().toURI()); - // Now Knox's regex will work if (isWebshellRequest(requestURI)) { - return handleWebshellRequest(req); // Note: Update handleWebshellRequest to accept ServerUpgradeRequest + return handleWebshellRequest(req); } final String backendURL = getMatchedBackendURL(requestURI); @@ -165,7 +163,6 @@ public class KnoxWebSocketCreator implements WebSocketCreator { @Override public void beforeRequest(final Map<String, List<String>> headers) { - // 1. Safely iterate over Jetty 12 HttpFields and copy them to the Jakarta map for (HttpField field : req.getHeaders()) { String headerName = field.getName(); if (!IGNORED_HEADERS.contains(headerName.toLowerCase(Locale.ROOT))) { @@ -174,7 +171,6 @@ public class KnoxWebSocketCreator implements WebSocketCreator { } } - // 2. Properly construct and override the Host header try { final URI backendURI = new URI(backendURL); diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/websockets/MessageFailureTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/websockets/MessageFailureTest.java index b2f6eb3d3..03063e6f2 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/websockets/MessageFailureTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/websockets/MessageFailureTest.java @@ -92,7 +92,7 @@ public class MessageFailureTest { */ @Test(timeout = 8000) public void testMessageBiggerThanDefault() throws Exception { - //Note: default is WebSocketConstants.DEFAULT_MAX_TEXT_MESSAGE_SIZE = 65536 + // WebSocketConstants.DEFAULT_MAX_TEXT_MESSAGE_SIZE = 65536 final String bigMessage = RandomStringUtils.randomAscii(66000); WebSocketContainer container = ContainerProvider.getWebSocketContainer(); diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/websockets/WebsocketServerInitiatedMessageTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/websockets/WebsocketServerInitiatedMessageTest.java index c148f516b..e643d5ab0 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/websockets/WebsocketServerInitiatedMessageTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/websockets/WebsocketServerInitiatedMessageTest.java @@ -120,9 +120,6 @@ public class WebsocketServerInitiatedMessageTest extends WebsocketEchoTestBase { public void onWebSocketOpen(Session session) { super.onWebSocketOpen(session); - // In Jetty 12, we send the text directly on the session and provide a Callback. - // We use Callback.NOOP since the original code passed null and ignored success/failure. - // BatchMode and manual flushing are handled automatically by the Jetty engine. session.sendText("echo", org.eclipse.jetty.websocket.api.Callback.NOOP); } } diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/websockets/WebsocketServerInitiatedPingTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/websockets/WebsocketServerInitiatedPingTest.java index 740a19067..a334e08c4 100644 --- a/gateway-server/src/test/java/org/apache/knox/gateway/websockets/WebsocketServerInitiatedPingTest.java +++ b/gateway-server/src/test/java/org/apache/knox/gateway/websockets/WebsocketServerInitiatedPingTest.java @@ -91,8 +91,6 @@ public class WebsocketServerInitiatedPingTest extends WebsocketEchoTestBase { try (jakarta.websocket.Session session = container.connectToServer(client, new URI(serverUri.toString() + "gateway/websocket/123foo456bar/channels"))) { assertThat(session.isOpen(), is(true)); - //session.getBasicRemote().sendText("Echo"); - // Wait for the backend server to receive the automatic PONG from Knox's JSR-356 container String pongPayload = pingHandler.socket.pongFuture.get(10000, TimeUnit.MILLISECONDS); assertThat(pongPayload, is("PingPong")); @@ -141,15 +139,11 @@ public class WebsocketServerInitiatedPingTest extends WebsocketEchoTestBase { final ByteBuffer binaryMessage = ByteBuffer.wrap( textMessage.getBytes(StandardCharsets.UTF_8)); - // In Jetty 12, we send the ping directly on the session and provide a Callback. - // We use Callback.NOOP since the original code didn't do anything on success/failure. - // BatchMode and manual flushing are handled automatically by the Jetty engine. session.sendPing(binaryMessage, org.eclipse.jetty.websocket.api.Callback.NOOP); } @Override public void onWebSocketFrame(Frame frame, org.eclipse.jetty.websocket.api.Callback callback) { - // Intercept PONG frames returning from Knox if (frame.getOpCode() == OpCode.PONG) { ByteBuffer payload = frame.getPayload(); if (payload != null) {
