Address comments on Gzip implementation

- don't mutate PATH_INFO in env, test
- test fallback content type matches Rack::File
- change assertion style
- make HTTP_ACCEPT_ENCODING comparison case insensitive
- return gzip path from method instead of true/false so we don't have to assume later
- don't allocate un-needed hash.

Original comments:

https://github.com/rails/rails/commit/
cfaaacd9763642e91761de54c90669a88d772e5a#commitcomment-7468728

cc @jeremy

actionpack/lib/action_dispatch/middleware/static.rb

@@ -16,8 +16,7 @@ module ActionDispatch
     def initialize(root, cache_control)
       @root          = root.chomp('/')
       @compiled_root = /^#{Regexp.escape(root)}/
-      headers        = {}
-      headers['Cache-Control'] = cache_control if cache_control
+      headers        = cache_control && { 'Cache-Control' => cache_control }
       @file_server = ::Rack::File.new(@root, headers)
     end
 
@@ -37,10 +36,11 @@ module ActionDispatch
     end
 
     def call(env)
-      path             = env['PATH_INFO']
-      gzip_file_exists = gzip_file_exists?(path)
-      if gzip_file_exists && gzip_encoding_accepted?(env)
-        env['PATH_INFO'] = "#{path}.gz"
+      path      = env['PATH_INFO']
+      gzip_path = gzip_file_path(path)
+
+      if gzip_path && gzip_encoding_accepted?(env)
+        env['PATH_INFO']            = gzip_path
         status, headers, body       = @file_server.call(env)
         headers['Content-Encoding'] = 'gzip'
         headers['Content-Type']     = content_type(path)
@@ -48,8 +48,11 @@ module ActionDispatch
         status, headers, body = @file_server.call(env)
       end
 
-      headers['Vary'] = 'Accept-Encoding' if gzip_file_exists
+      headers['Vary'] = 'Accept-Encoding' if gzip_path
+
       return [status, headers, body]
+    ensure
+      env['PATH_INFO'] = path
     end
 
     private
@@ -73,11 +76,17 @@ module ActionDispatch
       end
 
       def gzip_encoding_accepted?(env)
-        env['HTTP_ACCEPT_ENCODING'] =~ /\bgzip\b/
+        env['HTTP_ACCEPT_ENCODING'] =~ /\bgzip\b/i
       end
 
-      def gzip_file_exists?(path)
-        File.exist?(File.join(@root, "#{::Rack::Utils.unescape(path)}.gz"))
+      def gzip_file_path(path)
+        can_gzip_mime = content_type(path) =~ /\A(?:text\/|application\/javascript)/
+        gzip_path     = "#{path}.gz"
+        if can_gzip_mime && File.exist?(File.join(@root, ::Rack::Utils.unescape(gzip_path)))
+          gzip_path
+        else
+          false
+        end
       end
   end
 

actionpack/test/dispatch/static_test.rb

@@ -115,8 +115,30 @@ module StaticTests
     assert_equal 'Accept-Encoding',        response.headers["Vary"]
     assert_equal 'gzip',                   response.headers["Content-Encoding"]
 
+    response  = get(file_name, 'HTTP_ACCEPT_ENCODING' => 'Gzip')
+    assert_gzip  file_name, response
+
+    response  = get(file_name, 'HTTP_ACCEPT_ENCODING' => 'GZIP')
+    assert_gzip  file_name, response
+
     response  = get(file_name, 'HTTP_ACCEPT_ENCODING' => '')
-    refute_equal 'gzip', response.headers["Content-Encoding"]
+    assert_not_equal 'gzip', response.headers["Content-Encoding"]
+  end
+
+  def test_does_not_modify_path_info
+    file_name = "/gzip/application-a71b3024f80aea3181c09774ca17e712.js"
+    env = {'PATH_INFO' => file_name, 'HTTP_ACCEPT_ENCODING' => 'gzip'}
+    @app.call(env)
+    assert_equal file_name, env['PATH_INFO']
+  end
+
+  def test_serves_gzip_with_propper_content_type_fallback
+    file_name = "/gzip/foo.zoo"
+    response  = get(file_name, 'HTTP_ACCEPT_ENCODING' => 'gzip')
+    assert_gzip  file_name, response
+
+    default_response = get(file_name) # no gzip
+    assert_equal default_response.headers['Content-Type'], response.headers['Content-Type']
   end
 
   # Windows doesn't allow \ / : * ? " < > | in filenames
@@ -147,7 +169,7 @@ module StaticTests
     def assert_html(body, response)
       assert_equal body, response.body
       assert_equal "text/html", response.headers["Content-Type"]
-      refute response.headers.key?("Vary")
+      assert_nil response.headers["Vary"]
     end
 
     def get(path, headers = {})

guides/source/4_2_release_notes.md

@@ -268,6 +268,13 @@ Please refer to the [Changelog][action-pack] for detailed changes.
 *   Added an option to disable logging of CSRF failures.
     ([Pull Request](https://github.com/rails/rails/pull/14280))
 
+*   When the Rails server is set to serve static assets, gzip assets will now be
+    served if the client supports it and a pre-generated gzip file (.gz) is on disk.
+    By default the asset pipeline generates `.gz` files for all compressible assets.
+    Serving gzip files minimizes data transfer and speeds up asset requests. Always
+    [use a CDN](http://guides.rubyonrails.org/asset_pipeline.html#cdns) if you are
+    serving assets from your Rails server in production.
+    ([Pull Request](https://github.com/rails/rails/pull/16466))
 
 Action View
 -------------