diff --git a/src/main/java/build/buf/protovalidate/exceptions/CompilationException.java b/src/main/java/build/buf/protovalidate/exceptions/CompilationException.java index 72e59ea5..8e2c2806 100644 --- a/src/main/java/build/buf/protovalidate/exceptions/CompilationException.java +++ b/src/main/java/build/buf/protovalidate/exceptions/CompilationException.java @@ -14,12 +14,13 @@ package build.buf.protovalidate.exceptions; -/** - * {@link CompilationException} extends {@link ValidationException} is returned when a constraint - * fails to compile. This is a fatal error. - */ +/** CompilationException is returned when a constraint fails to compile. This is a fatal error. */ public class CompilationException extends ValidationException { public CompilationException(String message) { super(message); } + + public CompilationException(String message, Throwable cause) { + super(message, cause); + } } diff --git a/src/main/java/build/buf/protovalidate/exceptions/ExecutionException.java b/src/main/java/build/buf/protovalidate/exceptions/ExecutionException.java index 7c56fddb..1d455d90 100644 --- a/src/main/java/build/buf/protovalidate/exceptions/ExecutionException.java +++ b/src/main/java/build/buf/protovalidate/exceptions/ExecutionException.java @@ -14,10 +14,7 @@ package build.buf.protovalidate.exceptions; -/** - * {@link ExecutionException} extends {@link ValidationException} is returned when a constraint - * fails to execute. This is a fatal error. - */ +/** ExecutionException is returned when a constraint fails to execute. This is a fatal error. */ public class ExecutionException extends ValidationException { public ExecutionException(String message) { super(message); diff --git a/src/main/java/build/buf/protovalidate/exceptions/ValidationException.java b/src/main/java/build/buf/protovalidate/exceptions/ValidationException.java index e5069e75..f4c5dc92 100644 --- a/src/main/java/build/buf/protovalidate/exceptions/ValidationException.java +++ b/src/main/java/build/buf/protovalidate/exceptions/ValidationException.java @@ -14,9 +14,13 @@ package build.buf.protovalidate.exceptions; -/** Extends {@link Exception} is the base exception for all validation errors. */ +/** ValidationException is the base exception for all validation errors. */ public class ValidationException extends Exception { public ValidationException(String message) { super(message); } + + public ValidationException(String message, Throwable cause) { + super(message, cause); + } } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintResolver.java b/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintResolver.java index ac26beba..5cff37e9 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintResolver.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/ConstraintResolver.java @@ -23,6 +23,7 @@ import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.Descriptors.OneofDescriptor; +import com.google.protobuf.ExtensionRegistry; import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.MessageLite; @@ -35,9 +36,13 @@ class ConstraintResolver { * @param desc the message descriptor. * @return the resolved {@link MessageConstraints}. */ - MessageConstraints resolveMessageConstraints(Descriptor desc) + MessageConstraints resolveMessageConstraints(Descriptor desc, ExtensionRegistry registry) throws InvalidProtocolBufferException, CompilationException { DescriptorProtos.MessageOptions options = desc.getOptions(); + // If the protovalidate message extension is unknown, reparse using extension registry. + if (options.getUnknownFields().hasField(ValidateProto.message.getNumber())) { + options = DescriptorProtos.MessageOptions.parseFrom(options.toByteString(), registry); + } if (!options.hasExtension(ValidateProto.message)) { return MessageConstraints.getDefaultInstance(); } @@ -61,9 +66,13 @@ MessageConstraints resolveMessageConstraints(Descriptor desc) * @param desc the oneof descriptor. * @return the resolved {@link OneofConstraints}. */ - OneofConstraints resolveOneofConstraints(OneofDescriptor desc) + OneofConstraints resolveOneofConstraints(OneofDescriptor desc, ExtensionRegistry registry) throws InvalidProtocolBufferException, CompilationException { DescriptorProtos.OneofOptions options = desc.getOptions(); + // If the protovalidate oneof extension is unknown, reparse using extension registry. + if (options.getUnknownFields().hasField(ValidateProto.oneof.getNumber())) { + options = DescriptorProtos.OneofOptions.parseFrom(options.toByteString(), registry); + } if (!options.hasExtension(ValidateProto.oneof)) { return OneofConstraints.getDefaultInstance(); } @@ -87,9 +96,13 @@ OneofConstraints resolveOneofConstraints(OneofDescriptor desc) * @param desc the field descriptor. * @return the resolved {@link FieldConstraints}. */ - FieldConstraints resolveFieldConstraints(FieldDescriptor desc) + FieldConstraints resolveFieldConstraints(FieldDescriptor desc, ExtensionRegistry registry) throws InvalidProtocolBufferException, CompilationException { DescriptorProtos.FieldOptions options = desc.getOptions(); + // If the protovalidate field option is unknown, reparse using extension registry. + if (options.getUnknownFields().hasField(ValidateProto.field.getNumber())) { + options = DescriptorProtos.FieldOptions.parseFrom(options.toByteString(), registry); + } if (!options.hasExtension(ValidateProto.field)) { return FieldConstraints.getDefaultInstance(); } diff --git a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java index 119d878c..21f55325 100644 --- a/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java +++ b/src/main/java/build/buf/protovalidate/internal/evaluator/EvaluatorBuilder.java @@ -45,12 +45,12 @@ /** A build-through cache of message evaluators keyed off the provided descriptor. */ public class EvaluatorBuilder { - private static final ExtensionRegistry extensionRegistry = ExtensionRegistry.newInstance(); + private static final ExtensionRegistry EXTENSION_REGISTRY = ExtensionRegistry.newInstance(); static { - extensionRegistry.add(ValidateProto.message); - extensionRegistry.add(ValidateProto.field); - extensionRegistry.add(ValidateProto.oneof); + EXTENSION_REGISTRY.add(ValidateProto.message); + EXTENSION_REGISTRY.add(ValidateProto.field); + EXTENSION_REGISTRY.add(ValidateProto.oneof); } private final Map evaluatorMap = new HashMap<>(); @@ -102,9 +102,10 @@ private Evaluator build(Descriptor desc) throws CompilationException { private void buildMessage(Descriptor desc, MessageEvaluator msgEval) throws CompilationException { try { DynamicMessage defaultInstance = - DynamicMessage.parseFrom(desc, new byte[0], extensionRegistry); + DynamicMessage.parseFrom(desc, new byte[0], EXTENSION_REGISTRY); Descriptor descriptor = defaultInstance.getDescriptorForType(); - MessageConstraints msgConstraints = resolver.resolveMessageConstraints(descriptor); + MessageConstraints msgConstraints = + resolver.resolveMessageConstraints(descriptor, EXTENSION_REGISTRY); if (msgConstraints.getDisabled()) { return; } @@ -112,7 +113,7 @@ private void buildMessage(Descriptor desc, MessageEvaluator msgEval) throws Comp processOneofConstraints(descriptor, msgEval); processFields(descriptor, msgEval); } catch (InvalidProtocolBufferException e) { - throw new CompilationException("failed to parse proto definition: " + desc.getFullName()); + throw new CompilationException("failed to parse proto definition: " + desc.getFullName(), e); } } @@ -142,7 +143,8 @@ private void processOneofConstraints(Descriptor desc, MessageEvaluator msgEval) throws InvalidProtocolBufferException, CompilationException { List oneofs = desc.getOneofs(); for (Descriptors.OneofDescriptor oneofDesc : oneofs) { - OneofConstraints oneofConstraints = resolver.resolveOneofConstraints(oneofDesc); + OneofConstraints oneofConstraints = + resolver.resolveOneofConstraints(oneofDesc, EXTENSION_REGISTRY); OneofEvaluator oneofEvaluatorEval = new OneofEvaluator(oneofDesc, oneofConstraints.getRequired()); msgEval.append(oneofEvaluatorEval); @@ -154,7 +156,8 @@ private void processFields(Descriptor desc, MessageEvaluator msgEval) List fields = desc.getFields(); for (FieldDescriptor fieldDescriptor : fields) { FieldDescriptor descriptor = desc.findFieldByName(fieldDescriptor.getName()); - FieldConstraints fieldConstraints = resolver.resolveFieldConstraints(descriptor); + FieldConstraints fieldConstraints = + resolver.resolveFieldConstraints(descriptor, EXTENSION_REGISTRY); FieldEvaluator fldEval = buildField(descriptor, fieldConstraints); msgEval.append(fldEval); } @@ -204,7 +207,7 @@ private void processFieldExpressions( try { DynamicMessage defaultInstance = DynamicMessage.parseFrom( - fieldDescriptor.getMessageType(), new byte[0], extensionRegistry); + fieldDescriptor.getMessageType(), new byte[0], EXTENSION_REGISTRY); opts = Arrays.asList( EnvOption.types(defaultInstance), @@ -213,7 +216,7 @@ private void processFieldExpressions( Variable.THIS_NAME, Decls.newObjectType(fieldDescriptor.getMessageType().getFullName())))); } catch (InvalidProtocolBufferException e) { - throw new CompilationException("field descriptor type is invalid " + e.getMessage()); + throw new CompilationException("field descriptor type is invalid " + e.getMessage(), e); } } else { opts = diff --git a/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java b/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java new file mode 100644 index 00000000..be4131c5 --- /dev/null +++ b/src/test/java/build/buf/protovalidate/ValidatorDynamicMessageTest.java @@ -0,0 +1,138 @@ +// Copyright 2023 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package build.buf.protovalidate; + +import static org.assertj.core.api.Assertions.assertThat; + +import build.buf.validate.Violation; +import com.example.noimports.validationtest.ExampleFieldConstraints; +import com.example.noimports.validationtest.ExampleMessageConstraints; +import com.example.noimports.validationtest.ExampleOneofConstraints; +import com.google.protobuf.DescriptorProtos; +import com.google.protobuf.Descriptors; +import com.google.protobuf.DynamicMessage; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.Message; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; +import org.junit.Test; + +/** + * This test mimics the behavior when performing validation with protovalidate on a file descriptor + * set (as created by protoc --retain_options --descriptor_set_out=...). These + * descriptor types have the protovalidate extensions as unknown fields and need to be parsed with + * an extension registry for the constraints to be recognized and validated. + */ +public class ValidatorDynamicMessageTest { + + @Test + public void testFieldConstraintDynamicMessage() throws Exception { + DynamicMessage.Builder messageBuilder = + createMessageWithUnknownOptions(ExampleFieldConstraints.getDefaultInstance()); + messageBuilder.setField( + messageBuilder.getDescriptorForType().findFieldByName("regex_string_field"), "0123456789"); + Violation expectedViolation = + Violation.newBuilder() + .setConstraintId("string.pattern") + .setFieldPath("regex_string_field") + .setMessage("value does not match regex pattern `^[a-z0-9]{1,9}$`") + .build(); + assertThat(new Validator().validate(messageBuilder.build()).getViolations()) + .containsExactly(expectedViolation); + } + + @Test + public void testOneofConstraintDynamicMessage() throws Exception { + DynamicMessage.Builder messageBuilder = + createMessageWithUnknownOptions(ExampleOneofConstraints.getDefaultInstance()); + Violation expectedViolation = + Violation.newBuilder() + .setFieldPath("contact_info") + .setConstraintId("required") + .setMessage("exactly one field is required in oneof") + .build(); + assertThat(new Validator().validate(messageBuilder.build()).getViolations()) + .containsExactly(expectedViolation); + } + + @Test + public void testMessageConstraintDynamicMessage() throws Exception { + DynamicMessage.Builder messageBuilder = + createMessageWithUnknownOptions(ExampleMessageConstraints.getDefaultInstance()); + messageBuilder.setField( + messageBuilder.getDescriptorForType().findFieldByName("secondary_email"), + "something@somewhere.com"); + Violation expectedViolation = + Violation.newBuilder() + .setConstraintId("secondary_email_depends_on_primary") + .setMessage("cannot set a secondary email without setting a primary one") + .build(); + assertThat(new Validator().validate(messageBuilder.build()).getViolations()) + .containsExactly(expectedViolation); + } + + private static void gatherDependencies( + Descriptors.FileDescriptor fd, Set dependencies) { + dependencies.add(fd.toProto()); + for (Descriptors.FileDescriptor dependency : fd.getDependencies()) { + gatherDependencies(dependency, dependencies); + } + } + + private static DescriptorProtos.FileDescriptorSet createFileDescriptorSetForMessage( + Descriptors.Descriptor message) { + DescriptorProtos.FileDescriptorSet.Builder builder = + DescriptorProtos.FileDescriptorSet.newBuilder(); + Set dependencies = new LinkedHashSet<>(); + gatherDependencies(message.getFile(), dependencies); + builder.addAllFile(dependencies); + return builder.build(); + } + + private static Descriptors.FileDescriptor getFileDescriptor( + String name, Map fds) + throws Descriptors.DescriptorValidationException { + DescriptorProtos.FileDescriptorProto fdProto = fds.get(name); + if (fdProto == null) { + throw new IllegalArgumentException("unable to file file descriptor proto: " + name); + } + Descriptors.FileDescriptor[] dependencies = + new Descriptors.FileDescriptor[fdProto.getDependencyCount()]; + for (int i = 0; i < fdProto.getDependencyCount(); i++) { + dependencies[i] = getFileDescriptor(fdProto.getDependency(i), fds); + } + return Descriptors.FileDescriptor.buildFrom(fdProto, dependencies); + } + + private static DynamicMessage.Builder createMessageWithUnknownOptions(Message message) + throws InvalidProtocolBufferException, Descriptors.DescriptorValidationException { + DescriptorProtos.FileDescriptorSet fds = + createFileDescriptorSetForMessage(message.getDescriptorForType()); + // Reparse file descriptor set from encoded form (loses known extensions). + fds = DescriptorProtos.FileDescriptorSet.parseFrom(fds.toByteArray()); + Map fdsMap = + fds.getFileList().stream() + .collect( + Collectors.toMap( + DescriptorProtos.FileDescriptorProto::getName, Function.identity())); + Descriptors.FileDescriptor descriptor = + getFileDescriptor(message.getDescriptorForType().getFile().getName(), fdsMap); + return DynamicMessage.newBuilder( + descriptor.findMessageTypeByName(message.getDescriptorForType().getName())); + } +}