Java: make sure message.getExtension() returns immutable lists

PiperOrigin-RevId: 675169719
protocolbuffers · Sep 16, 2024 · dca7bf0 · dca7bf0
1 parent 372ddb3
commit dca7bf0
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 5 deletions.
diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java
@@ -988,7 +988,7 @@ public final <T> T getExtension(final ExtensionLite<? extends MessageT, T> exten
       final Object value = extensions.getField(descriptor);
       if (value == null) {
         if (descriptor.isRepeated()) {
-          return (T) Collections.emptyList();
+          return (T) ProtobufArrayList.emptyList();
         } else if (descriptor.getJavaType() == FieldDescriptor.JavaType.MESSAGE) {
           return (T) extension.getMessageDefaultInstance();
         } else {
@@ -1826,10 +1826,12 @@ protected Object fromReflectionType(final Object value) {
         if (descriptor.getJavaType() == FieldDescriptor.JavaType.MESSAGE
             || descriptor.getJavaType() == FieldDescriptor.JavaType.ENUM) {
           // Must convert the whole list.
-          final List<Object> result = new ArrayList<>();
+          final ProtobufArrayList<Object> result = new ProtobufArrayList<>();
+          result.ensureCapacity(((List<?>) value).size());
           for (final Object element : (List<?>) value) {
             result.add(singularFromReflectionType(element));
           }
+          result.makeImmutable();
           return result;
         } else {
           return value;

diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessageLite.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessageLite.java
@@ -23,7 +23,6 @@
 import java.lang.reflect.Method;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -1160,7 +1159,7 @@ GeneratedExtension<ContainingType, Type> newRepeatedGeneratedExtension(
           final boolean isPacked,
           final Class singularType) {
     @SuppressWarnings("unchecked") // Subclasses ensure Type is a List
-    Type emptyList = (Type) Collections.emptyList();
+    Type emptyList = (Type) ProtobufArrayList.emptyList();
     return new GeneratedExtension<ContainingType, Type>(
         containingTypeDefaultInstance,
         emptyList,
@@ -1331,10 +1330,12 @@ public MessageLite getMessageDefaultInstance() {
     Object fromFieldSetType(Object value) {
       if (descriptor.isRepeated()) {
         if (descriptor.getLiteJavaType() == WireFormat.JavaType.ENUM) {
-          List<Object> result = new ArrayList<>();
+          ProtobufArrayList<Object> result = new ProtobufArrayList<>();
+          result.ensureCapacity(((List<?>) value).size());
           for (Object element : (List) value) {
             result.add(singularFromFieldSetType(element));
           }
+          result.makeImmutable();
           return result;
         } else {
           return value;

diff --git a/java/core/src/test/java/com/google/protobuf/TestUtil.java b/java/core/src/test/java/com/google/protobuf/TestUtil.java
@@ -211,6 +211,7 @@
 import protobuf_unittest.UnittestProto.TestRequired;
 import protobuf_unittest.UnittestProto.TestUnpackedTypes;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.logging.Handler;
@@ -2168,6 +2169,55 @@ public static void assertRepeatedExtensionsModified(TestAllExtensionsOrBuilder m
     assertEqualsExactType("525", message.getExtension(repeatedCordExtension, 1));
   }
 
+  public static void assertRepeatedExtensionsImmutable(TestAllExtensionsOrBuilder message) {
+    List<List<?>> extensions =
+        Arrays.asList(
+            message.getExtension(repeatedInt32Extension),
+            message.getExtension(repeatedInt64Extension),
+            message.getExtension(repeatedUint32Extension),
+            message.getExtension(repeatedUint64Extension),
+            message.getExtension(repeatedSint32Extension),
+            message.getExtension(repeatedSint64Extension),
+            message.getExtension(repeatedFixed32Extension),
+            message.getExtension(repeatedFixed64Extension),
+            message.getExtension(repeatedSfixed32Extension),
+            message.getExtension(repeatedSfixed64Extension),
+            message.getExtension(repeatedFloatExtension),
+            message.getExtension(repeatedDoubleExtension),
+            message.getExtension(repeatedBoolExtension),
+            message.getExtension(repeatedStringExtension),
+            message.getExtension(repeatedBytesExtension),
+            message.getExtension(repeatedGroupExtension),
+            message.getExtension(repeatedNestedMessageExtension),
+            message.getExtension(repeatedForeignMessageExtension),
+            message.getExtension(repeatedImportMessageExtension),
+            message.getExtension(repeatedLazyMessageExtension),
+            message.getExtension(repeatedNestedEnumExtension),
+            message.getExtension(repeatedForeignEnumExtension),
+            message.getExtension(repeatedImportEnumExtension),
+            message.getExtension(repeatedStringPieceExtension),
+            message.getExtension(repeatedCordExtension));
+
+    ArrayList<String> errors = new ArrayList<>();
+    for (List<?> extension : extensions) {
+      String listContents = extension.toString();
+      try {
+        extension.clear();
+        errors.add(
+            "List should not be immutable, but was able to be cleared, so it's mutable: "
+                + listContents
+                + " "
+                + extension.getClass());
+      } catch (UnsupportedOperationException expected) {
+        // We expect this exception to be thrown, since the list should be
+        // immutable.
+      }
+    }
+    if (!errors.isEmpty()) {
+      throw new AssertionError(String.join("\n", errors));
+    }
+  }
+
   public static void setPackedExtensions(TestPackedExtensions.Builder message) {
     message.addExtension(packedInt32Extension, 601);
     message.addExtension(packedInt64Extension, 602L);