diff --git a/firebase-messaging/CHANGELOG.md b/firebase-messaging/CHANGELOG.md index 3d44366e2d0..65a1ed2ba25 100644 --- a/firebase-messaging/CHANGELOG.md +++ b/firebase-messaging/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +- [fixed] Topic subscribe and unsubscribe failed with HTTP 400 when the topic name contained + percent characters. - [changed] Internal adjustments # 25.1.2 diff --git a/firebase-messaging/src/main/java/com/google/firebase/messaging/TopicSubscriptionClient.java b/firebase-messaging/src/main/java/com/google/firebase/messaging/TopicSubscriptionClient.java index e2c989c59ab..1a7c49dbc62 100644 --- a/firebase-messaging/src/main/java/com/google/firebase/messaging/TopicSubscriptionClient.java +++ b/firebase-messaging/src/main/java/com/google/firebase/messaging/TopicSubscriptionClient.java @@ -15,6 +15,7 @@ import static com.google.firebase.messaging.Constants.TAG; +import android.net.Uri; import android.os.Build; import android.util.Log; import androidx.annotation.NonNull; @@ -114,7 +115,7 @@ private void performTopicOperation(String topic, String token, String fid, Strin + "/registrations/" + fid + "/topicSubscriptions/" - + topic + + Uri.encode(topic) + ":" + operation); diff --git a/firebase-messaging/src/test/java/com/google/firebase/messaging/TopicSubscriptionClientRoboTest.java b/firebase-messaging/src/test/java/com/google/firebase/messaging/TopicSubscriptionClientRoboTest.java index 29468e850df..9e956bdcb40 100644 --- a/firebase-messaging/src/test/java/com/google/firebase/messaging/TopicSubscriptionClientRoboTest.java +++ b/firebase-messaging/src/test/java/com/google/firebase/messaging/TopicSubscriptionClientRoboTest.java @@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -39,6 +40,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -187,6 +189,75 @@ public void testSubscribe_failure404_closesErrorStreamAndDisconnects() throws Ex verify(mockConnection).disconnect(); } + @Test + public void testSubscribe_percentCharactersInTopic_arePathEncodedOnce() throws Exception { + when(mockConnection.getResponseCode()).thenReturn(200); + String topic = "email.example.name%2Bafterplus%40example.com"; + + runOnBackground(() -> client.subscribe(topic)); + + assertThat(capturedRequestUrl().getPath()) + .isEqualTo( + topicSubscriptionPath("email.example.name%252Bafterplus%2540example.com", "subscribe")); + } + + @Test + public void testSubscribe_documentedTopicCharset_encodesOnlyPercent() throws Exception { + when(mockConnection.getResponseCode()).thenReturn(200); + String topic = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_.~%"; + + runOnBackground(() -> client.subscribe(topic)); + + String path = capturedRequestUrl().getPath(); + assertThat(path) + .isEqualTo( + topicSubscriptionPath( + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_.~%25", + "subscribe")); + assertThat(path).contains("-_.~%25"); + assertThat(path).doesNotContain("%2D"); + assertThat(path).doesNotContain("%5F"); + assertThat(path).doesNotContain("%2E"); + assertThat(path).doesNotContain("%7E"); + } + + @Test + public void testUnsubscribe_usesSameTopicEncodingAsSubscribe() throws Exception { + when(mockConnection.getResponseCode()).thenReturn(200); + String topic = "email.example.name%2Bafterplus%40example.com"; + String encodedTopic = "email.example.name%252Bafterplus%2540example.com"; + + runOnBackground(() -> client.subscribe(topic)); + String subscribePath = capturedRequestUrl().getPath(); + clearInvocations(client); + runOnBackground(() -> client.unsubscribe(topic)); + String unsubscribePath = capturedRequestUrl().getPath(); + + assertThat(subscribePath).isEqualTo(topicSubscriptionPath(encodedTopic, "subscribe")); + assertThat(unsubscribePath).isEqualTo(topicSubscriptionPath(encodedTopic, "unsubscribe")); + assertThat(subscribePath).endsWith(":subscribe"); + assertThat(unsubscribePath).endsWith(":unsubscribe"); + assertThat(subscribePath).doesNotContain("%3Asubscribe"); + assertThat(unsubscribePath).doesNotContain("%3Aunsubscribe"); + } + + private URL capturedRequestUrl() throws IOException { + ArgumentCaptor urlCaptor = ArgumentCaptor.forClass(URL.class); + verify(client).createConnection(urlCaptor.capture()); + return urlCaptor.getValue(); + } + + private static String topicSubscriptionPath(String encodedTopic, String operation) { + return "/v1/projects/" + + TEST_PROJECT_ID + + "/registrations/" + + TEST_FID + + "/topicSubscriptions/" + + encodedTopic + + ":" + + operation; + } + private static class CloseTrackingInputStream extends ByteArrayInputStream { private boolean isClosed = false;