Avoid duplicating routes for HEAD requests.

Follow up to rails#15321

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.

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