Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[HV-1831] filters @Valid annotations from jvm and native types #1334

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

[HV-1831] filters @Valid annotations from jvm and native types #1334

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ea0e2c
[HV-1831] filters @Valid annotations from jvm and native types
Nov 1, 2023
59ce309
[HV-1831] filters @Valid annotations from String, boxed primitives an…
Nov 3, 2023
315214e
[HV-1831] adds testcase for cascading with int in list and set
Nov 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 27 additions & 0 deletions engine/src/main/java/org/hibernate/validator/internal/metadata/core/ConstraintHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import jakarta.validation.Valid;
import org.hibernate.validator.constraints.CodePointLength;
import org.hibernate.validator.constraints.ConstraintComposition;
import org.hibernate.validator.constraints.CreditCardNumber;
Expand Down Expand Up @@ -324,6 +325,7 @@
import org.hibernate.validator.internal.constraintvalidators.hv.time.DurationMaxValidator;
import org.hibernate.validator.internal.constraintvalidators.hv.time.DurationMinValidator;
import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorDescriptor;
import org.hibernate.validator.internal.properties.Constrainable;
import org.hibernate.validator.internal.util.CollectionHelper;
import org.hibernate.validator.internal.util.Contracts;
import org.hibernate.validator.internal.util.logging.Log;
Expand Down Expand Up @@ -381,6 +383,7 @@ public class ConstraintHelper {
private static final Log LOG = LoggerFactory.make( MethodHandles.lookup() );
private static final String JODA_TIME_CLASS_NAME = "org.joda.time.ReadableInstant";
private static final String JAVA_MONEY_CLASS_NAME = "javax.money.MonetaryAmount";
private static final String BUILTIN_TYPE_NAMES = "(boolean|byte|char|int|short|double|long)";

@Immutable
private final Map<Class<? extends Annotation>, List<? extends ConstraintValidatorDescriptor<?>>> enabledBuiltinConstraints;
Expand Down Expand Up @@ -1172,6 +1175,30 @@ private static <T> T run(PrivilegedAction<T> action) {
return System.getSecurityManager() != null ? AccessController.doPrivileged( action ) : action.run();
}

/**
* this method inspects the type of the <code>@Valid</code> annotation and decides, if the annotation is useful.
* <p>
* This method returns false, if the {@link jakarta.validation.Valid} annotation is applied to:
* <ul>
* <li>a native type (int, boolean, etc</li>
* <li>a type in a java.* or javax.* package</li>
* </ul>
* </p>
* @param annotation the Valid annotation
* @param constrainable the constraint element
* @return true, if the Valid annotation should not be applied
* @param <A> type of annotation
*/
public <A extends Annotation> boolean isNonApplicableValidAnnotation(A annotation, Constrainable constrainable) {
if ( !( annotation instanceof Valid ) ) {
return false;
}

return constrainable.getType().getTypeName().startsWith( "java." )
|| constrainable.getType().getTypeName().startsWith( "javax." )
|| constrainable.getType().getTypeName().matches( BUILTIN_TYPE_NAMES );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah ... I can see how it can be tempting to ignore these types 😃.
this probably will work for the primitives, but we cannot safely discard any classes. The user may define a model as:

class SomeClass {
    @Valid
    Object object;
}

which is a valid use case and the type of object field is checked at the runtime when SomeClass instance is passed for validation. So even if the class from these packages will not have any constraints, someone might extend the class and apply the constraints in that extended class...
Another problem is that even though these classes won't have any constraints declared as annotations, a user can always define a constraint through a programmatic DSL, or via XML...
That's why I'm leaning more towards this being a problem of the code generators that are adding redundant @Valid annotations ...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, good point. I thought about making this configurable, but I need to look at other config properties and how these are wired. I will remove the java check.

}

/**
* A type-safe wrapper around a concurrent map from constraint types to
* associated validator classes. The casts are safe as data is added trough
Expand Down
5 changes: 5 additions & 0 deletions ...n/java/org/hibernate/validator/internal/metadata/provider/AnnotationMetaDataProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,11 @@ protected <A extends Annotation> List<ConstraintDescriptorImpl<?>> findConstrain
return Collections.emptyList();
}

// address HV-1831: do not create an Annotation object for unwanted Valid annotations
if ( constraintCreationContext.getConstraintHelper().isNonApplicableValidAnnotation( annotation, constrainable ) ) {
return Collections.emptyList();
}

List<Annotation> constraints = newArrayList();
Class<? extends Annotation> annotationType = annotation.annotationType();
if ( constraintCreationContext.getConstraintHelper().isConstraintAnnotation( annotationType ) ) {
Expand Down
12 changes: 12 additions & 0 deletions .../src/test/java/org/hibernate/validator/test/internal/engine/traversableresolver/Suit.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,14 @@ public class Suit {

@Max(value = 50, groups = { Default.class, Cloth.class })
@Min(1)
@Valid // this should be ignored
private Integer size;
@Valid private Trousers trousers;
private Jacket jacket;

@Valid
private boolean awesomeDesign;

public Trousers getTrousers() {
return trousers;
}
Expand All @@ -48,4 +52,12 @@ public Integer getSize() {
public void setSize(Integer size) {
this.size = size;
}

public boolean isAwesomeDesign() {
return awesomeDesign;
}

public void setAwesomeDesign(boolean awesomeDesign) {
this.awesomeDesign = awesomeDesign;
}
}