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.

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

SpringValidatorAdapter is incorrectly resolving rejected value for bean based field level constraints [SPR-9332] #13970

Closed
spring-projects-issues opened this issue Apr 18, 2012 · 6 comments
Assignees
Labels
in: core type: bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Apr 18, 2012

Pavel Horal opened SPR-9332 and commented

SpringValidatorAdapter is converting JSR-303 ConstraintViolations to Spring's FieldErrors. This conversion contains a flaw, when the violation occurs on FIELD level constraint, which is validated as bean (e.g. subform).

Consider following example:

public class Address {

    private String street;
    private String number;

    // Getters and Setters

}
public class UserForm {

    @NotEmpty
    private String name;
    @AddressValid(/* some parameters */)
    private Address contactAddress;
    @AddressValid(/* some parameters */)
    private Address billingAddress;


    // Getters and Setters

}
public class AddressValidator implements ConstraintValidator<AddressValid, Address> {

    // initialize ...

    public boolean isValid(Address address, ConstraintValidationContext context) {
        // check null, disableDefaultConstraintViolation ...

        if (street == null || street.length() == 0) {
            context.buildConstraintViolationWithTemplate("{validation.noStreet}")
                    .addNode(street).addConstraintViolation();
            return false;
        );
        return true;
    }

}

When submitting empty street the validator above will produce ConstraintValidation with Address instance as the invalidValue and UserForm instance as the leafBean. Now consider current SpringValidatorAdapter code:

protected void processConstraintViolations(Set<ConstraintViolation<Object>> violations, Errors errors) {
	for (ConstraintViolation<Object> violation : violations) {
		// ... irrelevant part of code
				if (errors instanceof BindingResult) {
					BindingResult bindingResult = (BindingResult) errors;
					String nestedField = bindingResult.getNestedPath() + field;
					if ("".equals(nestedField)) {
						String[] errorCodes = bindingResult.resolveMessageCodes(errorCode);
						bindingResult.addError(new ObjectError(
								errors.getObjectName(), errorCodes, errorArgs, violation.getMessage()));
					}
					else {
						Object invalidValue = violation.getInvalidValue();
						// ... THIS IS WHAT IS WRONG - the rejected value is the whole bean Address!!!
						if (!"".equals(field) && invalidValue == violation.getLeafBean()) {
							// bean constraint with property path: retrieve the actual property value
							invalidValue = bindingResult.getRawFieldValue(field);
						}
						String[] errorCodes = bindingResult.resolveMessageCodes(errorCode, field);
						bindingResult.addError(new FieldError(
								errors.getObjectName(), nestedField, invalidValue, false,
								errorCodes, errorArgs, violation.getMessage()));
					}
				}
		// ... irrelevant part of code
	}
}

The impact of this behavior is critical - Spring's FORM tags are using rejected values as default values in forms. This means, that the user instead of the rejected value will see result of toString() method of the parent bean.

Correct behavior would be to check if the field contains path separator '*.*' instead of just checking invalidValue == violation.getLeafBean().


Affects: 3.1.1, 3.2 M1

Attachments:

Issue Links:

  • #14876 GenericConversionService.convert() throws IllegalArgumentException after updating to Spring 3.2.1
  • #19648 Spring validation crashes with Hibernate Validation 5 style list constraint violations
  • #20725 NumberFormatException caused by property paths from JSR-303 based validation with no index into a collection

Referenced from: commits 8e75eee

1 votes, 2 watchers

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Apr 18, 2012

Pavel Horal commented

One additional thought - this might also mean that there is critical flaw in Spring's FORM tags as they are using rejectedValue in case no bindingError occured (hence the rejectedValue should be on the bean itself, not on the error object). Maybe some responsible person from that part of framework might provide another information.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Apr 18, 2012

Pavel Horal commented

There is a workaround - changing the FIELD level constraint to TYPE (bean) level constraint. That will not allow for custom constraint parameters. That case needs to be solved by additional bean attributes, which can be checked by the validator.

The issue probably is not CRITICAL. Sorry for the high initial priority/severity.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Apr 20, 2012

Pavel Horal commented

-I have to take back my previous comment - the workaround is not working. In case you will use TYPE level constraint on inner bean, the leafBean is the root bean and the invalidValue is still the inner bean.-

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Apr 20, 2012

Pavel Horal commented

Added unit test (JUnit 4+, Hibernate Validator 4.2, Spring 3.1.1).

The problem is really only when the annotation is used on FIELD level. Putting the validation on TYPE level workarounds the problem (I made a mistake in my code before writing previous comment).

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jan 22, 2013

Juergen Hoeller commented

After some consideration, I've fixed this through a different kind of check along the lines that you suggested. This works for all of our tests but since there might be unexpected side effects, I'm fixing this for the 3.2 line only: i.e. for the 3.2.1 release coming up later this week.

Juergen

@hetalmshah
Copy link

@hetalmshah hetalmshah commented Jul 25, 2019

I am seeing the same issue with Spring Boot 2.1.5.RELEASE.
Is it resolved there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core type: bug
Projects
None yet
Development

No branches or pull requests

3 participants