Skip to content

RequestMappingInfoHandlerMapping.handleNoMatch returns null instead of throwing an exception [SPR-10193] #14826

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
spring-projects-issues opened this issue Jan 18, 2013 · 6 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jan 18, 2013

Shelley J. Baker opened SPR-10193 and commented

Update: Original request below is to raise HttpRequestMethodNotSupportedException. Actual solution raises UnsatisfiedServletRequestParameterException.


When a request is made to a URI using an unsupported method, a 404 error is returned instead of a 405 when another mapping exists for that method with matching parameters.

For example, given the following mappings:

@Controller
@RequestMapping("/test")
public class TestController {

    @RequestMapping(method = RequestMethod.GET)
    public void get() {}

    @RequestMapping(method = RequestMethod.POST, params = "test")
    public void post() {}
}

The following occurs:

POST /test

expected: 405
actual: 404

Specifically, the RequestMappingInfoHandlerMapping handleNoMatch method returns null instead of throwing the HttpRequestMethodNotSupportedException as expected.

This is a regression between 3.1.2 and 3.1.3; previously, the 405 occurred as expected. This may be related to the changes introduced by #14237.


Affects: 3.1.3

Referenced from: commits 72013f9, 3c09b07

Backported to: 3.1.4

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Why do you consider 405 to be the correct status? POST is allowed! The issue is with the missing parameter. It would be strange also to issue a POST request and to get back a 405 with the "Allow" response header listing GET and POST.

@spring-projects-issues
Copy link
Collaborator Author

Shelley J. Baker commented

Because a POST is not allowed at the specified Request-URI, /test. POST is only allowed at the Request-URI, /test?test. Returning a 404 would imply that nothing is found at the requested resource, /test, which is not true - the resource exists and GET is allowed at this URI.

I suppose you could make the argument that a 404 is returned because "the server does not wish to reveal exactly why the request has been refused" [1], but why not return a more specific status code 405 to indicate that the method (POST) is not allowed at the provided URI?

[1] http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Hm, I see what you mean about the parameter being included in the resource URI. However, I do think that can vary by use case. It's more common to use the path to identify the resource while query parameters may or may not apply. For example a product id in the query, helps to identify the target resource but a search results parameter does not.

I do agree that returning 404 is not the most intuitive outcome either. Perhaps raising UnsatisfiedServletRequestParameterException as the DefaultAnnotationHandlerMapping does? That gets translated into a 400 error via DefaultHandlerExceptionResolver.

@spring-projects-issues
Copy link
Collaborator Author

Shelley J. Baker commented

I agree that query params are commonly used as you described, in which case a 405 may or may not be appropriate. However, in this particular case, a 405 still seems most appropriate. If the query parameters are optional or used in the way you describe (such that they may or may not apply), they won't be specified in the @RequestMapping, but in this case, the params are a required part of the @RequestMapping/Request-URI.

The UnsatisfiedServletRequestParameterException seems like an appropriate exception; however, I don't agree that a 400 is appropriate for this case as this indicates that the request could not be understood by the server due to malformed syntax, which is not true - the syntax is valid and request is understood.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Well, I disagree that simply requiring a parameter makes it part of what identifies the target resource. I can require items completely unrelated to the target resource in which case returning 405 is odd.

400 is what we've been returning in case of missing Servlet request parameters for a while except in the case of RequestMappingHandlerMapping where we decided not to reveal the source of the error. The 400 error essentially means it's a client error and the client should not try the exact same request again because it won't succeed. So I think it's perfectly adequate. If you don't like that status code, you can handle the exception and return a 405 instead. I can see that as being appropriate in some cases. We do agree on raising a meaningful exception at least.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

RequestMappingInfoHandlerMapping now raises UnsatisfiedServletRequestParameterException if there are unmatched request mapping parameter conditions.

@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement in: core Issues in core modules (aop, beans, core, context, expression) labels Jan 11, 2019
@spring-projects-issues spring-projects-issues added this to the 3.2.1 milestone Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants