joshbuddy / http_router

A kick-ass HTTP router for use in Rack
MIT License
198 stars 44 forks source link

Incorrectly returns 405 for non-matching route if HTTP method is specified. #31

Open bhb opened 11 years ago

bhb commented 11 years ago

If you add a route that includes a named variable and requires a specific HTTP method parameter, then requests that don't match will return a 405 instead of a 404 (even though the request did use the correct HTTP method, it just requested an invalid URL)

Repro

require 'rubygems'
require 'http_router'

router = HttpRouter.new.tap do |router|
  router.add("/bar/:id").to(lambda { |env| [200, {}, ["Hi"]] })
  router.get("/foo/:id").to(lambda { |env| [200, {}, ["Hi"]] })
end

# Just to show that it will 404 if the HTTP method is not specified above
response = router.call({
                         'SERVER_NAME' => 'localhost',
                         'SERVER_PORT' => 80,
                         'REQUEST_METHOD' => 'GET',
                         'PATH_INFO' => '/bar/ab/123213',
                       })

puts response.first # Actual: 404, Expected: 404.

# And now for the incorrect case ... 
response = router.call({
                         'SERVER_NAME' => 'localhost',
                         'SERVER_PORT' => 80,
                         'REQUEST_METHOD' => 'GET',
                         'PATH_INFO' => '/foo/ab/1234/asdfasfasdf',
                       })

puts response.first # Actual: 405, Expected: 404.
bhb commented 11 years ago

The behavior changes if you provide a regexp for the variable, but it depends on the regexp:

require 'rubygems'
require 'http_router'

router = HttpRouter.new.tap do |router|
  router.get("/foo/:id", :id => /$[a-zA-Z0-9]+/).to(lambda { |env| [200, {}, ["Hi"]] })
  router.get("/baz/:id", :id => /[a-zA-Z0-9]+/).to(lambda { |env| [200, {}, ["Hi"]] })
end

response = router.call({
                         'SERVER_NAME' => 'localhost',
                         'SERVER_PORT' => 80,
                         'REQUEST_METHOD' => 'GET',
                         'PATH_INFO' => '/foo/ab/112323',
                         'SCRIPT_NAME' => ""
                        })

puts response.first # Actual: 404, Expected: 404

response = router.call({
                          'SERVER_NAME' => 'localhost',
                          'SERVER_PORT' => 80,
                          'REQUEST_METHOD' => 'GET',
                          'PATH_INFO' => '/baz/ab/112323',
                          'SCRIPT_NAME' => ""
                       })

puts response.first # Actual: 405, Expected: 404