diff --git a/java/core/src/main/java/com/google/protobuf/DynamicMessage.java b/java/core/src/main/java/com/google/protobuf/DynamicMessage.java index f25aa23e7302f..f518284147ead 100644 --- a/java/core/src/main/java/com/google/protobuf/DynamicMessage.java +++ b/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. + * + *

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. + * + *

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 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(lastValueByKey.values())); + } + } + } + @Override public Builder clone() { Builder result = new Builder(type); diff --git a/java/core/src/test/java/com/google/protobuf/MapTest.java b/java/core/src/test/java/com/google/protobuf/MapTest.java index 270d246ee1f02..d65554b49f213 100644 --- a/java/core/src/test/java/com/google/protobuf/MapTest.java +++ b/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()); + } + }