Skip to content

Allow explicitly declared null in providedArgs when no argument resolver matches #35192

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.bind.annotation.RestControllerAdvice;
import org.springframework.web.method.HandlerMethod;

import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.forwardedUrl;
Expand Down Expand Up @@ -62,6 +63,14 @@ void globalExceptionHandlerMethod() throws Exception {
.andExpect(status().isOk())
.andExpect(forwardedUrl("globalErrorView"));
}

@Test
void nullHandlerMethod() throws Exception {
standaloneSetup(new PersonController()).setControllerAdvice(new GlobalExceptionHandler()).build()
.perform(get("/invalid"))

Choose a reason for hiding this comment

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

The test only covers the scenario where handlerMethod is null, but does not cover multiple parameters, complex types, or nested parameters. Additional tests are suggested.

.andExpect(status().isOk())
.andExpect(forwardedUrl("null handlerMethod exception"));
}
}


Expand Down Expand Up @@ -92,6 +101,14 @@ private static class GlobalExceptionHandler {
String handleException(IllegalStateException exception) {
return "globalErrorView";
}

@ExceptionHandler(Exception.class)
String handleAllExceptions(IllegalStateException exception, HandlerMethod handlerMethod) {
if (handlerMethod == null) {
return "null handlerMethod exception";
}
return "simulated exception";
}
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,10 @@ public void setMethodValidator(@Nullable MethodValidator methodValidator) {
continue;
}
if (!this.resolvers.supportsParameter(parameter)) {
if (isParameterDeclaredButNull(parameter, providedArgs)) {
args[i] = null;

Choose a reason for hiding this comment

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

The new isParameterDeclaredButNull method checks if a parameter is explicitly set to null in providedArgs and assigns null directly, bypassing the exception. While this improves compatibility in some cases, it may hide the real cause of argument resolution failures, reducing code robustness and maintainability.

continue;
}
throw new IllegalStateException(formatArgumentError(parameter, "No suitable resolver"));
}
try {
Expand All @@ -237,6 +241,27 @@ public void setMethodValidator(@Nullable MethodValidator methodValidator) {
return args;
}

/**
* If there is null in providedArgs and the paramType does not have any non-null value matching, then consider null to be explicit.
*/
private boolean isParameterDeclaredButNull(MethodParameter parameter, @Nullable Object[] providedArgs) {
if (ObjectUtils.isEmpty(providedArgs)) {
return false;
}
Class<?> paramType = parameter.getParameterType();
boolean hasNull = false;
for (Object arg : providedArgs) {
if (arg == null) {
hasNull = true;
continue;
}
if (paramType.isAssignableFrom(arg.getClass())) {
return false;
}
}
return hasNull;
}

/**
* Invoke the handler method with the given argument values.
*/
Expand Down