Merge pull request #3157 from Huge/fix/rpn-ack-malformed-2xx

iab: treat a missing or non-JSON 2xx billing response as an error

Author: hussainmohd-a

app/src/play/java/com/celzero/bravedns/iab/RpnPurchaseAckServerResponse.kt

@@ -25,8 +25,9 @@ import org.json.JSONObject
  * Sealed result returned by every Rethink server billing endpoint.
  *
  * Use [RpnPurchaseAckServerResponse.from] to deserialise raw HTTP response bodies.
- * A 2xx response with `"success" != false` is treated as [Ok]; everything
- * else (non-2xx, `"error"` key present, or `"success" == false`) is [Err].
+ * A 2xx response with a JSON body and `"success" != false` is treated as [Ok];
+ * everything else (non-2xx, missing or non-JSON body, `"error"` key present, or
+ * `"success" == false`) is [Err].
  */
 sealed class RpnPurchaseAckServerResponse {
     data class Ok(val payload: ResponseOk) : RpnPurchaseAckServerResponse()
@@ -48,7 +49,14 @@ sealed class RpnPurchaseAckServerResponse {
                 return Err(payload)
             }
 
-            val json = body?.trimmedJson() ?: JSONObject()
+            // The server always replies with a JSON object (it carries at least "ray"),
+            // so a missing or non-JSON 2xx body (e.g. from a proxy) is not a success.
+            val json = body?.trimmedJson()
+            if (json == null) {
+                val payload = ResponseErr(error = "malformed response", httpCode = httpCode)
+                Logger.d(LOG_IAB, "PlayErr: $payload")
+                return Err(payload)
+            }
 
             // If "error" key is present, or "success" is explicitly false → error
             return if (json.has("error") || !json.optBoolean("success", true)) {
@@ -63,10 +71,10 @@ sealed class RpnPurchaseAckServerResponse {
             }
         }
 
-        private fun String.trimmedJson(): JSONObject = try {
+        private fun String.trimmedJson(): JSONObject? = try {
             JSONObject(this.trim())
         } catch (_: Exception) {
-            JSONObject()
+            null
         }
     }
 }

app/src/testPlay/java/com/celzero/bravedns/iab/RpnPurchaseAckServerResponseTest.kt

@@ -111,4 +111,20 @@ class RpnPurchaseAckServerResponseTest {
         assertTrue(result is RpnPurchaseAckServerResponse.Ok)
         assertFalse((result as RpnPurchaseAckServerResponse.Ok).payload.isCancelled)
     }
+
+    @Test
+    fun `2xx without a body is not ok`() {
+        val result = RpnPurchaseAckServerResponse.from(null, 200)
+
+        assertTrue(result is RpnPurchaseAckServerResponse.Err)
+        assertEquals(200, (result as RpnPurchaseAckServerResponse.Err).payload.httpCode)
+    }
+
+    @Test
+    fun `non-json 2xx body from a proxy is not ok`() {
+        val result = RpnPurchaseAckServerResponse.from("<html>Login required</html>", 200)
+
+        assertTrue(result is RpnPurchaseAckServerResponse.Err)
+        assertFalse((result as RpnPurchaseAckServerResponse.Err).payload.isPurchaseCancelled)
+    }
 }

app/src/testWebsite/java/com/celzero/bravedns/iab/RpnPurchaseAckServerResponseTest.kt

@@ -111,4 +111,20 @@ class RpnPurchaseAckServerResponseTest {
         assertTrue(result is RpnPurchaseAckServerResponse.Ok)
         assertFalse((result as RpnPurchaseAckServerResponse.Ok).payload.isCancelled)
     }
+
+    @Test
+    fun `2xx without a body is not ok`() {
+        val result = RpnPurchaseAckServerResponse.from(null, 200)
+
+        assertTrue(result is RpnPurchaseAckServerResponse.Err)
+        assertEquals(200, (result as RpnPurchaseAckServerResponse.Err).payload.httpCode)
+    }
+
+    @Test
+    fun `non-json 2xx body from a proxy is not ok`() {
+        val result = RpnPurchaseAckServerResponse.from("<html>Login required</html>", 200)
+
+        assertTrue(result is RpnPurchaseAckServerResponse.Err)
+        assertFalse((result as RpnPurchaseAckServerResponse.Err).payload.isPurchaseCancelled)
+    }
 }

app/src/website/java/com/celzero/bravedns/iab/RpnPurchaseAckServerResponse.kt

@@ -25,8 +25,9 @@ import org.json.JSONObject
  * Sealed result returned by every Rethink server billing endpoint.
  *
  * Use [RpnPurchaseAckServerResponse.from] to deserialise raw HTTP response bodies.
- * A 2xx response with `"success" != false` is treated as [Ok]; everything
- * else (non-2xx, `"error"` key present, or `"success" == false`) is [Err].
+ * A 2xx response with a JSON body and `"success" != false` is treated as [Ok];
+ * everything else (non-2xx, missing or non-JSON body, `"error"` key present, or
+ * `"success" == false`) is [Err].
  */
 sealed class RpnPurchaseAckServerResponse {
     data class Ok(val payload: ResponseOk) : RpnPurchaseAckServerResponse()
@@ -48,7 +49,14 @@ sealed class RpnPurchaseAckServerResponse {
                 return Err(payload)
             }
 
-            val json = body?.trimmedJson() ?: JSONObject()
+            // The server always replies with a JSON object (it carries at least "ray"),
+            // so a missing or non-JSON 2xx body (e.g. from a proxy) is not a success.
+            val json = body?.trimmedJson()
+            if (json == null) {
+                val payload = ResponseErr(error = "malformed response", httpCode = httpCode)
+                Logger.d(LOG_IAB, "PlayErr: $payload")
+                return Err(payload)
+            }
 
             // If "error" key is present, or "success" is explicitly false → error
             return if (json.has("error") || !json.optBoolean("success", true)) {
@@ -63,10 +71,10 @@ sealed class RpnPurchaseAckServerResponse {
             }
         }
 
-        private fun String.trimmedJson(): JSONObject = try {
+        private fun String.trimmedJson(): JSONObject? = try {
             JSONObject(this.trim())
         } catch (_: Exception) {
-            JSONObject()
+            null
         }
     }
 }