Skip to content

Commit

Permalink
Updated WebSocketHandler to correctly propagate query params from web…
Browse files Browse the repository at this point in the history
…server to Tyrus. Moved UriComponent over to common/http for sharing purposes. Updated tests to verify changes. (#4647)

Signed-off-by: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
  • Loading branch information
spericas committed Aug 1, 2022
1 parent 954e1ba commit 058ef43
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 23 deletions.
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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
* <p>
* 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() {
}
Expand All @@ -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);
}

Expand All @@ -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()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -60,6 +64,7 @@ class WebSocketHandler extends SimpleChannelInboundHandler<Object> {

private final WebSocketEngine engine;
private final String path;
private final String queryString;
private final FullHttpRequest upgradeRequest;
private final HttpHeaders upgradeResponseHeaders;
private final WebSocketRouting webSocketRouting;
Expand All @@ -72,7 +77,14 @@ class WebSocketHandler extends SimpleChannelInboundHandler<Object> {
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;
Expand Down Expand Up @@ -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<String, String[]> 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
Expand Down
1 change: 1 addition & 0 deletions webserver/websocket/src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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");
}
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit 058ef43

Please sign in to comment.