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

HandlerMethodValidationException.Visitor incorrectly triggers other() method instead of requestParam() for @RequestParam validation errors #31329

Closed
unwx opened this issue Sep 27, 2023 · 1 comment
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Milestone

Comments

@unwx
Copy link

unwx commented Sep 27, 2023

Environment

Spring Boot: 3.2.0-M3
Spring Web: spring-web-6.1.0-M5.jar
Java: 17


Expected Behavior:

When a @RequestParam value is invalid, HandlerMethodValidationException.Visitor should call the requestParam(RequestParam, ParameterValidationResult) method.

Observed Behavior:

Instead, HandlerMethodValidationException.Visitor calls other(ParameterValidationResult).

Reproduce the error

Spring Initializr

@RestController
@RequestMapping("/api")
public class Controller {
    @GetMapping(
            value = "/invalid",
            produces = MediaType.APPLICATION_JSON_VALUE)
    public ResponseEntity<Object> invalid(@RequestParam @Positive int num) {
        return ResponseEntity.ok(Map.of("positive", num));
    }
}
@RestControllerAdvice
public class ExceptionHandler extends ResponseEntityExceptionHandler {
    @Override
    protected ResponseEntity<Object> handleHandlerMethodValidationException(HandlerMethodValidationException ex,
                                                                            HttpHeaders headers,
                                                                            HttpStatusCode status,
                                                                            WebRequest request) {

        AtomicReference<ResponseEntity<Object>> responseEntityRef = new AtomicReference<>();
        ex.visitResults(new HandlerMethodValidationException.Visitor() {
            /*
             * @Overrides...
             */
            
            @Override
            public void requestParam(RequestParam requestParam, ParameterValidationResult result) {
                responseEntityRef.set(new ResponseEntity<>(Map.of("invalid_param", ex.getMessage()), HttpStatus.BAD_REQUEST));
            }

            @Override
            public void other(ParameterValidationResult result) {
                throw new IllegalStateException("Why not @RequestParam?");
            }
        });

        return responseEntityRef.get();
    }
}

Execute

curl --location 'localhost:8080/api/invalid?num=-5'

Debugging Insights

In HandlerMethodValidationException#visitResults(HandlerMethodValidationException.Visitor):

if (this.requestParamPredicate.test(param)) {
    RequestParam requestParam = param.getParameterAnnotation(RequestParam.class);
    visitor.requestParam(requestParam, result);
    continue;
}

Line this.requestParamPredicate.test(param) is false, because:

In org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter#methodParamPredicate(List, Class)}

return parameter -> {
    for (HandlerMethodArgumentResolver resolver : resolvers) {
        if (resolver.supportsParameter(parameter)) {
            return resolverType.isInstance(resolver);
        }
    }
    return false;
};

Line return resolverType.isInstance(resolver); returns false, because:

resolverType is class org.springframework.web.service.invoker.RequestParamArgumentResolver and
resolver.getClass() is class org.springframework.web.method.annotation.RequestParamMethodArgumentResolver

The mismatch between resolverType and resolver.getClass() appears to be the crux of the problem.


Thank you for looking into this!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 27, 2023
@rstoyanchev rstoyanchev self-assigned this Oct 19, 2023
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 19, 2023
@rstoyanchev rstoyanchev added this to the 6.1.0-RC2 milestone Oct 19, 2023
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Oct 19, 2023

Thanks for giving this a try and narrowing the cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants