summary refs log tree commit
diff options
context:
space:
mode:
authorJeremy Evans <code@jeremyevans.net>2022-05-06 12:35:31 -0700
committerJeremy Evans <code@jeremyevans.net>2022-05-07 10:27:29 -0700
commita17af027b7b1b785565d2dae73ec8adecd8bd208 (patch)
treeeac87b845b80be2b8a32c3406258e77c04e89674
parent86fb70643042ed14f92d73ddbb93e36f920904ad (diff)
downloadrack-a17af027b7b1b785565d2dae73ec8adecd8bd208.tar.gz
Add 100% line/branch coverage to rack/deflater.rb
Fix handling of accept-encoding in vary header, since Array#include?
when called with an array doesn't check if any element in the array
matches.

Coverage after this commit:

3305 relevant lines, 3257 lines covered and 48 lines missed. ( 98.55% )
1133 total branches, 1036 branches covered and 97 branches missed. ( 91.44% )
-rw-r--r--lib/rack/deflater.rb7
-rw-r--r--test/spec_deflater.rb69
2 files changed, 73 insertions, 3 deletions
diff --git a/lib/rack/deflater.rb b/lib/rack/deflater.rb
index f316688a..7752d17e 100644
--- a/lib/rack/deflater.rb
+++ b/lib/rack/deflater.rb
@@ -57,7 +57,7 @@ module Rack
 
       # Set the Vary HTTP header.
       vary = headers["vary"].to_s.split(",").map(&:strip)
-      unless vary.include?("*") || vary.include?(/accept-encoding/i)
+      unless vary.include?("*") || vary.any?{|v| v.downcase == 'accept-encoding'}
         headers["vary"] = vary.push("Accept-Encoding").join(",")
       end
 
@@ -70,7 +70,8 @@ module Rack
         [status, headers, GzipStream.new(body, mtime, @sync)]
       when "identity"
         [status, headers, body]
-      when nil
+      else # when nil
+        # Only possible encoding values here are 'gzip', 'identity', and nil
         message = "An acceptable encoding for the requested resource #{request.fullpath} could not be found."
         bp = Rack::BodyProxy.new([message]) { body.close if body.respond_to?(:close) }
         [406, { CONTENT_TYPE => "text/plain", CONTENT_LENGTH => message.length.to_s }, bp]
@@ -99,7 +100,7 @@ module Rack
         gzip = ::Zlib::GzipWriter.new(self)
         gzip.mtime = @mtime if @mtime
         # @body.each is equivalent to @body.gets (slow)
-        if @body.is_a? ::File
+        if @body.is_a? ::File # XXX: Should probably be ::IO
           while part = @body.read(BUFFER_LENGTH)
             gzip.write(part)
             gzip.flush if @sync
diff --git a/test/spec_deflater.rb b/test/spec_deflater.rb
index 71436685..ed22c1fb 100644
--- a/test/spec_deflater.rb
+++ b/test/spec_deflater.rb
@@ -92,6 +92,7 @@ describe Rack::Deflater do
 
     # yield full response verification
     yield(status, headers, body) if block_given?
+    body.close if body.respond_to?(:close)
   end
 
   # automatic gzip detection (streamable)
@@ -116,6 +117,18 @@ describe Rack::Deflater do
     end
   end
 
+  it 'should not update vary response header if it includes * or accept-encoding' do
+    verify(200, 'foobar', deflate_or_gzip, 'response_headers' => { 'vary' => 'Accept-Encoding' } ) do |status, headers, body|
+      headers['vary'].must_equal 'Accept-Encoding'
+    end
+    verify(200, 'foobar', deflate_or_gzip, 'response_headers' => { 'vary' => '*' } ) do |status, headers, body|
+      headers['vary'].must_equal '*'
+    end
+    verify(200, 'foobar', deflate_or_gzip, 'response_headers' => { 'vary' => 'Do-Not-Accept-Encoding' } ) do |status, headers, body|
+      headers['vary'].must_equal 'Do-Not-Accept-Encoding,Accept-Encoding'
+    end
+  end
+
   it 'be able to deflate bodies that respond to each and contain empty chunks' do
     app_body = Object.new
     class << app_body; def each; yield('foo'); yield(''); yield('bar'); end; end
@@ -199,6 +212,16 @@ describe Rack::Deflater do
     end
   end
 
+  it 'be able to gzip files' do
+    verify(200, File.binread(__FILE__), 'gzip', { 'app_body' => File.open(__FILE__)}) do |status, headers, body|
+      headers.must_equal({
+        'content-encoding' => 'gzip',
+        'vary' => 'Accept-Encoding',
+        'content-type' => 'text/plain'
+      })
+    end
+  end
+
   it 'flush gzipped chunks to the client as they become ready' do
     app_body = Object.new
     class << app_body; def each; yield('foo'); yield('bar'); end; end
@@ -252,6 +275,17 @@ describe Rack::Deflater do
         'PATH_INFO' => '/foo/bar'
       }
     }
+    
+    app_body3 = [app_body]
+    closed = false
+    app_body3.define_singleton_method(:close){closed = true}
+    options3 = {
+      'app_status' => 200,
+      'app_body' => app_body3,
+      'request_headers' => {
+        'PATH_INFO' => '/'
+      }
+    }
 
     verify(406, not_found_body1, 'identity;q=0', options1) do |status, headers, body|
       headers.must_equal({
@@ -266,6 +300,14 @@ describe Rack::Deflater do
         'content-length' => not_found_body2.length.to_s
       })
     end
+
+    verify(406, not_found_body1, 'identity;q=0', options3) do |status, headers, body|
+      headers.must_equal({
+        'content-type' => 'text/plain',
+        'content-length' => not_found_body1.length.to_s
+      })
+    end
+    closed.must_equal true
   end
 
   it 'handle gzip response with last-modified header' do
@@ -441,4 +483,31 @@ describe Rack::Deflater do
       raw_bytes.must_be(:<, expect.bytesize)
     end
   end
+
+  it 'will honor sync: false to avoid unnecessary flushing when deflating files' do
+    content = File.binread(__FILE__)
+    options = {
+      'deflater_options' => { sync: false },
+      'app_body' => File.open(__FILE__),
+      'skip_body_verify' => true,
+    }
+    verify(200, content, deflate_or_gzip, options) do |status, headers, body|
+      headers.must_equal({
+        'content-encoding' => 'gzip',
+        'vary' => 'Accept-Encoding',
+        'content-type' => 'text/plain'
+      })
+
+      buf = ''.dup
+      raw_bytes = 0
+      inflater = auto_inflater
+      body.each do |part|
+        raw_bytes += part.bytesize
+        buf << inflater.inflate(part)
+      end
+      buf << inflater.finish
+      buf.must_equal content
+      raw_bytes.must_be(:<, content.bytesize)
+    end
+  end
 end