From 8e31fa3b72892f7421b85b5a79d71a2d726ccddd Mon Sep 17 00:00:00 2001
From: schneems <richard.schneeman@gmail.com>
Date: Thu, 21 Aug 2014 11:52:25 -0500
Subject: 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
---
 .../lib/action_dispatch/middleware/static.rb       | 29 ++++++++++++++--------
 1 file changed, 19 insertions(+), 10 deletions(-)

(limited to 'actionpack/lib')

diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb
index a3e8126ace..0733132277 100644
--- a/actionpack/lib/action_dispatch/middleware/static.rb
+++ b/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
 
-- 
cgit v1.2.3