From 6e8b9e4a4a478c99a93925035b004a3829fd1e4f Mon Sep 17 00:00:00 2001 From: "kenton@google.com" Date: Tue, 22 Dec 2009 23:51:20 +0000 Subject: 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. --- .../java/com/google/protobuf/GeneratedMessage.java | 50 +++++++------- .../com/google/protobuf/GeneratedMessageLite.java | 78 +++++++++++++--------- 2 files changed, 69 insertions(+), 59 deletions(-) (limited to 'java/src') 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 GeneratedExtension - newGeneratedExtension(final FieldDescriptor descriptor, - final Class type) { - if (descriptor.isRepeated()) { - throw new IllegalArgumentException( - "Must call newRepeatedGeneratedExtension() for repeated types."); - } - return new GeneratedExtension(descriptor, type); - } - - /** For use by generated code only. */ - public static - GeneratedExtension> - newRepeatedGeneratedExtension( - final FieldDescriptor descriptor, final Class type) { - if (!descriptor.isRepeated()) { - throw new IllegalArgumentException( - "Must call newGeneratedExtension() for non-repeated types."); - } - return new GeneratedExtension>(descriptor, type); + newGeneratedExtension() { + return new GeneratedExtension(); } /** @@ -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 GeneratedExtension - newGeneratedExtension( - final ContainingType containingTypeDefaultInstance, - final Type defaultValue, - final MessageLite messageDefaultInstance, - final Internal.EnumLiteMap enumTypeMap, - final int number, - final WireFormat.FieldType type) { - return new GeneratedExtension( - containingTypeDefaultInstance, defaultValue, messageDefaultInstance, - new ExtensionDescriptor(enumTypeMap, number, type, - false /* isRepeated */, false /* isPacked */)); - } - - /** For use by generated code only. */ - public static - GeneratedExtension> - newRepeatedGeneratedExtension( - final ContainingType containingTypeDefaultInstance, - final MessageLite messageDefaultInstance, - final Internal.EnumLiteMap enumTypeMap, - final int number, - final WireFormat.FieldType type, - final boolean isPacked) { - return new GeneratedExtension>( - containingTypeDefaultInstance, Collections.emptyList(), - messageDefaultInstance, - new ExtensionDescriptor( - enumTypeMap, number, type, true /* isRepeated */, isPacked)); + newGeneratedExtension() { + return new GeneratedExtension(); } 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. -- cgit v1.2.3