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/17109/head
Sandy Zhang 9 months ago committed by Copybara-Service
parent 40ffd46cec
commit 415a147189
  1. 35
      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(); public abstract FileDescriptor getFile();
void resolveFeatures(FeatureSet unresolvedFeatures) throws DescriptorValidationException { void resolveFeatures(FeatureSet unresolvedFeatures) throws DescriptorValidationException {
// Unknown java features may be passed by users via public buildFrom but should not occur from if (this.parent != null
// generated code. && unresolvedFeatures.equals(FeatureSet.getDefaultInstance())
if (!unresolvedFeatures.getUnknownFields().isEmpty() && !hasInferredLegacyProtoFeatures()) {
&& unresolvedFeatures.getUnknownFields().hasField(JavaFeaturesProto.java_.getNumber())) { 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(); ExtensionRegistry registry = ExtensionRegistry.newInstance();
registry.add(JavaFeaturesProto.java_); registry.add(JavaFeaturesProto.java_);
ByteString bytes = unresolvedFeatures.toByteString(); ByteString bytes = unresolvedFeatures.toByteString();
@ -2800,13 +2820,6 @@ public final class Descriptors {
} }
} }
if (this.parent != null
&& unresolvedFeatures.equals(FeatureSet.getDefaultInstance())
&& !hasInferredLegacyProtoFeatures()) {
this.features = this.parent.features;
validateFeatures();
return;
}
FeatureSet.Builder features; FeatureSet.Builder features;
if (this.parent == null) { if (this.parent == null) {
Edition edition = getFile().getEdition(); Edition edition = getFile().getEdition();

@ -352,8 +352,9 @@ public class DescriptorsTest {
} }
@Test @Test
public void testProto2FieldDescriptorLegacyEnumFieldTreatedAsClosed() throws Exception { public void testFieldDescriptorLegacyEnumFieldTreatedAsOpen() throws Exception {
// Make an open enum definition. // Make an open enum definition and message that treats enum fields as open.
FileDescriptorProto openEnumFile = FileDescriptorProto openEnumFile =
FileDescriptorProto.newBuilder() FileDescriptorProto.newBuilder()
.setName("open_enum.proto") .setName("open_enum.proto")
@ -367,30 +368,9 @@ public class DescriptorsTest {
.setNumber(0) .setNumber(0)
.build()) .build())
.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( .addMessageType(
DescriptorProto.newBuilder() DescriptorProto.newBuilder()
.setName("TestClosedEnumField") .setName("TestOpenEnumField")
.addField( .addField(
FieldDescriptorProto.newBuilder() FieldDescriptorProto.newBuilder()
.setName("int_field") .setName("int_field")
@ -406,32 +386,21 @@ public class DescriptorsTest {
.setTypeName("TestEnumOpen") .setTypeName("TestEnumOpen")
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL) .setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
.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())
.build(); .build();
Descriptor closedMessage = FileDescriptor openEnumFileDescriptor =
Descriptors.FileDescriptor.buildFrom( Descriptors.FileDescriptor.buildFrom(openEnumFile, new FileDescriptor[0]);
closedEnumFile, new FileDescriptor[] {openFileDescriptor}) Descriptor openMessage = openEnumFileDescriptor.getMessageTypes().get(0);
.getMessageTypes() EnumDescriptor openEnum = openEnumFileDescriptor.findEnumTypeByName("TestEnumOpen");
.get(0); assertThat(openEnum.isClosed()).isFalse();
assertThat(closedMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed()) assertThat(openMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed())
.isFalse();
assertThat(openMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed())
.isFalse(); .isFalse();
assertThat(closedMessage.findFieldByName("closed_enum").legacyEnumFieldTreatedAsClosed())
.isTrue();
assertThat(closedMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed())
.isTrue();
} }
@Test @Test
public void testEditionFieldDescriptorLegacyEnumFieldTreatedAsClosed() throws Exception { public void testEditionFieldDescriptorLegacyEnumFieldTreatedAsClosedUnknown() throws Exception {
// Make an open enum definition. // Make an open enum definition.
FileDescriptorProto openEnumFile = FileDescriptorProto openEnumFile =
FileDescriptorProto.newBuilder() FileDescriptorProto.newBuilder()
@ -536,12 +505,19 @@ public class DescriptorsTest {
} }
@Test @Test
public void testFieldDescriptorLegacyEnumFieldTreatedAsOpen() throws Exception { public void testEditionFieldDescriptorLegacyEnumFieldTreatedAsClosedCustomPool()
// Make an open enum definition and message that treats enum fields as open. throws Exception {
FileDescriptor javaFeaturesDescriptor =
Descriptors.FileDescriptor.buildFrom(
JavaFeaturesProto.getDescriptor().toProto(),
new FileDescriptor[] {DescriptorProtos.getDescriptor()});
// Make an open enum definition.
FileDescriptorProto openEnumFile = FileDescriptorProto openEnumFile =
FileDescriptorProto.newBuilder() FileDescriptorProto.newBuilder()
.setName("open_enum.proto") .setName("open_enum.proto")
.setSyntax("proto3") .setSyntax("editions")
.setEdition(Edition.EDITION_2023)
.addEnumType( .addEnumType(
EnumDescriptorProto.newBuilder() EnumDescriptorProto.newBuilder()
.setName("TestEnumOpen") .setName("TestEnumOpen")
@ -551,9 +527,38 @@ public class DescriptorsTest {
.setNumber(0) .setNumber(0)
.build()) .build())
.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( .addMessageType(
DescriptorProto.newBuilder() DescriptorProto.newBuilder()
.setName("TestOpenEnumField") .setName("TestClosedEnumField")
.addField( .addField(
FieldDescriptorProto.newBuilder() FieldDescriptorProto.newBuilder()
.setName("int_field") .setName("int_field")
@ -568,18 +573,53 @@ public class DescriptorsTest {
.setType(FieldDescriptorProto.Type.TYPE_ENUM) .setType(FieldDescriptorProto.Type.TYPE_ENUM)
.setTypeName("TestEnumOpen") .setTypeName("TestEnumOpen")
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL) .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()) .build())
.build(); .build();
FileDescriptor openEnumFileDescriptor = // Reparse using custom java features descriptor.
Descriptors.FileDescriptor.buildFrom(openEnumFile, new FileDescriptor[0]); ExtensionRegistry registry = ExtensionRegistry.newInstance();
Descriptor openMessage = openEnumFileDescriptor.getMessageTypes().get(0); registry.add(
EnumDescriptor openEnum = openEnumFileDescriptor.findEnumTypeByName("TestEnumOpen"); javaFeaturesDescriptor.getExtensions().get(0),
assertThat(openEnum.isClosed()).isFalse(); DynamicMessage.getDefaultInstance(
assertThat(openMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed()) javaFeaturesDescriptor.getExtensions().get(0).getMessageType()));
.isFalse(); editionsClosedEnumFile =
assertThat(openMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed()) 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(); .isFalse();
assertThat(
editionsClosedMessage.findFieldByName("closed_enum").legacyEnumFieldTreatedAsClosed())
.isTrue();
assertThat(
editionsClosedMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed())
.isTrue();
} }
@Test @Test

Loading…
Cancel
Save