Make extension identifiers final. This improves security when untrusted code is present in the same class loader. In order to get around initialization ordering issues, I simply made the constructor for extension identifiers take no arguments and deferred initialization to an internalInit() method, which generated code will always call during init.

2 files changed, 69 insertions, 59 deletions

diff --git a/java/src/main/java/com/google/protobuf/GeneratedMessage.java b/java/src/main/java/com/google/protobuf/GeneratedMessage.java
index 47652e54..dba0ec83 100644
--- a/java/src/main/java/com/google/protobuf/GeneratedMessage.java
+++ b/java/src/main/java/com/google/protobuf/GeneratedMessage.java
@@ -724,25 +724,8 @@ public abstract class GeneratedMessage extends AbstractMessage {
   /** For use by generated code only. */
   public static <ContainingType extends Message, Type>
       GeneratedExtension<ContainingType, Type>
-      newGeneratedExtension(final FieldDescriptor descriptor,
-                            final Class<Type> type) {
-    if (descriptor.isRepeated()) {
-      throw new IllegalArgumentException(
-        "Must call newRepeatedGeneratedExtension() for repeated types.");
-    }
-    return new GeneratedExtension<ContainingType, Type>(descriptor, type);
-  }
-
-  /** For use by generated code only. */
-  public static <ContainingType extends Message, Type>
-      GeneratedExtension<ContainingType, List<Type>>
-      newRepeatedGeneratedExtension(
-        final FieldDescriptor descriptor, final Class<Type> type) {
-    if (!descriptor.isRepeated()) {
-      throw new IllegalArgumentException(
-        "Must call newGeneratedExtension() for non-repeated types.");
-    }
-    return new GeneratedExtension<ContainingType, List<Type>>(descriptor, type);
+      newGeneratedExtension() {
+    return new GeneratedExtension<ContainingType, Type>();
   }
 
   /**
@@ -775,8 +758,23 @@ public abstract class GeneratedMessage extends AbstractMessage {
     // TODO(kenton):  Find ways to avoid using Java reflection within this
     //   class.  Also try to avoid suppressing unchecked warnings.
 
-    private GeneratedExtension(final FieldDescriptor descriptor,
-                               final Class type) {
+    // We can't always initialize a GeneratedExtension when we first construct
+    // it due to initialization order difficulties (namely, the descriptor may
+    // not have been constructed yet, since it is often constructed by the
+    // initializer of a separate module).  So, we construct an uninitialized
+    // GeneratedExtension once, then call internalInit() on it later.  Generated
+    // code will always call internalInit() on all extensions as part of the
+    // static initialization code, and internalInit() throws an exception if
+    // called more than once, so this method is useless to users.
+    private GeneratedExtension() {}
+
+    /** For use by generated code only. */
+    public void internalInit(final FieldDescriptor descriptor,
+                             final Class type) {
+      if (this.descriptor != null) {
+        throw new IllegalStateException("Already initialized.");
+      }
+
       if (!descriptor.isExtension()) {
         throw new IllegalArgumentException(
           "GeneratedExtension given a regular (non-extension) field.");
@@ -811,11 +809,11 @@ public abstract class GeneratedMessage extends AbstractMessage {
       }
     }
 
-    private final FieldDescriptor descriptor;
-    private final Class type;
-    private final Method enumValueOf;
-    private final Method enumGetValueDescriptor;
-    private final Message messageDefaultInstance;
+    private FieldDescriptor descriptor;
+    private Class type;
+    private Method enumValueOf;
+    private Method enumGetValueDescriptor;
+    private Message messageDefaultInstance;
 
     public FieldDescriptor getDescriptor() { return descriptor; }
 
diff --git a/java/src/main/java/com/google/protobuf/GeneratedMessageLite.java b/java/src/main/java/com/google/protobuf/GeneratedMessageLite.java
index e327f745..9cdd4e94 100644
--- a/java/src/main/java/com/google/protobuf/GeneratedMessageLite.java
+++ b/java/src/main/java/com/google/protobuf/GeneratedMessageLite.java
@@ -420,34 +420,8 @@ public abstract class GeneratedMessageLite extends AbstractMessageLite {
   /** For use by generated code only. */
   public static <ContainingType extends MessageLite, Type>
       GeneratedExtension<ContainingType, Type>
-      newGeneratedExtension(
-        final ContainingType containingTypeDefaultInstance,
-        final Type defaultValue,
-        final MessageLite messageDefaultInstance,
-        final Internal.EnumLiteMap<?> enumTypeMap,
-        final int number,
-        final WireFormat.FieldType type) {
-    return new GeneratedExtension<ContainingType, Type>(
-      containingTypeDefaultInstance, defaultValue, messageDefaultInstance,
-      new ExtensionDescriptor(enumTypeMap, number, type,
-        false /* isRepeated */, false /* isPacked */));
-  }
-
-  /** For use by generated code only. */
-  public static <ContainingType extends MessageLite, Type>
-      GeneratedExtension<ContainingType, List<Type>>
-      newRepeatedGeneratedExtension(
-        final ContainingType containingTypeDefaultInstance,
-        final MessageLite messageDefaultInstance,
-        final Internal.EnumLiteMap<?> enumTypeMap,
-        final int number,
-        final WireFormat.FieldType type,
-        final boolean isPacked) {
-    return new GeneratedExtension<ContainingType, List<Type>>(
-      containingTypeDefaultInstance, Collections.<Type>emptyList(),
-      messageDefaultInstance,
-      new ExtensionDescriptor(
-        enumTypeMap, number, type, true /* isRepeated */, isPacked));
+      newGeneratedExtension() {
+    return new GeneratedExtension<ContainingType, Type>();
   }
 
   private static final class ExtensionDescriptor
@@ -515,7 +489,16 @@ public abstract class GeneratedMessageLite extends AbstractMessageLite {
    */
   public static final class GeneratedExtension<
       ContainingType extends MessageLite, Type> {
-    private GeneratedExtension(
+    // We can't always initialize a GeneratedExtension when we first construct
+    // it due to initialization order difficulties (namely, the default
+    // instances may not have been constructed yet).  So, we construct an
+    // uninitialized GeneratedExtension once, then call internalInit() on it
+    // later.  Generated code will always call internalInit() on all extensions
+    // as part of the static initialization code, and internalInit() throws an
+    // exception if called more than once, so this method is useless to users.
+    private GeneratedExtension() {}
+
+    private void internalInit(
         final ContainingType containingTypeDefaultInstance,
         final Type defaultValue,
         final MessageLite messageDefaultInstance,
@@ -526,10 +509,39 @@ public abstract class GeneratedMessageLite extends AbstractMessageLite {
       this.descriptor = descriptor;
     }
 
-    private final ContainingType containingTypeDefaultInstance;
-    private final Type defaultValue;
-    private final MessageLite messageDefaultInstance;
-    private final ExtensionDescriptor descriptor;
+    /** For use by generated code only. */
+    public void internalInitSingular(
+        final ContainingType containingTypeDefaultInstance,
+        final Type defaultValue,
+        final MessageLite messageDefaultInstance,
+        final Internal.EnumLiteMap<?> enumTypeMap,
+        final int number,
+        final WireFormat.FieldType type) {
+      internalInit(
+        containingTypeDefaultInstance, defaultValue, messageDefaultInstance,
+        new ExtensionDescriptor(enumTypeMap, number, type,
+          false /* isRepeated */, false /* isPacked */));
+    }
+
+    /** For use by generated code only. */
+    public void internalInitRepeated(
+        final ContainingType containingTypeDefaultInstance,
+        final MessageLite messageDefaultInstance,
+        final Internal.EnumLiteMap<?> enumTypeMap,
+        final int number,
+        final WireFormat.FieldType type,
+        final boolean isPacked) {
+      internalInit(
+        containingTypeDefaultInstance, (Type) Collections.emptyList(),
+        messageDefaultInstance,
+        new ExtensionDescriptor(
+          enumTypeMap, number, type, true /* isRepeated */, isPacked));
+    }
+
+    private ContainingType containingTypeDefaultInstance;
+    private Type defaultValue;
+    private MessageLite messageDefaultInstance;
+    private ExtensionDescriptor descriptor;
 
     /**
      * Default instance of the type being extended, used to identify that type.