From 058ef43b6b2aeb8ff6087329f25f044dd0eba4d1 Mon Sep 17 00:00:00 2001 From: Santiago Pericasgeertsen Date: Mon, 1 Aug 2022 13:06:29 -0400 Subject: [PATCH] Updated WebSocketHandler to correctly propagate query params from webserver to Tyrus. Moved UriComponent over to common/http for sharing purposes. Updated tests to verify changes. (#4647) Signed-off-by: Santiago Pericasgeertsen --- .../io/helidon/common/http}/UriComponent.java | 16 ++++--------- .../common/http}/UriComponentTest.java | 6 ++--- .../java/io/helidon/webserver/Request.java | 1 + .../webserver/websocket/WebSocketHandler.java | 23 ++++++++++++++++--- .../websocket/src/main/java/module-info.java | 1 + .../websocket/test/EchoEndpoint.java | 21 +++++++++++++++++ .../websocket/test/EchoServiceTest.java | 4 ++-- .../websocket/test/HttpClientTest.java | 2 +- .../webserver/websocket/test/RoutingTest.java | 4 ++-- 9 files changed, 55 insertions(+), 23 deletions(-) rename {webserver/webserver/src/main/java/io/helidon/webserver => common/http/src/main/java/io/helidon/common/http}/UriComponent.java (88%) rename {webserver/webserver/src/test/java/io/helidon/webserver => common/http/src/test/java/io/helidon/common/http}/UriComponentTest.java (94%) diff --git a/webserver/webserver/src/main/java/io/helidon/webserver/UriComponent.java b/common/http/src/main/java/io/helidon/common/http/UriComponent.java similarity index 88% rename from webserver/webserver/src/main/java/io/helidon/webserver/UriComponent.java rename to common/http/src/main/java/io/helidon/common/http/UriComponent.java index f8be1df7c40..1f64e728ae4 100644 --- a/webserver/webserver/src/main/java/io/helidon/webserver/UriComponent.java +++ b/common/http/src/main/java/io/helidon/common/http/UriComponent.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2021 Oracle and/or its affiliates. + * Copyright (c) 2022 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,24 +14,18 @@ * limitations under the License. */ -package io.helidon.webserver; +package io.helidon.common.http; import java.net.URLDecoder; -import io.helidon.common.http.HashParameters; -import io.helidon.common.http.Parameters; - import static java.nio.charset.StandardCharsets.UTF_8; /** * Extracted from Jersey *

* Utility class for validating, encoding and decoding components of a URI. - * - * @author Paul Sandoz - * @author Marek Potociar (marek.potociar at oracle.com) */ -final class UriComponent { +public final class UriComponent { private UriComponent() { } @@ -47,7 +41,7 @@ private UriComponent() { * should be in decoded form. * @return the multivalued map of query parameters. */ - static Parameters decodeQuery(String query, boolean decode) { + public static Parameters decodeQuery(String query, boolean decode) { return decodeQuery(query, true, decode); } @@ -64,7 +58,7 @@ static Parameters decodeQuery(String query, boolean decode) { * should be in decoded form. * @return the multivalued map of query parameters. */ - static Parameters decodeQuery(String query, boolean decodeNames, boolean decodeValues) { + public static Parameters decodeQuery(String query, boolean decodeNames, boolean decodeValues) { Parameters queryParameters = HashParameters.create(); if (query == null || query.isEmpty()) { diff --git a/webserver/webserver/src/test/java/io/helidon/webserver/UriComponentTest.java b/common/http/src/test/java/io/helidon/common/http/UriComponentTest.java similarity index 94% rename from webserver/webserver/src/test/java/io/helidon/webserver/UriComponentTest.java rename to common/http/src/test/java/io/helidon/common/http/UriComponentTest.java index db26ac03e2a..0d90ee4cd3a 100644 --- a/webserver/webserver/src/test/java/io/helidon/webserver/UriComponentTest.java +++ b/common/http/src/test/java/io/helidon/common/http/UriComponentTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2021 Oracle and/or its affiliates. + * Copyright (c) 2022 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,14 +14,12 @@ * limitations under the License. */ -package io.helidon.webserver; +package io.helidon.common.http; import java.net.URI; import java.net.URLEncoder; import java.util.Optional; -import io.helidon.common.http.Parameters; - import org.junit.jupiter.api.Test; import static java.nio.charset.StandardCharsets.US_ASCII; diff --git a/webserver/webserver/src/main/java/io/helidon/webserver/Request.java b/webserver/webserver/src/main/java/io/helidon/webserver/Request.java index 574402ac074..8fd2adc7a0a 100644 --- a/webserver/webserver/src/main/java/io/helidon/webserver/Request.java +++ b/webserver/webserver/src/main/java/io/helidon/webserver/Request.java @@ -33,6 +33,7 @@ import io.helidon.common.http.Http; import io.helidon.common.http.MediaType; import io.helidon.common.http.Parameters; +import io.helidon.common.http.UriComponent; import io.helidon.common.reactive.Single; import io.helidon.media.common.MessageBodyContext; import io.helidon.media.common.MessageBodyReadableContent; diff --git a/webserver/websocket/src/main/java/io/helidon/webserver/websocket/WebSocketHandler.java b/webserver/websocket/src/main/java/io/helidon/webserver/websocket/WebSocketHandler.java index 56b450c1566..e8f1e71c839 100644 --- a/webserver/websocket/src/main/java/io/helidon/webserver/websocket/WebSocketHandler.java +++ b/webserver/websocket/src/main/java/io/helidon/webserver/websocket/WebSocketHandler.java @@ -20,13 +20,17 @@ import java.io.IOException; import java.net.URI; import java.nio.ByteBuffer; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; +import io.helidon.common.http.Parameters; +import io.helidon.common.http.UriComponent; import io.helidon.common.reactive.BufferedEmittingPublisher; import io.helidon.common.reactive.Multi; @@ -60,6 +64,7 @@ class WebSocketHandler extends SimpleChannelInboundHandler { private final WebSocketEngine engine; private final String path; + private final String queryString; private final FullHttpRequest upgradeRequest; private final HttpHeaders upgradeResponseHeaders; private final WebSocketRouting webSocketRouting; @@ -72,7 +77,14 @@ class WebSocketHandler extends SimpleChannelInboundHandler { FullHttpRequest upgradeRequest, HttpHeaders upgradeResponseHeaders, WebSocketRouting webSocketRouting) { - this.path = path; + int k = path.indexOf('?'); + if (k > 0) { + this.path = path.substring(0, k); + this.queryString = path.substring(k + 1); + } else { + this.path = path; + this.queryString = ""; + } this.upgradeRequest = upgradeRequest; this.upgradeResponseHeaders = upgradeResponseHeaders; this.webSocketRouting = webSocketRouting; @@ -166,11 +178,16 @@ private void sendBytesToTyrus(ChannelHandlerContext ctx, ByteBuffer nioBuffer) { WebSocketEngine.UpgradeInfo upgrade(ChannelHandlerContext ctx) { LOGGER.fine("Initiating WebSocket handshake ..."); - // Create Tyrus request context and copy request headers + + // Create Tyrus request context, copy request headers and query params + Map paramsMap = new HashMap<>(); + Parameters params = UriComponent.decodeQuery(queryString, true); + params.toMap().forEach((key, value) -> paramsMap.put(key, value.toArray(new String[0]))); RequestContext requestContext = RequestContext.Builder.create() .requestURI(URI.create(path)) // excludes context path + .queryString(queryString) + .parameterMap(paramsMap) .build(); - upgradeRequest.headers().forEach(e -> requestContext.getHeaders().put(e.getKey(), List.of(e.getValue()))); // Use Tyrus to process a WebSocket upgrade request diff --git a/webserver/websocket/src/main/java/module-info.java b/webserver/websocket/src/main/java/module-info.java index c0dbb9ccc1e..cf2eaf5f9ca 100644 --- a/webserver/websocket/src/main/java/module-info.java +++ b/webserver/websocket/src/main/java/module-info.java @@ -25,6 +25,7 @@ requires java.logging; requires transitive io.helidon.webserver; + requires io.helidon.common.http; requires transitive jakarta.websocket; requires io.netty.transport; requires io.netty.handler; diff --git a/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/EchoEndpoint.java b/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/EchoEndpoint.java index 957e6424533..2b42e767efd 100644 --- a/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/EchoEndpoint.java +++ b/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/EchoEndpoint.java @@ -63,6 +63,24 @@ private static void verifyRunningThread(Session session, Logger logger) throws I } } + /** + * Verify session includes expected query params. + * + * @param session Websocket session. + * @param logger A logger. + * @throws IOException Exception during close. + */ + private static void verifyQueryParams(Session session, Logger logger) throws IOException { + if (!"user=Helidon".equals(session.getQueryString())) { + logger.warning("Websocket session does not include required query params"); + session.close(); + } + if (!session.getRequestParameterMap().get("user").get(0).equals("Helidon")) { + logger.warning("Websocket session does not include required query parameter map"); + session.close(); + } + } + public static class ServerConfigurator extends ServerEndpointConfig.Configurator { @Override @@ -77,6 +95,7 @@ public void modifyHandshake(ServerEndpointConfig sec, HandshakeRequest request, public void onOpen(Session session) throws IOException { LOGGER.info("OnOpen called"); verifyRunningThread(session, LOGGER); + verifyQueryParams(session, LOGGER); if (!modifyHandshakeCalled.get()) { session.close(); // unexpected } @@ -86,6 +105,7 @@ public void onOpen(Session session) throws IOException { public void echo(Session session, String message) throws Exception { LOGGER.info("Endpoint OnMessage called '" + message + "'"); verifyRunningThread(session, LOGGER); + verifyQueryParams(session, LOGGER); if (!isDecoded(message)) { throw new InternalError("Message has not been decoded"); } @@ -102,6 +122,7 @@ public void onError(Throwable t) { public void onClose(Session session) throws IOException { LOGGER.info("OnClose called"); verifyRunningThread(session, LOGGER); + verifyQueryParams(session, LOGGER); modifyHandshakeCalled.set(false); } } diff --git a/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/EchoServiceTest.java b/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/EchoServiceTest.java index 7dc3d397a7a..ef918ad9a8f 100644 --- a/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/EchoServiceTest.java +++ b/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/EchoServiceTest.java @@ -37,7 +37,7 @@ public static void startServer() throws Exception { @Test public void testEchoSingle() { try { - URI uri = URI.create("ws://localhost:" + webServer().port() + "/tyrus/echo"); + URI uri = URI.create("ws://localhost:" + webServer().port() + "/tyrus/echo?user=Helidon"); new EchoClient(uri).echo("One"); } catch (Exception e) { fail("Unexpected exception " + e); @@ -47,7 +47,7 @@ public void testEchoSingle() { @Test public void testEchoMultiple() { try { - URI uri = URI.create("ws://localhost:" + webServer().port() + "/tyrus/echo"); + URI uri = URI.create("ws://localhost:" + webServer().port() + "/tyrus/echo?user=Helidon"); new EchoClient(uri).echo("One", "Two", "Three"); } catch (Exception e) { fail("Unexpected exception " + e); diff --git a/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/HttpClientTest.java b/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/HttpClientTest.java index 85593def5f9..0e5486382a4 100644 --- a/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/HttpClientTest.java +++ b/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/HttpClientTest.java @@ -42,7 +42,7 @@ static void startServer() throws ExecutionException, InterruptedException, Timeo @Test void testJdkClient() throws ExecutionException, InterruptedException, TimeoutException { - URI uri = URI.create("ws://localhost:" + webServer().port() + "/tyrus/echo"); + URI uri = URI.create("ws://localhost:" + webServer().port() + "/tyrus/echo?user=Helidon"); ClientListener listener = new ClientListener(); WebSocket webSocket = HttpClient.newHttpClient() diff --git a/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/RoutingTest.java b/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/RoutingTest.java index 9f656fd03b2..07028971cf3 100644 --- a/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/RoutingTest.java +++ b/webserver/websocket/src/test/java/io/helidon/webserver/websocket/test/RoutingTest.java @@ -37,7 +37,7 @@ public static void startServer() throws Exception { @Test public void testEcho() { try { - URI uri = URI.create("ws://localhost:" + webServer().port() + "/tyrus/echo"); + URI uri = URI.create("ws://localhost:" + webServer().port() + "/tyrus/echo?user=Helidon"); new EchoClient(uri).echo("One"); } catch (Exception e) { fail("Unexpected exception " + e); @@ -47,7 +47,7 @@ public void testEcho() { @Test public void testDoubleEcho() { try { - URI uri = URI.create("ws://localhost:" + webServer().port() + "/tyrus/doubleEcho"); + URI uri = URI.create("ws://localhost:" + webServer().port() + "/tyrus/doubleEcho?user=Helidon"); new EchoClient(uri, (s1, s2) -> s2.equals(s1 + s1)).echo("One"); } catch (Exception e) { fail("Unexpected exception " + e);