Don't raise ActionController::UnknownHttpMethod from ActionDispatch::Static

The `ActionDispatch::Static` middleware is used low down in the stack to serve
static assets before doing much processing. Since it's called from so low in
the stack, we don't have access to the request ID at this point, and generally
won't have any exception handling defined (by default `ShowExceptions` is added
to the stack quite a bit higher and relies on logging and request ID).

Before https://github.com/rails/rails/commit/8f27d6036a2ddc3cb7a7ad98afa2666ec163c2c3
this middleware would ignore unknown HTTP methods, and an exception about these
would be raised higher in the stack. After that commit, however, that exception
will be raised here.

If we want to keep `ActionDispatch::Static` so low in the stack (I think we do)
we should suppress the `ActionController::UnknownHttpMethod` exception here,
and instead let it be raised higher up the stack, once we've had a chance to
define exception handling behaviour.

This PR updates `ActionDispatch::Static` so it passes `Rack::Request` objects to
`ActionDispatch::FileHandler`, which won't raise an
`ActionController::UnknownHttpMethod` error. If an unknown method is
passed, it should exception higher in the stack instead, once we've had a
chance to define exception handling behaviour.`

3 files changed, 21 insertions, 3 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index c6a942ddd1..1bb37d661a 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,12 @@
+*   Don't raise ActionController::UnknownHttpMethod from ActionDispatch::Static
+
+    Pass `Rack::Request` objects to `ActionDispatch::FileHandler` to avoid it
+    raising `ActionController::UnknownHttpMethod`. If an unknown method is
+    passed, it should exception higher in the stack instead, once we've had a
+    chance to define exception handling behaviour.
+
+    *Grey Baker*
+
 *   Handle `Rack::QueryParser` errors in `ActionDispatch::ExceptionWrapper`
 
     Updated `ActionDispatch::ExceptionWrapper` to handle the Rack 2.0 namespace
diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb
index 2c5721dc22..4161c1d110 100644
--- a/actionpack/lib/action_dispatch/middleware/static.rb
+++ b/actionpack/lib/action_dispatch/middleware/static.rb
@@ -46,7 +46,7 @@ module ActionDispatch
     end
 
     def call(env)
-      serve ActionDispatch::Request.new env
+      serve(Rack::Request.new(env))
     end
 
     def serve(request)
@@ -82,7 +82,7 @@ module ActionDispatch
       end
 
       def gzip_encoding_accepted?(request)
-        request.accept_encoding =~ /\bgzip\b/i
+        request.accept_encoding.any? { |enc, quality| enc =~ /\bgzip\b/i }
       end
 
       def gzip_file_path(path)
@@ -119,7 +119,7 @@ module ActionDispatch
     end
 
     def call(env)
-      req = ActionDispatch::Request.new env
+      req = Rack::Request.new env
 
       if req.get? || req.head?
         path = req.path_info.chomp('/'.freeze)
diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb
index ea8b5e904e..1036758f35 100644
--- a/actionpack/test/dispatch/static_test.rb
+++ b/actionpack/test/dispatch/static_test.rb
@@ -160,6 +160,9 @@ module StaticTests
     response  = get(file_name, 'HTTP_ACCEPT_ENCODING' => 'GZIP')
     assert_gzip  file_name, response
 
+    response  = get(file_name, 'HTTP_ACCEPT_ENCODING' => 'compress;q=0.5, gzip;q=1.0')
+    assert_gzip  file_name, response
+
     response  = get(file_name, 'HTTP_ACCEPT_ENCODING' => '')
     assert_not_equal 'gzip', response.headers["Content-Encoding"]
   end
@@ -205,6 +208,12 @@ module StaticTests
     assert_equal "I'm a teapot",           response.headers["X-Custom-Header"]
   end
 
+  def test_ignores_unknown_http_methods
+    app = ActionDispatch::Static.new(DummyApp, @root)
+
+    assert_nothing_raised { Rack::MockRequest.new(app).request("BAD_METHOD", "/foo/bar.html") }
+  end
+
   # Windows doesn't allow \ / : * ? " < > | in filenames
   unless RbConfig::CONFIG['host_os'] =~ /mswin|mingw/
     def test_serves_static_file_with_colon