diff --git a/java/core/src/main/java/com/google/protobuf/Descriptors.java b/java/core/src/main/java/com/google/protobuf/Descriptors.java index d42a93aac3c3..df36ce4a37e4 100644 --- a/java/core/src/main/java/com/google/protobuf/Descriptors.java +++ b/java/core/src/main/java/com/google/protobuf/Descriptors.java @@ -29,6 +29,7 @@ import com.google.protobuf.DescriptorProtos.OneofOptions; import com.google.protobuf.DescriptorProtos.ServiceDescriptorProto; import com.google.protobuf.DescriptorProtos.ServiceOptions; +import com.google.protobuf.Descriptors.DescriptorValidationException; import com.google.protobuf.JavaFeaturesProto.JavaFeatures; import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; @@ -618,14 +619,18 @@ private FileDescriptor( } public void resolveAllFeaturesImmutable() { - resolveAllFeaturesInternal(); + try { + resolveAllFeaturesInternal(); + } catch (DescriptorValidationException e) { + throw new IllegalArgumentException("Invalid features for \"" + proto.getName() + "\".", e); + } } /** * This method is to be called by generated code only. It resolves features for the descriptor * and all of its children. */ - private void resolveAllFeaturesInternal() { + private void resolveAllFeaturesInternal() throws DescriptorValidationException { if (this.features != null) { return; } @@ -712,22 +717,26 @@ private void crossLink() throws DescriptorValidationException { private synchronized void setProto(final FileDescriptorProto proto) { this.proto = proto; this.options = null; - this.features = resolveFeatures(proto.getOptions().getFeatures()); + try { + this.features = resolveFeatures(proto.getOptions().getFeatures()); - for (int i = 0; i < messageTypes.length; i++) { - messageTypes[i].setProto(proto.getMessageType(i)); - } + for (int i = 0; i < messageTypes.length; i++) { + messageTypes[i].setProto(proto.getMessageType(i)); + } - for (int i = 0; i < enumTypes.length; i++) { - enumTypes[i].setProto(proto.getEnumType(i)); - } + for (int i = 0; i < enumTypes.length; i++) { + enumTypes[i].setProto(proto.getEnumType(i)); + } - for (int i = 0; i < services.length; i++) { - services[i].setProto(proto.getService(i)); - } + for (int i = 0; i < services.length; i++) { + services[i].setProto(proto.getService(i)); + } - for (int i = 0; i < extensions.length; i++) { - extensions[i].setProto(proto.getExtension(i)); + for (int i = 0; i < extensions.length; i++) { + extensions[i].setProto(proto.getExtension(i)); + } + } catch (DescriptorValidationException e) { + throw new IllegalArgumentException("Invalid features for \"" + proto.getName() + "\".", e); } } } @@ -1102,7 +1111,7 @@ private Descriptor( } /** See {@link FileDescriptor#resolveAllFeatures}. */ - private void resolveAllFeatures() { + private void resolveAllFeatures() throws DescriptorValidationException { this.features = resolveFeatures(proto.getOptions().getFeatures()); for (Descriptor nestedType : nestedTypes) { @@ -1162,7 +1171,7 @@ private void validateNoDuplicateFieldNumbers() throws DescriptorValidationExcept } /** See {@link FileDescriptor#setProto}. */ - private void setProto(final DescriptorProto proto) { + private void setProto(final DescriptorProto proto) throws DescriptorValidationException { this.proto = proto; this.options = null; this.features = resolveFeatures(proto.getOptions().getFeatures()); @@ -1327,7 +1336,9 @@ public boolean isRequired() { /** Is this field declared optional? */ public boolean isOptional() { - return proto.getLabel() == FieldDescriptorProto.Label.LABEL_OPTIONAL; + return proto.getLabel() == FieldDescriptorProto.Label.LABEL_OPTIONAL + && this.features.getFieldPresence() + != DescriptorProtos.FeatureSet.FieldPresence.LEGACY_REQUIRED; } /** Is this field declared repeated? */ @@ -1732,7 +1743,7 @@ private FieldDescriptor( } /** See {@link FileDescriptor#resolveAllFeatures}. */ - private void resolveAllFeatures() { + private void resolveAllFeatures() throws DescriptorValidationException { this.features = resolveFeatures(proto.getOptions().getFeatures()); } @@ -1791,6 +1802,19 @@ boolean hasInferredLegacyProtoFeatures() { return false; } + @Override + void validateFeatures(FeatureSet features) throws DescriptorValidationException { + if (containingType != null + && containingType.toProto().getOptions().getMessageSetWireFormat()) { + if (isExtension()) { + if (!isOptional() || getType() != Type.MESSAGE) { + throw new DescriptorValidationException( + this, "Extensions of MessageSets must be optional messages."); + } + } + } + } + /** Look up and cross-link all field types, etc. */ private void crossLink() throws DescriptorValidationException { if (proto.hasExtendee()) { @@ -1962,23 +1986,10 @@ private void crossLink() throws DescriptorValidationException { } } } - - if (containingType != null - && containingType.toProto().getOptions().getMessageSetWireFormat()) { - if (isExtension()) { - if (!isOptional() || getType() != Type.MESSAGE) { - throw new DescriptorValidationException( - this, "Extensions of MessageSets must be optional messages."); - } - } else { - throw new DescriptorValidationException( - this, "MessageSets cannot have fields, only extensions."); - } - } } /** See {@link FileDescriptor#setProto}. */ - private void setProto(final FieldDescriptorProto proto) { + private void setProto(final FieldDescriptorProto proto) throws DescriptorValidationException { this.proto = proto; this.options = null; this.features = resolveFeatures(proto.getOptions().getFeatures()); @@ -2250,7 +2261,7 @@ private EnumDescriptor( } /** See {@link FileDescriptor#resolveAllFeatures}. */ - private void resolveAllFeatures() { + private void resolveAllFeatures() throws DescriptorValidationException { this.features = resolveFeatures(proto.getOptions().getFeatures()); for (EnumValueDescriptor value : values) { value.resolveAllFeatures(); @@ -2258,7 +2269,7 @@ private void resolveAllFeatures() { } /** See {@link FileDescriptor#setProto}. */ - private void setProto(final EnumDescriptorProto proto) { + private void setProto(final EnumDescriptorProto proto) throws DescriptorValidationException { this.proto = proto; this.options = null; this.features = resolveFeatures(proto.getOptions().getFeatures()); @@ -2402,12 +2413,13 @@ private EnumValueDescriptor(final EnumDescriptor parent, final Integer number) { } /** See {@link FileDescriptor#resolveAllFeatures}. */ - private void resolveAllFeatures() { + private void resolveAllFeatures() throws DescriptorValidationException { this.features = resolveFeatures(proto.getOptions().getFeatures()); } /** See {@link FileDescriptor#setProto}. */ - private void setProto(final EnumValueDescriptorProto proto) { + private void setProto(final EnumValueDescriptorProto proto) + throws DescriptorValidationException { this.proto = proto; this.options = null; this.features = resolveFeatures(proto.getOptions().getFeatures()); @@ -2517,7 +2529,7 @@ private ServiceDescriptor( } /** See {@link FileDescriptor#resolveAllFeatures}. */ - private void resolveAllFeatures() { + private void resolveAllFeatures() throws DescriptorValidationException { this.features = resolveFeatures(proto.getOptions().getFeatures()); for (MethodDescriptor method : methods) { @@ -2532,7 +2544,7 @@ private void crossLink() throws DescriptorValidationException { } /** See {@link FileDescriptor#setProto}. */ - private void setProto(final ServiceDescriptorProto proto) { + private void setProto(final ServiceDescriptorProto proto) throws DescriptorValidationException { this.proto = proto; this.options = null; this.features = resolveFeatures(proto.getOptions().getFeatures()); @@ -2655,7 +2667,7 @@ private MethodDescriptor( } /** See {@link FileDescriptor#resolveAllFeatures}. */ - private void resolveAllFeatures() { + private void resolveAllFeatures() throws DescriptorValidationException { this.features = resolveFeatures(proto.getOptions().getFeatures()); } @@ -2682,7 +2694,7 @@ private void crossLink() throws DescriptorValidationException { } /** See {@link FileDescriptor#setProto}. */ - private void setProto(final MethodDescriptorProto proto) { + private void setProto(final MethodDescriptorProto proto) throws DescriptorValidationException { this.proto = proto; this.options = null; this.features = resolveFeatures(proto.getOptions().getFeatures()); @@ -2723,7 +2735,7 @@ private GenericDescriptor() {} public abstract FileDescriptor getFile(); - FeatureSet resolveFeatures(FeatureSet unresolvedFeatures) { + FeatureSet resolveFeatures(FeatureSet unresolvedFeatures) throws DescriptorValidationException { if (this.parent != null && unresolvedFeatures.equals(FeatureSet.getDefaultInstance()) && !hasInferredLegacyProtoFeatures()) { @@ -2738,6 +2750,7 @@ FeatureSet resolveFeatures(FeatureSet unresolvedFeatures) { } features.mergeFrom(inferLegacyProtoFeatures()); features.mergeFrom(unresolvedFeatures); + validateFeatures(features.build()); return internFeatures(features.build()); } @@ -2749,6 +2762,9 @@ boolean hasInferredLegacyProtoFeatures() { return false; } + void validateFeatures(FeatureSet features) throws DescriptorValidationException { + } + GenericDescriptor parent; volatile FeatureSet features; } @@ -3214,11 +3230,11 @@ boolean isSynthetic() { } /** See {@link FileDescriptor#resolveAllFeatures}. */ - private void resolveAllFeatures() { + private void resolveAllFeatures() throws DescriptorValidationException { this.features = resolveFeatures(proto.getOptions().getFeatures()); } - private void setProto(final OneofDescriptorProto proto) { + private void setProto(final OneofDescriptorProto proto) throws DescriptorValidationException { this.proto = proto; this.options = null; this.features = resolveFeatures(proto.getOptions().getFeatures());