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

Return MethodNotAllowed for request with wrong HTTP method #111

Open
dbrgn opened this issue Mar 17, 2016 · 10 comments
Open

Return MethodNotAllowed for request with wrong HTTP method #111

dbrgn opened this issue Mar 17, 2016 · 10 comments

Comments

@dbrgn
Copy link

dbrgn commented Mar 17, 2016

It would be great if router would return a "HTTP 405 Method Not Allowed" for routes where a route exists, but a non-matching request method was used.

Example:

router.get("/things", thing_list_handler);
router.post("/things", thing_create_handler);

Here a PUT request to /things should result in a HTTP 405.

I realize that this could be implemented with an any route for all paths, but turning this on globally would make things much simpler.

@untitaker
Copy link
Member

Nice idea.

router currently first dispatches by method (selecting the appropriate router from a mapping), and only then by path: This would have to change.

@leoyvens
Copy link
Contributor

@untitaker Can't we just check for each method?

@untitaker
Copy link
Member

True, but that's slightly slower in the case of a 405.

@sapsan4eg
Copy link
Contributor

sapsan4eg commented Nov 6, 2016

I have solution of this case.

  1. in implementation router use this data type https://github.com/iron/router/blob/master/src/router.rs#L18
    pub routers: HashMap<method::Method, Recognizer<Box<Handler>>>
    I propose to replace
    pub routers: Recognizer<HashMap<method::Method, Arc<Handler>>>,
  2. Then you can replace the function recognize https://github.com/iron/router/blob/master/src/router.rs#L139 of the so
fn recognize(&self, method: &method::Method, path: &str)
                       -> Result<Match<&Box<Handler>>, IronError> {

        if let Some(s) = self.inner.routers.recognize(path).ok() {
            if let Some(h) = s.handler.get(method) {
                return Ok(Match::new(h, s.params))
            } else if let Some(s) = self.inner.wildcard.recognize(path).ok() {
                return Ok(s)
            } else {
                return Err(IronError::new(MethodNotAllowed, status::MethodNotAllowed))
            }
        } else if let Some(s) = self.inner.wildcard.recognize(path).ok() {
            return Ok(s)
        }
        Err(IronError::new(NoRoute, status::NotFound))
    }
  1. And to make small changes in handle_options and handle_method functions.
    More detailed changes can be viewed in a fork of this repository
    sapsan4eg@e92922d

@untitaker
Copy link
Member

Cool, please make a PR :)

@sapsan4eg
Copy link
Contributor

Hi guys, i have couple questions.

  1. Make a response only 405 if the method is not found on this url, but other methods are available? Theoretically, we can use the flag to use 405 or 404
  2. If we use the flag is a preferable response to return the default?

@untitaker
Copy link
Member

  1. Make a response only 405 if the method is not found on this url, but other methods are available?

Yes exactly. I don't think we need a flag.

@sapsan4eg
Copy link
Contributor

Add PR #131

@untitaker
Copy link
Member

Closed by #131

@untitaker untitaker reopened this Jan 23, 2017
@untitaker
Copy link
Member

I had to revert #131 because it introduced a bug. Resolving by path first was a mistake. I added a testcase in master and published 0.5.1

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

4 participants