From 1622cace3e5640817668ae9ce384ff395af9b091 Mon Sep 17 00:00:00 2001
From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Date: Sun, 14 Jun 2026 01:33:38 -0700
Subject: [PATCH] Skip null list elements in moshi fromJson

---
 .../com/squareup/wire/MessageTypeAdapter.kt   |  2 +-
 .../com/squareup/wire/MessageJsonAdapter.kt   |  2 +-
 .../wire/internal/FieldOrOneOfBinding.kt      | 19 ++++++++++
 .../com/squareup/wire/WireJsonTest.kt         | 38 +++++++++++++++++++
 4 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/wire-gson-support/src/main/java/com/squareup/wire/MessageTypeAdapter.kt b/wire-gson-support/src/main/java/com/squareup/wire/MessageTypeAdapter.kt
index 5344b43b63..f45384faa3 100644
--- a/wire-gson-support/src/main/java/com/squareup/wire/MessageTypeAdapter.kt
+++ b/wire-gson-support/src/main/java/com/squareup/wire/MessageTypeAdapter.kt
@@ -71,7 +71,7 @@ internal class MessageTypeAdapter<M : Message<M, B>, B : Message.Builder<M, B>>(
       // interpreted as the appropriate default value when parsed into a protocol buffer."
       if (value == null) continue
 
-      jsonField.fieldBinding.set(builder, value)
+      jsonField.fieldBinding.set(builder, jsonField.fieldBinding.withoutStrayNullElements(value))
     }
     input.endObject()
     return builder.build()
diff --git a/wire-moshi-adapter/src/main/java/com/squareup/wire/MessageJsonAdapter.kt b/wire-moshi-adapter/src/main/java/com/squareup/wire/MessageJsonAdapter.kt
index 0d39a3410e..59ec5de2a3 100644
--- a/wire-moshi-adapter/src/main/java/com/squareup/wire/MessageJsonAdapter.kt
+++ b/wire-moshi-adapter/src/main/java/com/squareup/wire/MessageJsonAdapter.kt
@@ -79,7 +79,7 @@ internal class MessageJsonAdapter<M : Message<M, B>, B : Message.Builder<M, B>>(
       if (value == null) continue
 
       val fieldBinding = messageAdapter.fieldBindingsArray[index]
-      fieldBinding.set(builder, value)
+      fieldBinding.set(builder, fieldBinding.withoutStrayNullElements(value))
     }
     input.endObject()
     return builder.build()
diff --git a/wire-runtime/src/commonMain/kotlin/com/squareup/wire/internal/FieldOrOneOfBinding.kt b/wire-runtime/src/commonMain/kotlin/com/squareup/wire/internal/FieldOrOneOfBinding.kt
index 7a177ffe1a..504b52514c 100644
--- a/wire-runtime/src/commonMain/kotlin/com/squareup/wire/internal/FieldOrOneOfBinding.kt
+++ b/wire-runtime/src/commonMain/kotlin/com/squareup/wire/internal/FieldOrOneOfBinding.kt
@@ -89,4 +89,23 @@ abstract class FieldOrOneOfBinding<M, B> {
     if (isMap && syntax == Syntax.PROTO_3) return true
     return false
   }
+
+  /**
+   * Returns [value] with any null elements removed when it's the decoded JSON value for a repeated
+   * field. A literal `null` in a JSON array (e.g. `"items": [{...}, null]`) decodes to a null list
+   * element that isn't representable in a proto repeated field; left in place it would trip the
+   * `null !in result` check in [immutableCopyOf]. Struct fields (such as
+   * `repeated google.protobuf.NullValue`) are decoded by the struct adapter and may legitimately
+   * carry null entries, so they're returned unchanged.
+   */
+  fun withoutStrayNullElements(value: Any?): Any? {
+    if (!label.isRepeated || singleAdapter.isStructAdapter() || value !is List<*>) return value
+    if (null !in value) return value
+    return value.filterNotNull()
+  }
+
+  private fun ProtoAdapter<*>.isStructAdapter(): Boolean = this == ProtoAdapter.STRUCT_MAP ||
+    this == ProtoAdapter.STRUCT_LIST ||
+    this == ProtoAdapter.STRUCT_VALUE ||
+    this == ProtoAdapter.STRUCT_NULL
 }
diff --git a/wire-tests/wire-json-shared-kotlin-tests/com/squareup/wire/WireJsonTest.kt b/wire-tests/wire-json-shared-kotlin-tests/com/squareup/wire/WireJsonTest.kt
index 56ad435c15..553a67c480 100644
--- a/wire-tests/wire-json-shared-kotlin-tests/com/squareup/wire/WireJsonTest.kt
+++ b/wire-tests/wire-json-shared-kotlin-tests/com/squareup/wire/WireJsonTest.kt
@@ -342,6 +342,44 @@ class WireJsonTest {
     assertThat(parsed).isEqualTo(expected)
   }
 
+  /**
+   * A literal `null` element inside a repeated message field is not representable in proto, so it's
+   * dropped rather than passed to `Internal.immutableCopyOf` (which would throw `contains(null)`).
+   * https://github.com/square/wire/issues/3353
+   */
+  @Test fun nullElementInRepeatedMessageFieldIsDropped() {
+    val json = """{"pizzas":[{"toppings":["pineapple"]},null]}"""
+    val expected = PizzaDelivery.Builder()
+      .pizzas(listOf(Pizza.Builder().toppings(listOf("pineapple")).build()))
+      .build()
+    val parsed = jsonLibrary.fromJson(json, PizzaDelivery::class.java)
+    assertThat(parsed).isEqualTo(expected)
+  }
+
+  /** Same as above but for a repeated scalar field. */
+  @Test fun nullElementInRepeatedScalarFieldIsDropped() {
+    val json = """{"toppings":["pineapple",null,"onion"]}"""
+    val expected = Pizza.Builder().toppings(listOf("pineapple", "onion")).build()
+    val parsed = jsonLibrary.fromJson(json, Pizza::class.java)
+    assertThat(parsed).isEqualTo(expected)
+  }
+
+  /**
+   * Struct values legitimately represent null entries (e.g. inside a `google.protobuf.ListValue`),
+   * so a null element in a struct list must be preserved rather than stripped along with the stray
+   * nulls above.
+   */
+  @Test fun nullElementsInStructListArePreserved() {
+    if (jsonLibrary.writeIdentityValues) return
+
+    val json = """{"list":["a",null,3.0]}"""
+    val expected = AllStructs.Builder()
+      .list(listOf("a", null, 3.0))
+      .build()
+    val parsed = jsonLibrary.fromJson(json, AllStructs::class.java)
+    assertThat(parsed).isEqualTo(expected)
+  }
+
   @Test fun enumCanBeDecodedFromInt() {
     val json = """{"drink":9}"""
     val value = jsonLibrary.fromJson(json, FreeDrinkPromotion::class.java)