From 8bd62372fe20991c946ac888e59864e8ad3b4cf4 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 20 Oct 2023 13:59:44 +1100 Subject: [PATCH] Issue #10749 - allow UpgradeListener to see Response in case of non-successful upgrade Signed-off-by: Lachlan Roberts --- .../core/client/CoreClientUpgradeRequest.java | 20 ++- .../tests/client/ClientResponseTest.java | 123 ++++++++++++++++++ 2 files changed, 138 insertions(+), 5 deletions(-) create mode 100644 jetty-core/jetty-websocket/jetty-websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientResponseTest.java diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-client/src/main/java/org/eclipse/jetty/websocket/core/client/CoreClientUpgradeRequest.java b/jetty-core/jetty-websocket/jetty-websocket-core-client/src/main/java/org/eclipse/jetty/websocket/core/client/CoreClientUpgradeRequest.java index 9edf55c14122..0d82b1e5a4f0 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-client/src/main/java/org/eclipse/jetty/websocket/core/client/CoreClientUpgradeRequest.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-client/src/main/java/org/eclipse/jetty/websocket/core/client/CoreClientUpgradeRequest.java @@ -247,9 +247,19 @@ public void onComplete(Result result) } URI requestURI = result.getRequest().getURI(); + Request request = result.getRequest(); Response response = result.getResponse(); - int responseStatusCode = response.getStatus(); - String responseLine = responseStatusCode + " " + response.getReason(); + int status = response.getStatus(); + String responseLine = status + " " + response.getReason(); + + if ((request.getVersion() == HttpVersion.HTTP_2 && status != HttpStatus.OK_200) || + (request.getVersion() == HttpVersion.HTTP_1_1 && status != HttpStatus.SWITCHING_PROTOCOLS_101)) + { + // We have failed to upgrade but have received a response, so notify the listener. + Throwable listenerError = notifyUpgradeListeners((listener) -> listener.onHandshakeResponse(request, response)); + if (listenerError != null) + LOG.warn("error from listener", listenerError); + } if (result.isFailed()) { @@ -266,15 +276,15 @@ public void onComplete(Result result) Throwable failure = result.getFailure(); boolean wrapFailure = !(failure instanceof IOException) && !(failure instanceof UpgradeException); if (wrapFailure) - failure = new UpgradeException(requestURI, responseStatusCode, responseLine, failure); + failure = new UpgradeException(requestURI, status, responseLine, failure); handleException(failure); return; } - if (responseStatusCode != HttpStatus.SWITCHING_PROTOCOLS_101) + if (status != HttpStatus.SWITCHING_PROTOCOLS_101) { // Failed to upgrade (other reason) - handleException(new UpgradeException(requestURI, responseStatusCode, + handleException(new UpgradeException(requestURI, status, "Failed to upgrade to websocket: Unexpected HTTP Response Status Code: " + responseLine)); } } diff --git a/jetty-core/jetty-websocket/jetty-websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientResponseTest.java b/jetty-core/jetty-websocket/jetty-websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientResponseTest.java new file mode 100644 index 000000000000..ee6d4112ab64 --- /dev/null +++ b/jetty-core/jetty-websocket/jetty-websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientResponseTest.java @@ -0,0 +1,123 @@ +// +// ======================================================================== +// Copyright (c) 1995 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.websocket.tests.client; + +import java.net.URI; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.client.Request; +import org.eclipse.jetty.client.Response; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.websocket.api.exceptions.UpgradeException; +import org.eclipse.jetty.websocket.client.ClientUpgradeRequest; +import org.eclipse.jetty.websocket.client.JettyUpgradeListener; +import org.eclipse.jetty.websocket.client.WebSocketClient; +import org.eclipse.jetty.websocket.server.WebSocketCreator; +import org.eclipse.jetty.websocket.server.WebSocketUpgradeHandler; +import org.eclipse.jetty.websocket.tests.EchoSocket; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class ClientResponseTest +{ + private Server _server; + private ServerConnector _connector; + private WebSocketClient _client; + + public void before(WebSocketCreator creator) throws Exception + { + _server = new Server(); + _connector = new ServerConnector(_server); + _server.addConnector(_connector); + + ContextHandler contextHandler = new ContextHandler(); + WebSocketUpgradeHandler upgradeHandler = WebSocketUpgradeHandler.from(_server, contextHandler); + contextHandler.setHandler(upgradeHandler); + _server.setHandler(contextHandler); + upgradeHandler.configure(container -> container.addMapping("/", creator)); + + _server.start(); + + _client = new WebSocketClient(); + _client.start(); + } + + @AfterEach + public void after() throws Exception + { + _client.stop(); + _server.stop(); + } + + @Test + public void testResponseOnUpgradeFailure() throws Exception + { + before((req, resp, cb) -> + { + resp.setStatus(HttpStatus.IM_A_TEAPOT_418); + resp.getHeaders().put("specialHeader", "value123"); + resp.write(true, BufferUtil.toBuffer("failed by test"), cb); + return null; + }); + + EchoSocket clientEndpoint = new EchoSocket(); + URI uri = URI.create("ws://localhost:" + _connector.getLocalPort()); + ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest(); + + CompletableFuture responseFuture = new CompletableFuture<>(); + CompletableFuture contentFuture = new CompletableFuture<>(); + JettyUpgradeListener upgradeListener = new JettyUpgradeListener() + { + @Override + public void onHandshakeRequest(Request request) + { + request.onResponseContentSource((resp, source) -> + assertDoesNotThrow(() -> contentFuture.complete(IO.toString(Content.Source.asInputStream(source))))); + } + + @Override + public void onHandshakeResponse(Request request, Response response) + { + responseFuture.complete(response); + } + }; + + Throwable t = assertThrows(Throwable.class, () -> + _client.connect(clientEndpoint, uri, upgradeRequest, upgradeListener).get(5, TimeUnit.SECONDS)); + assertThat(t, instanceOf(ExecutionException.class)); + assertThat(t.getCause(), instanceOf(UpgradeException.class)); + assertThat(t.getCause().getMessage(), containsString("Failed to upgrade to websocket: Unexpected HTTP Response Status Code: 418")); + + Response response = responseFuture.get(5, TimeUnit.SECONDS); + String content = contentFuture.get(5, TimeUnit.SECONDS); + assertThat(response.getStatus(), equalTo(HttpStatus.IM_A_TEAPOT_418)); + assertThat(response.getHeaders().get("specialHeader"), equalTo("value123")); + assertThat(content, equalTo("failed by test")); + } +}