Spring Property dependent @RequestParam parameters are not resolved on bad request

Affects: \3.2.2

I came across this issue a while ago and since I can't find it anywhere nor create an easy workaround, I create this issue.

A bit of context first : I created my own fully customizable authentication service built over spring, and on the controller, I have the following method :

@PostMapping("${security.authentication.log-in.mapping}")
public ResponseEntity<Void> handleLogIn(@RequestParam("${security.authentication.identifier.parameter.name}") String identifier,
                                        @RequestParam("${security.authentication.raw-password.parameter.name}") String rawPassword) {
    //My authentication logic
}

With the following properties :

security.authentication.log-in.mapping=logIn
security.authentication.identifier.parameter.name=username
security.authentication.raw-password.parameter.name=password

The references to the properties are resolved as expected during normal situations, thus allowing me to fully customize the authentication endpoint mapping and parameter names.

However, if one of the property is misspelled or missing, The default error message of the BAD_REQUEST response does not resolve the property see the image below (mispelling usernam on purpose) : After sending the following POST request http://localhost:443/authentication/logIn?usernam=user&password=pwd Spring Property dependent @RequestParam parameters are not resolved on bad request

The expected message would be "Required parameter 'username' is not present."

Comment From: injae-kim

https://github.com/spring-projects/spring-framework/blob/d81619addd6d9060c9d8dc454c359054e517ed98/spring-web/src/main/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolver.java#L217

https://github.com/spring-projects/spring-framework/blob/d81619addd6d9060c9d8dc454c359054e517ed98/spring-web/src/main/java/org/springframework/web/bind/MissingServletRequestParameterException.java#L88-L90

Required paramter '${paramterName}' is not present.

Maybe we simply set above error message with parameterName now.

@RequestParam("${security.authentication.identifier.parameter.name}")`

In this case, ${security.authentication.identifier.parameter.name} is parameterName so ${security..} is set on error message directly.

The expected message would be "Required parameter 'username' is not present."

Hmm should we set real value when ${..} is used on @RequestParam instead of paramterName? (With current error message, anyway we can find which request param is not exist although it's not convenient 😅)

Comment From: KierannCode

I didn't dive into the source code, and I don't know where the inconsistency comes from (the parameter name is resolved during server startup, but the error message does not use it, it uses its own logic to resolve it).

The workaround I thought about was to intercept the exception, detect and replace the placeholder manually in the string by the configured parameter name, and update the exception message with some reflection (since I can only rethrow the same exception in a exception handler). Overall very smelly, I don't even want to try.

I thought maybe the solution would just be to resolve the parameter name the same way it's done at startup (using SpEL and properties probably).

Comment From: andrea-mauro

Sorry to jump in... I was looking into some good-first-issue to contribute to and dug a bit into this. What about using resolvedName.toString() at line 124 (and also at line 127) instead of namedValueInfo.name?

Comment From: injae-kim

What about using resolvedName.toString() at line 124 (and also at line 127) instead of namedValueInfo.name?

@andrea-mauro I think we need maintainer's opinion here :) let's wait maintainer's comment~!

Comment From: simonbasle

This looks like a good improvement on the error message, and I think that the fix proposed by @andrea-mauro on calls to handleMissingValue and handleNullValue is relevant. Marking this as an enhancement that targets 6.2 (the main branch).

Comment From: simonbasle

@andrea-mauro do you want to contribute your fix? Note that some unit testing would be required, and that the same pattern occurs in several places that all need to be fixed for consistency (look for multiple NamedValueInfo classes and their usages).

As for tests, you can take inspiration from e.g. RequestHeaderMethodArgumentResolverTests here.

Comment From: andrea-mauro

Hey @simonbasle. Sure, I can take it and thanks for the tips

Comment From: simonbasle

Closing as superseded by #32462