Repository: firebase/firebase-admin-java (java)

Issue:
Error Response with zero content throws NullPointerException
Hello, 

When a response comes back with an erred status code com.google.firebase.messaging.FirebaseMessaging#handleSendHttpError attempts to read the body, however if the content is null, the jsonFactory throws a NullPointerException, which is not converted into a FirebaseMessagingException. This ends up hiding the http response code.

In the tests, there is a test for testSendError, but there is no test that uses .setZeroContent() which will expose the problem.

To reproduce, send a mock response with an error status code and no content.

Pull request description:
Add handling of null response content
If an error response comes back with no content, the json factory throws a
NullPointerException when it attempts to read the response, which hides the
response code from the client.

fixes #166 

Patch:
diff --git a/src/main/java/com/google/firebase/messaging/FirebaseMessaging.java b/src/main/java/com/google/firebase/messaging/FirebaseMessaging.java
index 2daec84..e303ac8 100644
--- a/src/main/java/com/google/firebase/messaging/FirebaseMessaging.java
+++ b/src/main/java/com/google/firebase/messaging/FirebaseMessaging.java
@@ -253,13 +253,14 @@ public class FirebaseMessaging {
 
   private void handleSendHttpError(HttpResponseException e) throws FirebaseMessagingException {
     MessagingServiceErrorResponse response = new MessagingServiceErrorResponse();
-    try {
-      JsonParser parser = jsonFactory.createJsonParser(e.getContent());
-      parser.parseAndClose(response);
-    } catch (IOException ignored) {
-      // ignored
+    if (e.getContent() != null) {
+      try {
+        JsonParser parser = jsonFactory.createJsonParser(e.getContent());
+        parser.parseAndClose(response);
+      } catch (IOException ignored) {
+        // ignored
+      }
     }
-
     String code = FCM_ERROR_CODES.get(response.getErrorCode());
     if (code == null) {
       code = UNKNOWN_ERROR;


Test patch:
diff --git a/src/test/java/com/google/firebase/messaging/FirebaseMessagingTest.java b/src/test/java/com/google/firebase/messaging/FirebaseMessagingTest.java
index 2872d8a..d316680 100644
--- a/src/test/java/com/google/firebase/messaging/FirebaseMessagingTest.java
+++ b/src/test/java/com/google/firebase/messaging/FirebaseMessagingTest.java
@@ -172,6 +172,29 @@ public class FirebaseMessagingTest {
     }
   }
 
+  @Test
+  public void testSendErrorWithZeroContentResponse() throws Exception {
+    MockLowLevelHttpResponse response = new MockLowLevelHttpResponse();
+    FirebaseMessaging messaging = initMessaging(response);
+    for (int code : HTTP_ERRORS) {
+      response.setStatusCode(code).setZeroContent();
+      TestResponseInterceptor interceptor = new TestResponseInterceptor();
+      messaging.setInterceptor(interceptor);
+      try {
+        messaging.sendAsync(Message.builder().setTopic("test-topic").build()).get();
+        fail("No error thrown for HTTP error");
+      } catch (ExecutionException e) {
+        assertTrue(e.getCause() instanceof FirebaseMessagingException);
+        FirebaseMessagingException error = (FirebaseMessagingException) e.getCause();
+        assertEquals("unknown-error", error.getErrorCode());
+        assertEquals("Unexpected HTTP response with status: " + code + "; body: null",
+            error.getMessage());
+        assertTrue(error.getCause() instanceof HttpResponseException);
+      }
+      checkRequestHeader(interceptor);
+    }
+  }
+
   @Test
   public void testSendErrorWithDetails() throws Exception {
     MockLowLevelHttpResponse response = new MockLowLevelHttpResponse();