quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.36k stars 2.56k forks source link

`@NotEmpty`/`@NotNull` container element validators run against empty `Optional` #41262

Open krnhotwings opened 2 weeks ago

krnhotwings commented 2 weeks ago

Describe the bug

@GET
@Produces(MediaType.TEXT_PLAIN)
public String hello(@RestQuery Optional<@NotEmpty String> param) {
    return "Hello from Quarkus REST";
}

if ?param is not present, the validator will still run against the empty Optional and return a validation error instead of the expected 200 response.

Expected behavior

The validator should only run when the element is present. For example, Optional<@Email String> param will return 200 when param is not present but will validate param when present. Other containers like List<@NotEmpty String> param also work as expected. The issue seems to be explicitly Optional + @NotEmpty/@NotNull. I'm not sure if other single-element container types are also affected.

Actual behavior

Optional<@NotEmpty String> and @NotNull will run their validators against the empty element and return:

ViolationReport{title='Constraint Violation', status=400, violations=[Violation{field='hello.param', message='must not be empty'}]}

How to Reproduce?

  1. Create new project and add hibernate-validator extension
  2. See above example REST resource

Output of uname -a or ver

6.9.4-200.fc40.x86_64

Output of java -version

Temurin-21.0.3+9

Quarkus version or git rev

3.11.2

Build tool (ie. output of mvnw --version or gradlew --version)

gradle 8.8

Additional information

No response

krnhotwings commented 2 weeks ago

I should add that the bug essentially treats Optional<@NotEmpty String> param as @NotEmpty String param

krnhotwings commented 2 weeks ago

Ah, I should also mention that this also affects validation against beans where optionals are initialized to empty:

@POST
public String hello(@Valid MyBean bean) {
    return bean;
}
public class MyBean {
    public Optional<@NotEmpty String> param = Optional.empty();
}

The above will return a validation error against MyBean.param

quarkus-bot[bot] commented 1 day ago

/cc @gsmet (hibernate-validator), @marko-bekhta (hibernate-validator)

marko-bekhta commented 1 day ago

hey @krnhotwings

The problem you've encountered is caused by the value extractor for optional.. and also the @NotEmpty constraint. Unlike most of the other constraints, it does not allow null values.

looking at the spec here: https://jakarta.ee/specifications/bean-validation/3.0/jakarta-bean-validation-spec-3.0.html#valueextractordefinition-builtinvalueextractors

java.util.Optional; value() must be invoked, passing null as node name and passing the contained object as value or null if none is present

the behaviour you've described matches these expectations. If we'd want to change that -- that should be a change in the spec (feel free to start a discussion here https://github.com/jakartaee/validation/issues).

Now to how you can address your case to make things work as you'd like?

Override the value extractor for the optional

  1. Create an implementation like:

    public class OptionalValueExtractor implements ValueExtractor<Optional<@ExtractedValue ?>> {
    
    @Override
    public void extractValues(Optional<?> originalValue, ValueExtractor.ValueReceiver receiver) {
        // only pass an extracted value if something is present:
        if ( originalValue.isPresent() ) {
            receiver.value(null, originalValue.get());
        }
    }
    }
  2. Register this implementation via a META-INF/services/jakarta.validation.valueextraction.ValueExtractor
    some.package.where.the.impl.is.OptionalValueExtractor

This will override the builtin implementation of the value extractor and things will work as you'd wanted.

Do not use constraints that fail on null values on optional

As mentioned above, per spec if optional is empty -- null is passed for the validation. Soooo just don't use a constraint that fails on null values. You can create your own @NullOrNotEmpty and provide the implementation for the ConstraintValidator that would allow null values. Here's a blog post on how to add your custom constraints if you'd go this route: https://in.relation.to/2017/03/02/adding-custom-constraint-definitions-via-the-java-service-loader/ .