As of Hibernate Validator 6.2, EL expressions are disabled by default

[sleberknight]

Some of the kiwi validation annotations will stop producing the expected messages as of Hibernate Validator 6.2 and beyond, because as of that version, "The Expression Language is now disabled by default for custom violations." (1)

This is a problem that we need to address by either (somehow) fixing it, or documenting how to fix it (i.e. kiwi users need to explicitly enable EL expressions at the "bean-methods" level), or changing some of our annotations so they don't use EL expressions.

At least one of our annotations (@Range) would need to change in a breaking way by removing the minLabel and maxLabel attributes, since we use EL to determine what the message should be based on whether the label attributes are empty or not, i.e.

org.kiwiproject.validation.Range.between.message=must be between ${minLabel.length() > 0 ? minLabel : min} and ${maxLabel.length() > 0 ? maxLabel : max}

Dropwizard 2.0.x (we're on 2.0.28 and will soon update to 2.0.29 which was just released on 4/5/2022) is still using 6.1.7 but version 2.1.0 upgrades to 6.2.x, at which point a bunch of our validation annotations will have broken messages.

You can customize a Validator instance to permit EL expressions at the constraint expression level and/or custom constraint violations. See the Javadocs in BaseHibernateValidatorConfiguration for the constraintExpressionLanguageFeatureLevel and customViolationExpressionLanguageFeatureLevel methods for more details.

Here's an example of building a Validator that permits EL expressions such that the kiwi validation annotations like @Range will have their messages interpolated as before 6.2.x:

// Allows EL by everything (which is a security risk, and why they disabled by default in 6.2)
var elFeatureLevel = ExpressionLanguageFeatureLevel.BEAN_METHODS;

var validator = Validation.byProvider(HibernateValidator.class)
    .configure()
    .constraintExpressionLanguageFeatureLevel(elFeatureLevel)
    .customViolationExpressionLanguageFeatureLevel(elFeatureLevel)
    .buildValidatorFactory()
    .build();

You can also configure things on the ValidatorFactory returned by buildValidatorFactory(), e.g.

var validatorFactory = Validation.byProvider(HibernateValidator.class)
    // ...configuration...
    .buildValidatorFactory();

var validator = validatorFactory.usingContext()
    // addValueExtractor
    // constraintValidatorFactory
    // clockProvider
    // messageInterpolator
    // parameterNameProvider
    // traversableResolver
    .getValidator();

We need to consider whether we'd want to allow callers to easily create Validator instances via KiwiValidations and enable EL interpolation. e.g. currently you can:

// create a new instance on each invocation
var validator = KiwiValidations.newValidator();

// or get a singleton instance
var validator = KiwiValidations.getValidator();

Maybe we'd want to overload these and add parameters when creating new instances (since the singleton instance would not change), e.g.

var dangerousFeatureLevel = ExpressionLanguageFeatureLevel.BEAN_METHODS;

// apply to both constraint expressions and custom violations
var validator = KiwiValidations.newValidator(dangerousFeatureLevel);

// or have separate parameters
var validator = KiwiValidations.newValidator(dangerousFeatureLevel, dangerousFeatureLevel);

// or a parameter object containing configuration information
var validatorConfig = ValidatorConfig.builder()
    .constraintExpressionLanguageFeatureLevel(dangerousFeatureLevel )
    .customViolationExpressionLanguageFeatureLevel(dangerousFeatureLevel )
    .build();
var validator = KiwiValidations.newValidator(validatorConfig);

// or have our own custom builder
var validator = KiwiValidations.newValidatorBuilder()
    .addMessageBundleName("com.acme.ValidationMessages")
    .constraintExpressionLanguageFeatureLevel(dangerousFeatureLevel )
    .customViolationExpressionLanguageFeatureLevel(dangerousFeatureLevel )
    .build();

With the last two, we start encroaching and duplicating parts of Hibernate Validator itself, so maybe we should not bother at all. We do provide the static setValidator method as a way to configure in unit tests and (once) at application startup.

Another thing we could do is use a callback, e.g.

public static Validator newHibernateValidator(Consumer<HibernateValidatorConfiguration> configurer) {
    var config = Validation.byProvider(HibernateValidator.class).configure();
    configurer.accept(config);
    return config.buildValidationFactory().getValidator();
}

// and using it...
var validator = KiwiValidations.newHibernateValidator(config ->
    config.constraintExpressionLanguageFeatureLevel(dangerousFeatureLevel)
        .customViolationExpressionLanguageFeatureLevel(dangerousFeatureLevel)
        .messageInterpolator(new ResourceBundleMessageInterpolator(
            new AggregateResourceBundleLocator(bundleNames)))
);

While flexible, the Consumer approach doesn't look all that nice in the code (in my opinion anyway).

Another option is to change our validator implementations to use the labels (e.g. minLabel) or the values (e.g. min). It's not the most pretty solution, but it works and more importantly does not require EL to be enabled. Basically, just split each template into two separate templates, one with just the values and one with the labels. If we simply say that we'll use the labels if one is defined (e.g. if there is a minLabel or a maxLabel), then we'll use labels.

So, for kiwi we'd take:

org.kiwiproject.validation.Range.between.message=must be between ${minLabel.length() > 0 ? minLabel : min} and ${maxLabel.length() > 0 ? maxLabel : max}

and split it into two templates:

org.kiwiproject.validation.Range.between.message.minMaxValues=must be between {min} and {max}
org.kiwiproject.validation.Range.between.message.minMaxLabels=must be between {minLabel} and {maxLabel}

Inside the validator implementation class, in the initialize method we do this:

    @Override
    public void initialize(Range constraintAnnotation) {
        this.range = constraintAnnotation;
        this.useLabels = isNotBlank(constraintAnnotation.minLabel()) ||
                isNotBlank(constraintAnnotation.maxLabel());
    }

And then in the isValid when adding errors, we select the template based on the value of the useLabels field, for example:

var templatePrefix = "org.kiwiproject.validation.Range.between.message.";
var template = "{" + templatePrefix + (useLabels ? "minMaxLabels" : "minMaxValues") + "}";

So if useLabels is true, then template will be {org.kiwiproject.validation.Range.between.message.minMaxLabels} which does not require EL. And if useLabels is false, the template used will be {org.kiwiproject.validation.Range.between.message.minMaxValues}.

Doing it this way, all our validation annotations will work at the default EL feature level in 6.2 and beyond (i.e. we won't need EL).

But since we also have a bunch of very custom validation annotations in various services (not generic, very specific to the problem domain), that also use EL, we ought to provide the capability to enable EL if desired.

My proposed solution is to provide a way to customize the Validator instances using one or more of the ways discussed above, and to also change the existing kiwi validator implementations so that they do not require EL to be enabled. If we find there is some case where this is "too difficult" we can either document the requirements, or perhaps just change the validation error message even if that ends up providing less information.

---

References:

1. Hibernate Validator Expression Language overhaul in Upgrade Notes for Dropwizard 2.1.x
2. Enabling Expression Language features in Hibernate Validator Reference Guide
3. Custom Contexts in Hibernate Validator Reference Guide. (This section contains the text stating "By default, Expression Language interpolation is disabled for custom violations...to avoid arbitrary code execution or sensitive data leak if message templates are built from improperly escaped user input")
4. Disable Expression Language by default for custom constraint violations in Hibernate issue tracker
5. Pull request - HV-1816 Disable Expression Language by default for custom constraint violations hibernate/hibernate-validator#1138
6. Kiwi's ValidationMessages.properties

[sleberknight]

Also see #689 which has some discussion regarding Validator creation. If we add a builder capability we will want to consider the needs of this discussion (enabling/disabling EL) and #689's message bundle customization.

[sleberknight]

For kiwi version 2.1.0:

• refactored kiwi's custom validators so they don't require EL. See Kiwi validators should not require use of EL (expression language) #721
• updated javadoc in KiwiValidations#addError, adding a warning about EL usage in custom validators. See Update java doc in KiwiValidations#addError warning about EL usage in custom validators #722

[sleberknight]

With #722 there is no action that needs to be taken on this anymore, so we can close this discussion.