diff options
| -rw-r--r-- | actionpack/CHANGELOG.md | 8 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/http/request.rb | 6 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/journey/router.rb | 40 | ||||
| -rw-r--r-- | actionpack/test/dispatch/request_test.rb | 12 | ||||
| -rw-r--r-- | actionpack/test/journey/router_test.rb | 21 |
5 files changed, 69 insertions, 18 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 30986d0a6a..bdb14b69e5 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,11 @@ +* Avoid duplicating routes for HEAD requests. + + Instead of duplicating the routes, we will first match the HEAD request to + HEAD routes. If no match is found, we will then map the HEAD request to + GET routes. + + *Guo Xiang Tan*, *Andrew White* + * Requests that hit `ActionDispatch::Static` can now take advantage of gzipped assets on disk. By default a gzip asset will be served if the client supports gzip and a compressed file is on disk. diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index f35289253b..c441f7f95b 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -105,6 +105,12 @@ module ActionDispatch @request_method ||= check_method(env["REQUEST_METHOD"]) end + def request_method=(request_method) #:nodoc: + if check_method(request_method) + @request_method = env["REQUEST_METHOD"] = request_method + end + end + # Returns a symbol form of the #request_method def request_method_symbol HTTP_METHOD_LOOKUP[request_method] diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index 21817b374c..9131b65380 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -101,11 +101,13 @@ module ActionDispatch r.path.match(req.path_info) } - if req.env["REQUEST_METHOD"] === "HEAD" - routes.concat get_routes_as_head(routes) - end + routes = + if req.request_method == "HEAD" + match_head_routes(routes, req) + else + match_routes(routes, req) + end - routes.select! { |r| r.matches?(req) } routes.sort_by!(&:precedence) routes.map! { |r| @@ -118,19 +120,23 @@ module ActionDispatch } end - def get_routes_as_head(routes) - precedence = (routes.map(&:precedence).max || 0) + 1 - routes.select { |r| - r.verb === "GET" && !(r.verb === "HEAD") - }.map! { |r| - Route.new(r.name, - r.app, - r.path, - r.conditions.merge(request_method: "HEAD"), - r.defaults).tap do |route| - route.precedence = r.precedence + precedence - end - } + def match_head_routes(routes, req) + head_routes = match_routes(routes, req) + + if head_routes.empty? + begin + req.request_method = "GET" + match_routes(routes, req) + ensure + req.request_method = "HEAD" + end + else + head_routes + end + end + + def match_routes(routes, req) + routes.select { |r| r.matches?(req) } end end end diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 84bd392fd9..00b80c7357 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -650,6 +650,18 @@ class RequestMethod < BaseRequestTest end end + test "allow request method hacking" do + request = stub_request('REQUEST_METHOD' => 'POST') + + assert_equal 'POST', request.request_method + assert_equal 'POST', request.env["REQUEST_METHOD"] + + request.request_method = 'GET' + + assert_equal 'GET', request.request_method + assert_equal 'GET', request.env["REQUEST_METHOD"] + end + test "invalid http method raises exception" do assert_raise(ActionController::UnknownHttpMethod) do stub_request('REQUEST_METHOD' => 'RANDOM_METHOD').request_method diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index 2e7e8e1bea..8fc965adf4 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -503,6 +503,25 @@ module ActionDispatch assert called end + def test_recognize_head_route + path = Path::Pattern.from_string "/books(/:action(.:format))" + app = Object.new + conditions = { request_method: 'HEAD' } + @router.routes.add_route(app, path, conditions, {}) + + env = rails_env( + 'PATH_INFO' => '/books/list.rss', + 'REQUEST_METHOD' => 'HEAD' + ) + + called = false + @router.recognize(env) do |r, params| + called = true + end + + assert called + end + def test_recognize_head_request_as_get_route path = Path::Pattern.from_string "/books(/:action(.:format))" app = Object.new @@ -561,7 +580,7 @@ module ActionDispatch end def rails_env env, klass = ActionDispatch::Request - klass.new env + klass.new(rack_env(env)) end def rack_env env |