From 5ac4dc559c6a1ace634c6d49ebfdccbc92306331 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Thu, 25 Feb 2021 16:47:46 -0800 Subject: [PATCH 1/4] [Java] Fix skip negotiate null ref --- .../com/microsoft/signalr/HubConnection.java | 11 +++- .../microsoft/signalr/HubConnectionTest.java | 57 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java index c20e051eab46..076f64428def 100644 --- a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java +++ b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java @@ -264,7 +264,16 @@ public Completable start() { Transport transport = customTransport; if (transport == null) { Single tokenProvider = negotiateResponse.getAccessToken() != null ? Single.just(negotiateResponse.getAccessToken()) : accessTokenProvider; - switch (negotiateResponse.getChosenTransport()) { + TransportEnum chosenTransport; + if (this.skipNegotiate) { + if (this.transportEnum != TransportEnum.WEBSOCKETS) { + throw new RuntimeException("Negotiation can only be skipped when using the WebSocket transport directly."); + } + chosenTransport = this.transportEnum; + } else { + chosenTransport = negotiateResponse.getChosenTransport(); + } + switch (chosenTransport) { case LONG_POLLING: transport = new LongPollingTransport(localHeaders, httpClient, tokenProvider); break; diff --git a/src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/HubConnectionTest.java b/src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/HubConnectionTest.java index 6ebd67a2e124..999afb6a246c 100644 --- a/src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/HubConnectionTest.java +++ b/src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/HubConnectionTest.java @@ -2743,6 +2743,63 @@ public void noConnectionIdWhenSkippingNegotiate() { assertNull(hubConnection.getConnectionId()); } + @Test + public void SkippingNegotiateDoesNotNegotiate() { + try (TestLogger logger = new TestLogger(WebSocketTransport.class.getName())) { + AtomicBoolean negotiateCalled = new AtomicBoolean(false); + TestHttpClient client = new TestHttpClient().on("POST", "http://example.com/negotiate?negotiateVersion=1", + (req) -> { + negotiateCalled.set(true); + return Single.just(new HttpResponse(200, "", + TestUtils.stringToByteBuffer("{\"connectionId\":\"bVOiRPG8-6YiJ6d7ZcTOVQ\",\"" + + "availableTransports\":[{\"transport\":\"WebSockets\",\"transferFormats\":[\"Text\",\"Binary\"]}]}"))); + }); + + HubConnection hubConnection = HubConnectionBuilder + .create("http://example") + .withTransport(TransportEnum.WEBSOCKETS) + .shouldSkipNegotiate(true) + .withHttpClient(client) + .build(); + + try { + hubConnection.start().timeout(30, TimeUnit.SECONDS).blockingAwait(); + } catch (Exception e) { + assertEquals("WebSockets isn't supported in testing currently.", e.getMessage()); + } + assertEquals(HubConnectionState.DISCONNECTED, hubConnection.getConnectionState()); + assertFalse(negotiateCalled.get()); + + logger.assertLog("Starting Websocket connection."); + } + } + + @Test + public void ThrowsIfSkipNegotiationSetAndTransportIsNotWebSockets() { + AtomicBoolean negotiateCalled = new AtomicBoolean(false); + TestHttpClient client = new TestHttpClient().on("POST", "http://example.com/negotiate?negotiateVersion=1", + (req) -> { + negotiateCalled.set(true); + return Single.just(new HttpResponse(200, "", + TestUtils.stringToByteBuffer("{\"connectionId\":\"bVOiRPG8-6YiJ6d7ZcTOVQ\",\"" + + "availableTransports\":[{\"transport\":\"WebSockets\",\"transferFormats\":[\"Text\",\"Binary\"]}]}"))); + }); + + HubConnection hubConnection = HubConnectionBuilder + .create("http://example") + .shouldSkipNegotiate(true) + .withHttpClient(client) + .build(); + + try { + hubConnection.start().timeout(30, TimeUnit.SECONDS).blockingAwait(); + } catch (Exception e) { + assertEquals("Negotiation can only be skipped when using the WebSocket transport directly.", e.getMessage()); + } + assertEquals(HubConnectionState.DISCONNECTED, hubConnection.getConnectionState()); + assertFalse(negotiateCalled.get()); + } + @Test public void connectionIdIsAvailableAfterStart() { TestHttpClient client = new TestHttpClient().on("POST", "http://example.com/negotiate?negotiateVersion=1", From 27db9494ca0d1480ba915f674b3f0e6415b71631 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Thu, 25 Feb 2021 18:43:48 -0800 Subject: [PATCH 2/4] comment --- .../java/com/microsoft/signalr/HttpHubConnectionBuilder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HttpHubConnectionBuilder.java b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HttpHubConnectionBuilder.java index 24722eba1753..50d5bd2dc19c 100644 --- a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HttpHubConnectionBuilder.java +++ b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HttpHubConnectionBuilder.java @@ -69,7 +69,8 @@ public HttpHubConnectionBuilder withHubProtocol(HubProtocol protocol) { /** * Indicates to the {@link HubConnection} that it should skip the negotiate process. - * Note: This option only works with the Websockets transport and the Azure SignalR Service require the negotiate step. + * Note: This option only works with the {@link TransportEnum.WEBSOCKETS} transport selected via {@link HttpHubConnectionBuilder.withTransport}, + * additionally the Azure SignalR Service requires the negotiate step so this will fail when using the Azure SignalR Service. * * @param skipNegotiate Boolean indicating if the {@link HubConnection} should skip the negotiate step. * @return This instance of the HttpHubConnectionBuilder. From fb4b3cd085e6d3ba443f5bda7538d1fbe54d70b2 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Thu, 25 Feb 2021 19:19:09 -0800 Subject: [PATCH 3/4] fixup --- .../java/com/microsoft/signalr/HttpHubConnectionBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HttpHubConnectionBuilder.java b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HttpHubConnectionBuilder.java index 50d5bd2dc19c..09a4c04eddd1 100644 --- a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HttpHubConnectionBuilder.java +++ b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HttpHubConnectionBuilder.java @@ -69,7 +69,7 @@ public HttpHubConnectionBuilder withHubProtocol(HubProtocol protocol) { /** * Indicates to the {@link HubConnection} that it should skip the negotiate process. - * Note: This option only works with the {@link TransportEnum.WEBSOCKETS} transport selected via {@link HttpHubConnectionBuilder.withTransport}, + * Note: This option only works with the {@link TransportEnum#WEBSOCKETS} transport selected via {@link #withTransport(TransportEnum) withTransport}, * additionally the Azure SignalR Service requires the negotiate step so this will fail when using the Azure SignalR Service. * * @param skipNegotiate Boolean indicating if the {@link HubConnection} should skip the negotiate step. From 953f23353541614eabe5c271f5b6ce657790bbec Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Tue, 2 Mar 2021 10:33:10 -0800 Subject: [PATCH 4/4] minimize change --- .../signalr/HttpHubConnectionBuilder.java | 3 +-- .../com/microsoft/signalr/HubConnection.java | 3 --- .../microsoft/signalr/HubConnectionTest.java | 26 ------------------- 3 files changed, 1 insertion(+), 31 deletions(-) diff --git a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HttpHubConnectionBuilder.java b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HttpHubConnectionBuilder.java index 09a4c04eddd1..24722eba1753 100644 --- a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HttpHubConnectionBuilder.java +++ b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HttpHubConnectionBuilder.java @@ -69,8 +69,7 @@ public HttpHubConnectionBuilder withHubProtocol(HubProtocol protocol) { /** * Indicates to the {@link HubConnection} that it should skip the negotiate process. - * Note: This option only works with the {@link TransportEnum#WEBSOCKETS} transport selected via {@link #withTransport(TransportEnum) withTransport}, - * additionally the Azure SignalR Service requires the negotiate step so this will fail when using the Azure SignalR Service. + * Note: This option only works with the Websockets transport and the Azure SignalR Service require the negotiate step. * * @param skipNegotiate Boolean indicating if the {@link HubConnection} should skip the negotiate step. * @return This instance of the HttpHubConnectionBuilder. diff --git a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java index 076f64428def..6fe0e3b725bf 100644 --- a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java +++ b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java @@ -266,9 +266,6 @@ public Completable start() { Single tokenProvider = negotiateResponse.getAccessToken() != null ? Single.just(negotiateResponse.getAccessToken()) : accessTokenProvider; TransportEnum chosenTransport; if (this.skipNegotiate) { - if (this.transportEnum != TransportEnum.WEBSOCKETS) { - throw new RuntimeException("Negotiation can only be skipped when using the WebSocket transport directly."); - } chosenTransport = this.transportEnum; } else { chosenTransport = negotiateResponse.getChosenTransport(); diff --git a/src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/HubConnectionTest.java b/src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/HubConnectionTest.java index 999afb6a246c..3a1413e0ab11 100644 --- a/src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/HubConnectionTest.java +++ b/src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/HubConnectionTest.java @@ -2774,32 +2774,6 @@ public void SkippingNegotiateDoesNotNegotiate() { } } - @Test - public void ThrowsIfSkipNegotiationSetAndTransportIsNotWebSockets() { - AtomicBoolean negotiateCalled = new AtomicBoolean(false); - TestHttpClient client = new TestHttpClient().on("POST", "http://example.com/negotiate?negotiateVersion=1", - (req) -> { - negotiateCalled.set(true); - return Single.just(new HttpResponse(200, "", - TestUtils.stringToByteBuffer("{\"connectionId\":\"bVOiRPG8-6YiJ6d7ZcTOVQ\",\"" - + "availableTransports\":[{\"transport\":\"WebSockets\",\"transferFormats\":[\"Text\",\"Binary\"]}]}"))); - }); - - HubConnection hubConnection = HubConnectionBuilder - .create("http://example") - .shouldSkipNegotiate(true) - .withHttpClient(client) - .build(); - - try { - hubConnection.start().timeout(30, TimeUnit.SECONDS).blockingAwait(); - } catch (Exception e) { - assertEquals("Negotiation can only be skipped when using the WebSocket transport directly.", e.getMessage()); - } - assertEquals(HubConnectionState.DISCONNECTED, hubConnection.getConnectionState()); - assertFalse(negotiateCalled.get()); - } - @Test public void connectionIdIsAvailableAfterStart() { TestHttpClient client = new TestHttpClient().on("POST", "http://example.com/negotiate?negotiateVersion=1",