From 3ff3904ecda87d0cab610b91fc07fa39ed1c6e91 Mon Sep 17 00:00:00 2001 From: Keith Date: Mon, 5 Oct 2026 19:11:27 -0400 Subject: [PATCH] Fix crashes on empty auth responses and repeated reconnects after sign-in Gson returns null for a null or empty authorizer response, and the channel and sign-in code called methods on the result, so a NullPointerException escaped on the event thread. Treat a null response as an authorization failure. InternalUser.disconnect() runs on every CONNECTING and DISCONNECTED state change. After the first run cleared the user ID, the server-to-user channel was still marked as subscribed, so the next run called getName() without a user ID and threw. Only unsubscribe while there is a user ID, and mark the channel unsubscribed. Fixes #372 Fixes #367 --- .../channel/impl/PrivateChannelImpl.java | 6 ++-- .../impl/PrivateEncryptedChannelImpl.java | 3 +- .../pusher/client/user/impl/InternalUser.java | 11 +++++-- .../channel/impl/PrivateChannelImplTest.java | 12 +++++++ .../impl/PrivateEncryptedChannelImplTest.java | 10 ++++++ .../client/user/impl/InternalUserTest.java | 33 +++++++++++++++++++ 6 files changed, 69 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/pusher/client/channel/impl/PrivateChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/PrivateChannelImpl.java index 93bfded7..9d51b5b3 100644 --- a/src/main/java/com/pusher/client/channel/impl/PrivateChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PrivateChannelImpl.java @@ -81,14 +81,14 @@ public void bind(final String eventName, final SubscriptionEventListener listene private String authorize() { try { final AuthResponse authResponse = GSON.fromJson(getAuthorizationResponse(), AuthResponse.class); - channelData = authResponse.getChannelData(); - - if (authResponse.getAuth() == null) { + // Gson returns null for a null or empty response + if (authResponse == null || authResponse.getAuth() == null) { throw new AuthorizationFailureException( "Didn't receive all the fields expected " + "from the ChannelAuthorizer, expected an auth and shared_secret." ); } else { + channelData = authResponse.getChannelData(); return authResponse.getAuth(); } } catch (JsonSyntaxException e) { diff --git a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java index a2c40da2..9bd53a62 100644 --- a/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java +++ b/src/main/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImpl.java @@ -77,7 +77,8 @@ public String toSubscribeMessage() { private String authenticate() { try { final AuthResponse authResponse = GSON.fromJson(getAuthorizationResponse(), AuthResponse.class); - if (authResponse.getAuth() == null || authResponse.getSharedSecret() == null) { + // Gson returns null for a null or empty response + if (authResponse == null || authResponse.getAuth() == null || authResponse.getSharedSecret() == null) { throw new AuthorizationFailureException( "Didn't receive all the fields expected " + "from the ChannelAuthorizer, expected an auth and shared_secret." diff --git a/src/main/java/com/pusher/client/user/impl/InternalUser.java b/src/main/java/com/pusher/client/user/impl/InternalUser.java index 045ae702..b1c023c7 100644 --- a/src/main/java/com/pusher/client/user/impl/InternalUser.java +++ b/src/main/java/com/pusher/client/user/impl/InternalUser.java @@ -4,6 +4,7 @@ import com.google.gson.JsonSyntaxException; import com.pusher.client.AuthenticationFailureException; import com.pusher.client.UserAuthenticator; +import com.pusher.client.channel.ChannelState; import com.pusher.client.channel.PusherEvent; import com.pusher.client.channel.SubscriptionEventListener; import com.pusher.client.channel.impl.ChannelManager; @@ -107,7 +108,10 @@ private AuthenticationResponse getAuthenticationResponse() throws Authentication String response = userAuthenticator.authenticate(connection.getSocketId()); try { AuthenticationResponse authenticationResponse = GSON.fromJson(response, AuthenticationResponse.class); - if (authenticationResponse.getAuth() == null || authenticationResponse.getUserData() == null) { + // Gson returns null for a null or empty response + if (authenticationResponse == null + || authenticationResponse.getAuth() == null + || authenticationResponse.getUserData() == null) { throw new AuthenticationFailureException( "Didn't receive all the fields expected from the UserAuthenticator. Expected auth and user_data" ); @@ -135,8 +139,11 @@ private void onSigninSuccess(PusherEvent event) { } private void disconnect() { - if (serverToUserChannel.isSubscribed()) { + // disconnect() runs on every CONNECTING and DISCONNECTED state change, so it + // can run again after userId has been cleared. getName() needs the userId. + if (userId != null && serverToUserChannel.isSubscribed()) { channelManager.unsubscribeFrom(serverToUserChannel.getName()); + serverToUserChannel.updateState(ChannelState.UNSUBSCRIBED); } userId = null; } diff --git a/src/test/java/com/pusher/client/channel/impl/PrivateChannelImplTest.java b/src/test/java/com/pusher/client/channel/impl/PrivateChannelImplTest.java index 11b48106..a5f16ba0 100644 --- a/src/test/java/com/pusher/client/channel/impl/PrivateChannelImplTest.java +++ b/src/test/java/com/pusher/client/channel/impl/PrivateChannelImplTest.java @@ -123,6 +123,18 @@ public void testThrowsAuthorizationFailureExceptionIfAuthorizerReturnsBasicStrin channel.toSubscribeMessage(); } + @Test(expected = AuthorizationFailureException.class) + public void testThrowsAuthorizationFailureExceptionIfAuthorizerReturnsNull() { + when(mockChannelAuthorizer.authorize(eq(getChannelName()), anyString())).thenReturn(null); + channel.toSubscribeMessage(); + } + + @Test(expected = AuthorizationFailureException.class) + public void testThrowsAuthorizationFailureExceptionIfAuthorizerReturnsEmptyString() { + when(mockChannelAuthorizer.authorize(eq(getChannelName()), anyString())).thenReturn(""); + channel.toSubscribeMessage(); + } + @Test(expected = AuthorizationFailureException.class) public void testThrowsAuthorizationFailureExceptionIfAuthorizerReturnsInvalidJSON() { when(mockChannelAuthorizer.authorize(eq(getChannelName()), anyString())).thenReturn("{\"auth\":\""); diff --git a/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java index ab038c93..723d3443 100644 --- a/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java +++ b/src/test/java/com/pusher/client/channel/impl/PrivateEncryptedChannelImplTest.java @@ -169,6 +169,16 @@ public void authenticationThrowsExceptionIfNoSharedSecret() { channel.toSubscribeMessage(); } + @Test(expected = AuthorizationFailureException.class) + public void authenticationThrowsExceptionIfAuthorizerReturnsNull() { + when(mockChannelAuthorizer.authorize(Matchers.anyString(), Matchers.anyString())) + .thenReturn(null); + + PrivateEncryptedChannelImpl channel = newInstance(); + + channel.toSubscribeMessage(); + } + @Test(expected = AuthorizationFailureException.class) public void authenticationThrowsExceptionIfMalformedJson() { when(mockChannelAuthorizer.authorize(Matchers.anyString(), Matchers.anyString())) diff --git a/src/test/java/com/pusher/client/user/impl/InternalUserTest.java b/src/test/java/com/pusher/client/user/impl/InternalUserTest.java index e648c7f6..3e7a1e1c 100644 --- a/src/test/java/com/pusher/client/user/impl/InternalUserTest.java +++ b/src/test/java/com/pusher/client/user/impl/InternalUserTest.java @@ -10,16 +10,20 @@ import com.pusher.client.AuthenticationFailureException; import com.pusher.client.UserAuthenticator; +import com.pusher.client.channel.ChannelState; import com.pusher.client.channel.PusherEvent; import com.pusher.client.channel.SubscriptionEventListener; import com.pusher.client.channel.impl.ChannelManager; +import com.pusher.client.connection.ConnectionEventListener; import com.pusher.client.connection.ConnectionState; +import com.pusher.client.connection.ConnectionStateChange; import com.pusher.client.connection.impl.InternalConnection; import com.pusher.client.util.Factory; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; @@ -83,6 +87,35 @@ public void testSigninMalformedResponse() { user.signin(); } + @Test(expected = AuthenticationFailureException.class) + public void testSigninNullResponse() { + when(mockConnection.getState()).thenReturn(ConnectionState.CONNECTED); + when(mockUserAuthenticator.authenticate(socketId)).thenReturn(null); + user.signin(); + } + + @Test + public void testRepeatedReconnectsAfterSigninDoNotThrow() { + final ArgumentCaptor connectionListener = + ArgumentCaptor.forClass(ConnectionEventListener.class); + verify(mockConnection).bind(eq(ConnectionState.ALL), connectionListener.capture()); + + user.handleEvent(PusherEvent.fromJson(signinSuccessEvent)); + final ArgumentCaptor serverToUserChannel = + ArgumentCaptor.forClass(ServerToUserChannel.class); + verify(mockChannelManager).subscribeTo(serverToUserChannel.capture(), eq(null)); + serverToUserChannel.getValue().updateState(ChannelState.SUBSCRIBED); + + // Each reconnect attempt moves the connection through CONNECTING again + connectionListener.getValue() + .onConnectionStateChange(new ConnectionStateChange(ConnectionState.RECONNECTING, ConnectionState.CONNECTING)); + connectionListener.getValue() + .onConnectionStateChange(new ConnectionStateChange(ConnectionState.RECONNECTING, ConnectionState.CONNECTING)); + + verify(mockChannelManager).unsubscribeFrom("#server-to-user-1"); + assertNull(user.userId()); + } + @Test public void testHandleEventSigninSuccessEvent() { user.handleEvent(PusherEvent.fromJson(signinSuccessEvent));