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

Spring MVC raises MissingPathVariableException resulting in 500 instead of 400 error when path segment is u001F or u00D and cannot be converted to target type UUID #31382

Closed
ltsallas opened this issue Oct 8, 2023 · 2 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Milestone

Comments

@ltsallas
Copy link

ltsallas commented Oct 8, 2023

This is an corner case, discovered while performing some property-based testing for my API.

Affects spring-web:6.0.12

Here is a minimal code sample that reproduces the issue

https://gist.github.com/ltsallas/9903343b6b7a90013cdd155bfcd670ce

I think the the last two tests in the DemoControllerTest should return a 400(MethodArgumentTypeMismatchException) and not a 500(MissingPathVariableException) response.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 8, 2023
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Oct 20, 2023

This case where a value is present, but becomes null after conversion (StringToUUIDConverter calls UUID.fromString) was addressed in #26679. We now detect it and raise MissingPathVariableException, which is what we do generally with missing path variables since that could be an incorrectly defined URI template.

That said in this case a value is present, but not valid and indeed should be treated as a 400. As part of #26679 we did add a missingAfterConversion flag to MissingRequestValueException and could use that to differentiate this case from the more general case of a missing path variable.

@rstoyanchev rstoyanchev self-assigned this Oct 20, 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 20, 2023
@rstoyanchev rstoyanchev added this to the 6.0.14 milestone Oct 20, 2023
@rstoyanchev rstoyanchev changed the title Spring MVC throws http 500(MissingPathVariableException) instead of 400 (MethodArgumentTypeMismatchException) when u001F, u00D send as @PathVariable that cannot be matched to a type Spring MVC raises MissingPathVariableException resulting in 500 instead of 400 error when path segment is u001F or u00D and cannot be converted to target type UUID Oct 20, 2023
@ltsallas
Copy link
Author

Thanks for taking a look into this one @rstoyanchev
Are you suggesting this change in the getStatusCode() of the MissingPathVariableException ?

@Override
   public HttpStatusCode getStatusCode() {
       return isMissingAfterConversion() ? super.getStatusCode() : HttpStatus.INTERNAL_SERVER_ERROR;
   }

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