Apply last-key-wins semantics to duplicate map keys in DynamicMessage

java/core/src/main/java/com/google/protobuf/DynamicMessage.java

@@ -15,8 +15,10 @@
 import com.google.protobuf.Descriptors.OneofDescriptor;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -415,6 +417,8 @@ public DynamicMessage buildPartial() {
         }
       }
 
+      normalizeMapFields();
+
       DynamicMessage result =
           new DynamicMessage(
               type,
@@ -424,6 +428,56 @@ public DynamicMessage buildPartial() {
       return result;
     }
 
+
+    /**
+     * Collapses duplicate keys within map fields, keeping the last value seen.
+     *
+     * <p>The language guide requires that when a map is parsed from the wire and the same key
+     * appears more than once, the last value wins. Generated messages get this for free because
+     * they store map fields in a {@code MapField}, but {@code DynamicMessage} keeps them in a
+     * {@code FieldSet} as a plain list of {@code MapEntry} messages that is appended to
+     * unconditionally, so duplicate keys survived a parse. That made {@code DynamicMessage} and a
+     * generated message produce different results, and different serializations, for identical and
+     * well-formed wire input.
+     *
+     * <p>Normalizing once here rather than on every {@code addRepeatedField} keeps parsing linear;
+     * de-duplicating at insertion time would make parsing an n-entry map O(n^2).
+     */
+    private void normalizeMapFields() {
+      int numFields = type.getFieldCount();
+      for (int i = 0; i < numFields; i++) {
+        FieldDescriptor field = type.getField(i);
+        if (!field.isMapField()) {
+          continue;
+        }
+        // Map fields are repeated, so hasField() does not apply; getField() returns null when the
+        // field was never populated.
+        Object fieldValue = fields.getField(field);
+        if (!(fieldValue instanceof List)) {
+          continue;
+        }
+        List<?> entries = (List<?>) fieldValue;
+        if (entries.size() < 2) {
+          continue;
+        }
+        FieldDescriptor keyField = field.getMessageType().findFieldByNumber(1);
+        if (keyField == null) {
+          continue;
+        }
+        Map<Object, Object> lastValueByKey = new LinkedHashMap<>();
+        for (Object entry : entries) {
+          if (!(entry instanceof Message)) {
+            // A builder can be present while a caller is still mutating the field; leave it alone.
+            return;
+          }
+          lastValueByKey.put(((Message) entry).getField(keyField), entry);
+        }
+        if (lastValueByKey.size() != entries.size()) {
+          fields.setField(field, new ArrayList<Object>(lastValueByKey.values()));
+        }
+      }
+    }
+
     @Override
     public Builder clone() {
       Builder result = new Builder(type);

java/core/src/test/java/com/google/protobuf/MapTest.java

@@ -1626,4 +1626,50 @@ public void getAllFields_mapEntryListMutability() {
     builder.clearField(int2MessageMapField);
     assertThat(mapEntries).hasSize(1);
   }
+
+  @Test
+  public void testDuplicateMapKey_dynamicMessageMatchesGeneratedMessage() throws Exception {
+    // Hand-built wire bytes: string_to_int32_field (field 6) carrying the key "k" twice, with
+    // value 1 and then value 2. These cannot be produced through a builder, because a builder
+    // already applies map semantics; only the wire format can express a repeated key.
+    //   32 05 0a 01 6b 10 01   -> {"k": 1}
+    //   32 05 0a 01 6b 10 02   -> {"k": 2}
+    ByteString wire =
+        ByteString.copyFrom(
+            new byte[] {
+              0x32, 0x05, 0x0a, 0x01, 0x6b, 0x10, 0x01,
+              0x32, 0x05, 0x0a, 0x01, 0x6b, 0x10, 0x02
+            });
+
+    TestMap generated = TestMap.parseFrom(wire);
+    DynamicMessage dynamic = DynamicMessage.parseFrom(TestMap.getDescriptor(), wire);
+    FieldDescriptor mapField = TestMap.getDescriptor().findFieldByName("string_to_int32_field");
+
+    // The language guide requires the last value for a repeated key to win.
+    assertThat(generated.getStringToInt32FieldMap()).containsExactly("k", 2);
+    assertThat(dynamic.getRepeatedFieldCount(mapField)).isEqualTo(1);
+
+    // A DynamicMessage and a generated message must not disagree about identical wire input.
+    assertThat(dynamic.toByteString()).isEqualTo(generated.toByteString());
+  }
+
+  @Test
+  public void testDuplicateMapKey_dynamicMessagePreservesDistinctKeys() throws Exception {
+    // Same shape, but the two entries use different keys, so both must survive.
+    ByteString wire =
+        ByteString.copyFrom(
+            new byte[] {
+              0x32, 0x05, 0x0a, 0x01, 0x61, 0x10, 0x01,
+              0x32, 0x05, 0x0a, 0x01, 0x62, 0x10, 0x02
+            });
+
+    TestMap generated = TestMap.parseFrom(wire);
+    DynamicMessage dynamic = DynamicMessage.parseFrom(TestMap.getDescriptor(), wire);
+    FieldDescriptor mapField = TestMap.getDescriptor().findFieldByName("string_to_int32_field");
+
+    assertThat(generated.getStringToInt32FieldMap()).containsExactly("a", 1, "b", 2);
+    assertThat(dynamic.getRepeatedFieldCount(mapField)).isEqualTo(2);
+    assertThat(dynamic.toByteString()).isEqualTo(generated.toByteString());
+  }
+
 }