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.

3 files changed, 95 insertions, 95 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.
diff --git a/src/google/protobuf/compiler/java/java_extension.cc b/src/google/protobuf/compiler/java/java_extension.cc
index 5932433f..903b0a9b 100644
--- a/src/google/protobuf/compiler/java/java_extension.cc
+++ b/src/google/protobuf/compiler/java/java_extension.cc
@@ -112,16 +112,20 @@ void ExtensionGenerator::Generate(io::Printer* printer) {
     "public static final int $constant_name$ = $number$;\n");
   if (descriptor_->is_repeated()) {
     printer->Print(vars,
-      "public static\n"
+      "public static final\n"
       "  com.google.protobuf.GeneratedMessage$lite$.GeneratedExtension<\n"
       "    $containing_type$,\n"
-      "    java.util.List<$type$>> $name$;\n");
+      "    java.util.List<$type$>> $name$ =\n"
+      "      com.google.protobuf.GeneratedMessage$lite$\n"
+      "        .newGeneratedExtension();\n");
   } else {
     printer->Print(vars,
-      "public static\n"
+      "public static final\n"
       "  com.google.protobuf.GeneratedMessage$lite$.GeneratedExtension<\n"
       "    $containing_type$,\n"
-      "    $type$> $name$;\n");
+      "    $type$> $name$ =\n"
+      "      com.google.protobuf.GeneratedMessage$lite$\n"
+      "        .newGeneratedExtension();\n");
   }
 }
 
@@ -157,43 +161,29 @@ void ExtensionGenerator::GenerateInitializationCode(io::Printer* printer) {
   }
 
   if (HasDescriptorMethods(descriptor_->file())) {
-    if (descriptor_->is_repeated()) {
-      printer->Print(vars,
-        "$scope$.$name$ =\n"
-        "  com.google.protobuf.GeneratedMessage\n"
-        "    .newRepeatedGeneratedExtension(\n"
-        "      $scope$.getDescriptor().getExtensions().get($index$),\n"
-        "      $type$.class);\n");
-    } else {
-      printer->Print(vars,
-        "$scope$.$name$ =\n"
-        "  com.google.protobuf.GeneratedMessage.newGeneratedExtension(\n"
-        "    $scope$.getDescriptor().getExtensions().get($index$),\n"
-        "    $type$.class);\n");
-    }
+    printer->Print(vars,
+      "$scope$.$name$.internalInit(\n"
+      "    $scope$.getDescriptor().getExtensions().get($index$),\n"
+      "    $type$.class);\n");
   } else {
     if (descriptor_->is_repeated()) {
       printer->Print(vars,
-        "$scope$.$name$ =\n"
-        "  com.google.protobuf.GeneratedMessageLite\n"
-        "    .newRepeatedGeneratedExtension(\n"
-        "      $extendee$.getDefaultInstance(),\n"
-        "      $prototype$,\n"
-        "      $enum_map$,\n"
-        "      $number$,\n"
-        "      com.google.protobuf.WireFormat.FieldType.$type_constant$,\n"
-        "      $packed$);\n");
+        "$scope$.$name$.internalInitRepeated(\n"
+        "    $extendee$.getDefaultInstance(),\n"
+        "    $prototype$,\n"
+        "    $enum_map$,\n"
+        "    $number$,\n"
+        "    com.google.protobuf.WireFormat.FieldType.$type_constant$,\n"
+        "    $packed$);\n");
     } else {
       printer->Print(vars,
-        "$scope$.$name$ =\n"
-        "  com.google.protobuf.GeneratedMessageLite\n"
-        "    .newGeneratedExtension(\n"
-        "      $extendee$.getDefaultInstance(),\n"
-        "      $default$,\n"
-        "      $prototype$,\n"
-        "      $enum_map$,\n"
-        "      $number$,\n"
-        "      com.google.protobuf.WireFormat.FieldType.$type_constant$);\n");
+        "$scope$.$name$.internalInitSingular(\n"
+        "    $extendee$.getDefaultInstance(),\n"
+        "    $default$,\n"
+        "    $prototype$,\n"
+        "    $enum_map$,\n"
+        "    $number$,\n"
+        "    com.google.protobuf.WireFormat.FieldType.$type_constant$);\n");
     }
   }
 }