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

ResourceHttpRequestHandler throws method not allowed even if resource doesn't exist #4876

Closed
graemerocher opened this issue Jan 4, 2016 · 10 comments

Comments

@graemerocher
Copy link
Contributor

Not sure if this should be raised against Spring or Spring Boot but it relates to

grails/grails-core#9430

Current behaviour of ResourceHttpRequestHandler is that it first checks if the HTTP method is valid and if it is not throws an exception, regardless if the underlying resource exists or not. This means if you perform a POST to an endpoint that doesn't exist you receive a 405 instead of a 404, which seems erroneous to me.

I am opening this up for discussion to see whether this can result in the associated Grails issue being addressed:

grails/grails-core#9430

@graemerocher
Copy link
Contributor Author

Anybody from the Boot team have any thoughts on this?

@wilkinsona
Copy link
Member

It feels like it's working as designed to me.

Boot's default configuration is to use /** as the static path pattern. This means that a resource handler is registered to handle /** and it'll pick up any request that hasn't been handled by a handler with higher precedence.

Current behaviour of ResourceHttpRequestHandler is that it first checks if the HTTP method is valid and if it is not throws an exception, regardless if the underlying resource exists or not

One reason for ResourceHttpRequestHandler checking the request method first could be that it's far quicker to do that than to check for the existence of a resource.

If you don't like the default behaviour of ResourceHttpRequestHandler then a Spring Framework issue might be appropriate to discuss the possibility of changing it or making it configurable (/cc @rstoyanchev). Alternatively, Grails could change Boot's default static path pattern so that static resources are isolated under a specific path.

@wilkinsona wilkinsona removed the status: waiting-for-triage An issue we've not yet triaged label Jan 13, 2016
@graemerocher
Copy link
Contributor Author

@wilkinsona yeah I can understand the reasoning behind the approach, but I do agree with the original reporter that the error code is misleading. It has an impact on usability too because if you believe your URL routes are correct and you get a 405 it may lead you to think it is something wrong with your accepted HTTP methods configuration, when in fact it could be that your route definition is wrong so it sends users down a debugging path IMO.

I don't really want to introduce hacks into Grails to workaround the problem hence why I raised the issue here so maybe I should raise a Spring framework issue, interested to hear @rstoyanchev's thoughts

@wilkinsona
Copy link
Member

I don't really want to introduce hacks into Grails to workaround the problem

Changing the default static path pattern is far from a hack, IMO. It's a fully supported configuration option that can be changed via spring.mvc.static-path-pattern.

@graemerocher
Copy link
Contributor Author

ok I will experiment with that

@graemerocher
Copy link
Contributor Author

Ok that works and we will go with that approach. Thanks for the feedback.

@stephennimmo
Copy link

This issue has come up with me, but when implementing a REST api. I have a test which performs a GET on an api endpoint that does not exist ("/api/garbage") and it correctly returns and handles a 404. However, when i perform a POST on the same endpoint, it returns a 405, because as previously stated, it performs the checkRequest method, which does not have POST as a supported method, prior to checking if the endpoint exists.

The part that causes me grief is that the error on the POST does not get handled in my @ControllerAdvice. I have extended the ResponseEntityExceptionHandler and overridden the handleHttpRequestMethodNotSupported to be able to elegantly catch and handle the exception. The error on the GET is correctly caught and handled, but the POST is not, meaning I cannot handle the exception.

I believe there should be a correction made to check 404 first, then check 405 and ensure the advice intercepts the exceptions. Thoughts?

@rstoyanchev
Copy link
Contributor

@graemerocher apologies for missing this ticket so far. It may be old news but we did address this for 4.3 spring-projects/spring-framework@57b466f based on https://jira.spring.io/browse/SPR-13944.

@graemerocher
Copy link
Contributor Author

good to know. thanks.

@stephennimmo
Copy link

Thanks for replying @graemerocher. Thanks for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants