iron / router

Router middleware for the Iron web framework.
165 stars 74 forks source link

Return MethodNotAllowed for request with wrong HTTP method #111

Open dbrgn opened 8 years ago

dbrgn commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

untitaker commented 8 years ago

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

sapsan4eg commented 7 years ago

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))
    }

3) And to make small changes in handle_options and handle_method functions. More detailed changes can be viewed in a fork of this repository https://github.com/sapsan4eg/router/commit/e92922d3b21618274f64ea002b245e06eda6ff4a

untitaker commented 7 years ago

Cool, please make a PR :)

sapsan4eg commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

Add PR https://github.com/iron/router/pull/131

untitaker commented 7 years ago

Closed by #131

untitaker commented 7 years ago

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