Reserialize all unresolved features using java features from the generated pool in case of descriptors from the custom pool.

This extends previous workaround for java features in unknown fields, to include features extensions that are known but use a mismatched descriptor.

This can happen when users bring their own descriptors via buildFrom.

PiperOrigin-RevId: 644013578
pull/17161/head
Sandy Zhang 8 months ago
parent e5ddc45645
commit 2426a02b90
  1. 37
      java/core/src/main/java/com/google/protobuf/Descriptors.java
  2. 152
      java/core/src/test/java/com/google/protobuf/DescriptorsTest.java

@ -2785,10 +2785,30 @@ public final class Descriptors {
public abstract FileDescriptor getFile();
void resolveFeatures(FeatureSet unresolvedFeatures) throws DescriptorValidationException {
// Unknown java features may be passed by users via public buildFrom but should not occur from
// generated code.
if (!unresolvedFeatures.getUnknownFields().isEmpty()
&& unresolvedFeatures.getUnknownFields().hasField(JavaFeaturesProto.java_.getNumber())) {
if (this.parent != null
&& unresolvedFeatures.equals(FeatureSet.getDefaultInstance())
&& !hasInferredLegacyProtoFeatures()) {
this.features = this.parent.features;
validateFeatures();
return;
}
// Java features from a custom pool (i.e. buildFrom) may end up in unknown fields or
// use a different descriptor from the generated pool used by the Java runtime.
boolean hasPossibleCustomJavaFeature = false;
for (FieldDescriptor f : unresolvedFeatures.getExtensionFields().keySet()) {
if (f.getNumber() == JavaFeaturesProto.java_.getNumber()
&& f != JavaFeaturesProto.java_.getDescriptor()) {
hasPossibleCustomJavaFeature = true;
continue;
}
}
boolean hasPossibleUnknownJavaFeature =
!unresolvedFeatures.getUnknownFields().isEmpty()
&& unresolvedFeatures
.getUnknownFields()
.hasField(JavaFeaturesProto.java_.getNumber());
if (hasPossibleCustomJavaFeature || hasPossibleUnknownJavaFeature) {
ExtensionRegistry registry = ExtensionRegistry.newInstance();
registry.add(JavaFeaturesProto.java_);
ByteString bytes = unresolvedFeatures.toByteString();
@ -2799,14 +2819,7 @@ public final class Descriptors {
this, "Failed to parse features with Java feature extension registry.", e);
}
}
if (this.parent != null
&& unresolvedFeatures.equals(FeatureSet.getDefaultInstance())
&& !hasInferredLegacyProtoFeatures()) {
this.features = this.parent.features;
validateFeatures();
return;
}
FeatureSet.Builder features;
if (this.parent == null) {
Edition edition = getFile().getEdition();

@ -352,8 +352,9 @@ public class DescriptorsTest {
}
@Test
public void testProto2FieldDescriptorLegacyEnumFieldTreatedAsClosed() throws Exception {
// Make an open enum definition.
public void testFieldDescriptorLegacyEnumFieldTreatedAsOpen() throws Exception {
// Make an open enum definition and message that treats enum fields as open.
FileDescriptorProto openEnumFile =
FileDescriptorProto.newBuilder()
.setName("open_enum.proto")
@ -367,30 +368,9 @@ public class DescriptorsTest {
.setNumber(0)
.build())
.build())
.build();
FileDescriptor openFileDescriptor =
Descriptors.FileDescriptor.buildFrom(openEnumFile, new FileDescriptor[0]);
EnumDescriptor openEnum = openFileDescriptor.getEnumTypes().get(0);
assertThat(openEnum.isClosed()).isFalse();
// Create a message that treats enum fields as closed.
FileDescriptorProto closedEnumFile =
FileDescriptorProto.newBuilder()
.setName("closed_enum_field.proto")
.addDependency("open_enum.proto")
.setSyntax("proto2")
.addEnumType(
EnumDescriptorProto.newBuilder()
.setName("TestEnum")
.addValue(
EnumValueDescriptorProto.newBuilder()
.setName("TestEnum_VALUE0")
.setNumber(0)
.build())
.build())
.addMessageType(
DescriptorProto.newBuilder()
.setName("TestClosedEnumField")
.setName("TestOpenEnumField")
.addField(
FieldDescriptorProto.newBuilder()
.setName("int_field")
@ -406,32 +386,21 @@ public class DescriptorsTest {
.setTypeName("TestEnumOpen")
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
.build())
.addField(
FieldDescriptorProto.newBuilder()
.setName("closed_enum")
.setNumber(3)
.setType(FieldDescriptorProto.Type.TYPE_ENUM)
.setTypeName("TestEnum")
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
.build())
.build())
.build();
Descriptor closedMessage =
Descriptors.FileDescriptor.buildFrom(
closedEnumFile, new FileDescriptor[] {openFileDescriptor})
.getMessageTypes()
.get(0);
assertThat(closedMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed())
FileDescriptor openEnumFileDescriptor =
Descriptors.FileDescriptor.buildFrom(openEnumFile, new FileDescriptor[0]);
Descriptor openMessage = openEnumFileDescriptor.getMessageTypes().get(0);
EnumDescriptor openEnum = openEnumFileDescriptor.findEnumTypeByName("TestEnumOpen");
assertThat(openEnum.isClosed()).isFalse();
assertThat(openMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed())
.isFalse();
assertThat(openMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed())
.isFalse();
assertThat(closedMessage.findFieldByName("closed_enum").legacyEnumFieldTreatedAsClosed())
.isTrue();
assertThat(closedMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed())
.isTrue();
}
@Test
public void testEditionFieldDescriptorLegacyEnumFieldTreatedAsClosed() throws Exception {
public void testEditionFieldDescriptorLegacyEnumFieldTreatedAsClosedUnknown() throws Exception {
// Make an open enum definition.
FileDescriptorProto openEnumFile =
FileDescriptorProto.newBuilder()
@ -536,12 +505,19 @@ public class DescriptorsTest {
}
@Test
public void testFieldDescriptorLegacyEnumFieldTreatedAsOpen() throws Exception {
// Make an open enum definition and message that treats enum fields as open.
public void testEditionFieldDescriptorLegacyEnumFieldTreatedAsClosedCustomPool()
throws Exception {
FileDescriptor javaFeaturesDescriptor =
Descriptors.FileDescriptor.buildFrom(
JavaFeaturesProto.getDescriptor().toProto(),
new FileDescriptor[] {DescriptorProtos.getDescriptor()});
// Make an open enum definition.
FileDescriptorProto openEnumFile =
FileDescriptorProto.newBuilder()
.setName("open_enum.proto")
.setSyntax("proto3")
.setSyntax("editions")
.setEdition(Edition.EDITION_2023)
.addEnumType(
EnumDescriptorProto.newBuilder()
.setName("TestEnumOpen")
@ -551,9 +527,38 @@ public class DescriptorsTest {
.setNumber(0)
.build())
.build())
.build();
FileDescriptor openFileDescriptor =
Descriptors.FileDescriptor.buildFrom(openEnumFile, new FileDescriptor[0]);
EnumDescriptor openEnum = openFileDescriptor.getEnumTypes().get(0);
assertThat(openEnum.isClosed()).isFalse();
// Create a message that treats enum fields as closed.
FileDescriptorProto editionsClosedEnumFile =
FileDescriptorProto.newBuilder()
.setName("editions_closed_enum_field.proto")
.addDependency("open_enum.proto")
.setSyntax("editions")
.setEdition(Edition.EDITION_2023)
.setOptions(
FileOptions.newBuilder()
.setFeatures(
DescriptorProtos.FeatureSet.newBuilder()
.setEnumType(DescriptorProtos.FeatureSet.EnumType.CLOSED)
.build())
.build())
.addEnumType(
EnumDescriptorProto.newBuilder()
.setName("TestEnum")
.addValue(
EnumValueDescriptorProto.newBuilder()
.setName("TestEnum_VALUE0")
.setNumber(0)
.build())
.build())
.addMessageType(
DescriptorProto.newBuilder()
.setName("TestOpenEnumField")
.setName("TestClosedEnumField")
.addField(
FieldDescriptorProto.newBuilder()
.setName("int_field")
@ -568,18 +573,53 @@ public class DescriptorsTest {
.setType(FieldDescriptorProto.Type.TYPE_ENUM)
.setTypeName("TestEnumOpen")
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
.setOptions(
DescriptorProtos.FieldOptions.newBuilder()
.setFeatures(
DescriptorProtos.FeatureSet.newBuilder()
.setExtension(
// Extension cannot be directly set using custom
// descriptor, so set using generated for now.
JavaFeaturesProto.java_,
JavaFeaturesProto.JavaFeatures.newBuilder()
.setLegacyClosedEnum(true)
.build())
.build())
.build())
.build())
.addField(
FieldDescriptorProto.newBuilder()
.setName("closed_enum")
.setNumber(3)
.setType(FieldDescriptorProto.Type.TYPE_ENUM)
.setTypeName("TestEnum")
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
.build())
.build())
.build();
FileDescriptor openEnumFileDescriptor =
Descriptors.FileDescriptor.buildFrom(openEnumFile, new FileDescriptor[0]);
Descriptor openMessage = openEnumFileDescriptor.getMessageTypes().get(0);
EnumDescriptor openEnum = openEnumFileDescriptor.findEnumTypeByName("TestEnumOpen");
assertThat(openEnum.isClosed()).isFalse();
assertThat(openMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed())
.isFalse();
assertThat(openMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed())
// Reparse using custom java features descriptor.
ExtensionRegistry registry = ExtensionRegistry.newInstance();
registry.add(
javaFeaturesDescriptor.getExtensions().get(0),
DynamicMessage.getDefaultInstance(
javaFeaturesDescriptor.getExtensions().get(0).getMessageType()));
editionsClosedEnumFile =
FileDescriptorProto.parseFrom(editionsClosedEnumFile.toByteString(), registry);
Descriptor editionsClosedMessage =
Descriptors.FileDescriptor.buildFrom(
editionsClosedEnumFile,
new FileDescriptor[] {openFileDescriptor, javaFeaturesDescriptor})
.getMessageTypes()
.get(0);
assertThat(
editionsClosedMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed())
.isFalse();
assertThat(
editionsClosedMessage.findFieldByName("closed_enum").legacyEnumFieldTreatedAsClosed())
.isTrue();
assertThat(
editionsClosedMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed())
.isTrue();
}
@Test

Loading…
Cancel
Save