Fix NPE crash when body is null due to Http code 204 or 205 (#63)

* Fix NPE crash when body is null due to Http code 204 or 205 * Added more specification to the null body case to avoid any side effects * Codestyle * Avoid using reflections before it's actually needed. Added test case for code 205 also * Removed not needed empty map * Used cast instead * Extraced http codes to consts * Codestyle * Changed to correct http code --------- Co-authored-by: Messejana <jose.messejana@sky.uk>
slackhq · May 31, 2023 · ca5cc5b · ca5cc5b
1 parent 9ba4799
commit ca5cc5b
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 0 deletions.
diff --git a/src/main/java/com/slack/eithernet/ApiResult.kt b/src/main/java/com/slack/eithernet/ApiResult.kt
@@ -262,6 +262,12 @@ public object ApiResultCallAdapterFactory : CallAdapter.Factory() {
     private val decodeErrorBody: Boolean,
     private val annotations: Array<Annotation>,
   ) : CallAdapter<ApiResult<*, *>, Call<ApiResult<*, *>>> {
+
+    private companion object {
+      private const val HTTP_NO_CONTENT = 204
+      private const val HTTP_RESET_CONTENT = 205
+    }
+
     override fun adapt(call: Call<ApiResult<*, *>>): Call<ApiResult<*, *>> {
       return object : Call<ApiResult<*, *>> by call {
         @Suppress("LongMethod")
@@ -308,6 +314,18 @@ public object ApiResultCallAdapterFactory : CallAdapter.Factory() {
                   val withTag =
                     when (val result = response.body()) {
                       is Success -> result.withTags(result.tags + tags)
+                      null -> {
+                        val responseCode = response.code()
+                        if (
+                          (responseCode == HTTP_NO_CONTENT || responseCode == HTTP_RESET_CONTENT) &&
+                            apiResultType.actualTypeArguments[0] == Unit::class.java
+                        ) {
+                          @Suppress("UNCHECKED_CAST")
+                          ApiResult.success(Unit).withTags(tags as Map<KClass<*>, Any>)
+                        } else {
+                          null
+                        }
+                      }
                       else -> null
                     }
                   callback.onResponse(call, Response.success(withTag))

diff --git a/src/test/kotlin/com/slack/eithernet/ApiResultTest.kt b/src/test/kotlin/com/slack/eithernet/ApiResultTest.kt
@@ -86,6 +86,42 @@ class ApiResultTest {
     assertThat(result.value).isEqualTo(Unit)
   }
 
+  @Test
+  fun successWithUnitWithNullBody_code204() = runTest {
+    val response = MockResponse().setResponseCode(204)
+
+    server.enqueue(response)
+    val result = service.unitEndpoint()
+    check(result is Success)
+    assertThat(result.value).isEqualTo(Unit)
+  }
+
+  @Test(expected = NullPointerException::class)
+  fun successWithoutBody_code204() = runTest {
+    val response = MockResponse().setResponseCode(204)
+
+    server.enqueue(response)
+    service.testEndpoint()
+  }
+
+  @Test
+  fun successWithUnitWithNullBody_code205() = runTest {
+    val response = MockResponse().setResponseCode(205)
+
+    server.enqueue(response)
+    val result = service.unitEndpoint()
+    check(result is Success)
+    assertThat(result.value).isEqualTo(Unit)
+  }
+
+  @Test(expected = NullPointerException::class)
+  fun successWithoutBody_code205() = runTest {
+    val response = MockResponse().setResponseCode(205)
+
+    server.enqueue(response)
+    service.testEndpoint()
+  }
+
   @Test
   fun failureWithUnit() = runTest {
     val response = MockResponse().setResponseCode(404).setBody("Ignored errors!")