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.

15 years ago · 6e8b9e4a4a
parent 7f4938b244
commit 6e8b9e4a4a
3 changed files with 95 additions and 95 deletions
--- 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; }

--- 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.
--- 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");
    }
  }
 }