diff options
author | Eric Wong <e@80x24.org> | 2016-07-26 23:07:41 +0000 |
---|---|---|
committer | Eric Wong <e@80x24.org> | 2016-07-26 23:14:55 +0000 |
commit | d6380043a8953dca63743c947c8027f465d29a5d (patch) | |
tree | 25eab9fbf2373397c94b61edc010927667be06f9 | |
parent | 25a549883b85fb33970b4a1530a365c0c9e51f95 (diff) | |
download | rack-d6380043a8953dca63743c947c8027f465d29a5d.tar.gz |
deflater: remove "deflate" encoding support no-deflate
This will improve cache hit rates and reduce caching overhead at small expense of increased header overhead for some user agents. For reference, Varnish cache supports only gzip as well: https://www.varnish-cache.org/docs/4.1/phk/gzip.html In the past, "deflate" encoding was more likely to trigger user-agent bugs, as noted in the comments removed with this change as well as the Varnish documentation referenced above.
-rw-r--r-- | lib/rack/deflater.rb | 37 | ||||
-rw-r--r-- | test/spec_deflater.rb | 38 |
2 files changed, 25 insertions, 50 deletions
diff --git a/lib/rack/deflater.rb b/lib/rack/deflater.rb index 62a11243..79e39f51 100644 --- a/lib/rack/deflater.rb +++ b/lib/rack/deflater.rb @@ -8,7 +8,6 @@ module Rack # Currently supported compression algorithms: # # * gzip - # * deflate # * identity (no transformation) # # The middleware automatically detects when compression is supported @@ -41,7 +40,7 @@ module Rack request = Request.new(env) - encoding = Utils.select_best_encoding(%w(gzip deflate identity), + encoding = Utils.select_best_encoding(%w(gzip identity), request.accept_encoding) # Set the Vary HTTP header. @@ -57,10 +56,6 @@ module Rack mtime = headers.key?("Last-Modified") ? Time.httpdate(headers["Last-Modified"]) : Time.now [status, headers, GzipStream.new(body, mtime)] - when "deflate" - headers['Content-Encoding'] = "deflate" - headers.delete(CONTENT_LENGTH) - [status, headers, DeflateStream.new(body)] when "identity" [status, headers, body] when nil @@ -101,36 +96,6 @@ module Rack end end - class DeflateStream - DEFLATE_ARGS = [ - Zlib::DEFAULT_COMPRESSION, - # drop the zlib header which causes both Safari and IE to choke - -Zlib::MAX_WBITS, - Zlib::DEF_MEM_LEVEL, - Zlib::DEFAULT_STRATEGY - ] - - def initialize(body) - @body = body - @closed = false - end - - def each - deflator = ::Zlib::Deflate.new(*DEFLATE_ARGS) - @body.each { |part| yield deflator.deflate(part, Zlib::SYNC_FLUSH) } - yield fin = deflator.finish - ensure - deflator.finish unless fin - deflator.close - end - - def close - return if @closed - @closed = true - @body.close if @body.respond_to?(:close) - end - end - private def should_deflate?(env, status, headers, body) diff --git a/test/spec_deflater.rb b/test/spec_deflater.rb index ba7ec5d3..0f27c859 100644 --- a/test/spec_deflater.rb +++ b/test/spec_deflater.rb @@ -81,13 +81,22 @@ describe Rack::Deflater do yield(status, headers, body) if block_given? end + # automatic gzip detection (streamable) + def auto_inflater + Zlib::Inflate.new(32 + Zlib::MAX_WBITS) + end + + def deflate_or_gzip + {'deflate, gzip' => 'gzip'} + end + it 'be able to deflate bodies that respond to each' do app_body = Object.new class << app_body; def each; yield('foo'); yield('bar'); end; end - verify(200, 'foobar', 'deflate', { 'app_body' => app_body }) do |status, headers, body| + verify(200, 'foobar', deflate_or_gzip, { 'app_body' => app_body }) do |status, headers, body| headers.must_equal({ - 'Content-Encoding' => 'deflate', + 'Content-Encoding' => 'gzip', 'Vary' => 'Accept-Encoding', 'Content-Type' => 'text/plain' }) @@ -98,15 +107,15 @@ describe Rack::Deflater do app_body = Object.new class << app_body; def each; yield('foo'); yield('bar'); end; end - verify(200, app_body, 'deflate', { 'skip_body_verify' => true }) do |status, headers, body| + verify(200, app_body, deflate_or_gzip, { 'skip_body_verify' => true }) do |status, headers, body| headers.must_equal({ - 'Content-Encoding' => 'deflate', + 'Content-Encoding' => 'gzip', 'Vary' => 'Accept-Encoding', 'Content-Type' => 'text/plain' }) buf = [] - inflater = Zlib::Inflate.new(-Zlib::MAX_WBITS) + inflater = auto_inflater body.each { |part| buf << inflater.inflate(part) } buf << inflater.finish @@ -118,32 +127,33 @@ describe Rack::Deflater do app_body = Object.new class << app_body; def each; yield('foo'); yield('bar'); end; end opts = { 'skip_body_verify' => true } - verify(200, app_body, 'deflate', opts) do |status, headers, body| + verify(200, app_body, 'gzip', opts) do |status, headers, body| headers.must_equal({ - 'Content-Encoding' => 'deflate', + 'Content-Encoding' => 'gzip', 'Vary' => 'Accept-Encoding', 'Content-Type' => 'text/plain' }) buf = [] - inflater = Zlib::Inflate.new(-Zlib::MAX_WBITS) + inflater = auto_inflater FakeDisconnect = Class.new(RuntimeError) assert_raises(FakeDisconnect, "not Zlib::DataError not raised") do body.each do |part| - buf << inflater.inflate(part) + tmp = inflater.inflate(part) + buf << tmp if tmp.bytesize > 0 raise FakeDisconnect end end - assert_raises(Zlib::BufError) { inflater.finish } + inflater.finish buf.must_equal(%w(foo)) end end # TODO: This is really just a special case of the above... it 'be able to deflate String bodies' do - verify(200, 'Hello world!', 'deflate') do |status, headers, body| + verify(200, 'Hello world!', deflate_or_gzip) do |status, headers, body| headers.must_equal({ - 'Content-Encoding' => 'deflate', + 'Content-Encoding' => 'gzip', 'Vary' => 'Accept-Encoding', 'Content-Type' => 'text/plain' }) @@ -280,7 +290,7 @@ describe Rack::Deflater do 'Content-Encoding' => 'identity' } } - verify(200, 'Hello World!', 'deflate', options) + verify(200, 'Hello World!', deflate_or_gzip, options) end it "deflate if content-type matches :include" do @@ -334,7 +344,7 @@ describe Rack::Deflater do :if => lambda { |env, status, headers, body| true } } } - verify(200, 'Hello World!', 'deflate', options) + verify(200, 'Hello World!', deflate_or_gzip, options) end it "not deflate if :if lambda evaluates to false" do |