From 197bb96c94ac90cbe8df7ab569bf8a2f2f501945 Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Tue, 2 May 2017 13:46:43 -0400 Subject: [PATCH] Use built-in Net::HTTP support for compression. The Net::HTTP in ruby 2.0+ supports gzip and deflate decoding on its own. Remove Accept-Encoding from the default headers set, and instead let Net::HTTP take care of it. Note: this could break compatibility for users who are setting Accept-Encoding headers themselves or who expect the body to still be compressed with `:raw_response => true`. --- lib/restclient/request.rb | 30 +++++++---------------- spec/integration/httpbin_spec.rb | 16 +++++++++++++ spec/integration/integration_spec.rb | 7 ------ spec/unit/request2_spec.rb | 12 +++++----- spec/unit/request_spec.rb | 36 +++++----------------------- 5 files changed, 36 insertions(+), 65 deletions(-) diff --git a/lib/restclient/request.rb b/lib/restclient/request.rb index 763c68c..6dbf980 100644 --- a/lib/restclient/request.rb +++ b/lib/restclient/request.rb @@ -505,24 +505,6 @@ module RestClient cert_store end - def self.decode content_encoding, body - if (!body) || body.empty? - body - elsif content_encoding == 'gzip' - Zlib::GzipReader.new(StringIO.new(body)).read - elsif content_encoding == 'deflate' - begin - Zlib::Inflate.new.inflate body - rescue Zlib::DataError - # No luck with Zlib decompression. Let's try with raw deflate, - # like some broken web servers do. - Zlib::Inflate.new(-Zlib::MAX_WBITS).inflate body - end - else - body - end - end - def redacted_uri if uri.password sanitized_uri = uri.dup @@ -578,10 +560,13 @@ module RestClient end end + # Default headers set by RestClient. In addition to these headers, servers + # will receive headers set by Net::HTTP, such as Accept-Encoding and Host. + # + # @return [Hash] def default_headers { :accept => '*/*', - :accept_encoding => 'gzip, deflate', :user_agent => RestClient::Platform.default_user_agent, } end @@ -814,11 +799,12 @@ module RestClient # @param start_time [Time] Time of request start def process_result(res, start_time, tempfile=nil, &block) if @raw_response - # We don't decode raw requests + unless tempfile + raise ArgumentError.new('tempfile is required') + end response = RawResponse.new(tempfile, res, self, start_time) else - decoded = Request.decode(res['content-encoding'], res.body) - response = Response.create(decoded, res, self, start_time) + response = Response.create(res.body, res, self, start_time) end response.log_response diff --git a/spec/integration/httpbin_spec.rb b/spec/integration/httpbin_spec.rb index 8c83b37..64a0ea2 100644 --- a/spec/integration/httpbin_spec.rb +++ b/spec/integration/httpbin_spec.rb @@ -83,5 +83,21 @@ describe RestClient::Request do expect(ex.http_code).to eq 401 } end + + it 'handles gzipped/deflated responses' do + [['gzip', 'gzipped'], ['deflate', 'deflated']].each do |encoding, var| + raw = execute_httpbin(encoding, method: :get) + + begin + data = JSON.parse(raw) + rescue + puts "Failed to parse: " + raw.inspect + raise + end + + expect(data['method']).to eq 'GET' + expect(data.fetch(var)).to be true + end + end end end diff --git a/spec/integration/integration_spec.rb b/spec/integration/integration_spec.rb index 963bad0..ed86279 100644 --- a/spec/integration/integration_spec.rb +++ b/spec/integration/integration_spec.rb @@ -12,13 +12,6 @@ describe RestClient do expect(response.body).to eq body end - it "a simple request with gzipped content" do - stub_request(:get, "www.example.com").with(:headers => { 'Accept-Encoding' => 'gzip, deflate' }).to_return(:body => "\037\213\b\b\006'\252H\000\003t\000\313T\317UH\257\312,HM\341\002\000G\242(\r\v\000\000\000", :status => 200, :headers => { 'Content-Encoding' => 'gzip' } ) - response = RestClient.get "www.example.com" - expect(response.code).to eq 200 - expect(response.body).to eq "i'm gziped\n" - end - it "a 404" do body = "Ho hai ! I'm not here !" stub_request(:get, "www.example.com").to_return(:body => body, :status => 404) diff --git a/spec/unit/request2_spec.rb b/spec/unit/request2_spec.rb index 71e5d6e..235ce55 100644 --- a/spec/unit/request2_spec.rb +++ b/spec/unit/request2_spec.rb @@ -4,20 +4,20 @@ describe RestClient::Request do context 'params for GET requests' do it "manage params for get requests" do - stub_request(:get, 'http://some/resource?a=b&c=d').with(:headers => {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip, deflate', 'Foo'=>'bar'}).to_return(:body => 'foo', :status => 200) + stub_request(:get, 'http://some/resource?a=b&c=d').with(:headers => {'Accept'=>'*/*', 'Foo'=>'bar'}).to_return(:body => 'foo', :status => 200) expect(RestClient::Request.execute(:url => 'http://some/resource', :method => :get, :headers => {:foo => :bar, :params => {:a => :b, 'c' => 'd'}}).body).to eq 'foo' - stub_request(:get, 'http://some/resource').with(:headers => {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip, deflate', 'Foo'=>'bar'}).to_return(:body => 'foo', :status => 200) + stub_request(:get, 'http://some/resource').with(:headers => {'Accept'=>'*/*', 'Foo'=>'bar'}).to_return(:body => 'foo', :status => 200) expect(RestClient::Request.execute(:url => 'http://some/resource', :method => :get, :headers => {:foo => :bar, :params => :a}).body).to eq 'foo' end it 'adds GET params when params are present in URL' do - stub_request(:get, 'http://some/resource?a=b&c=d').with(:headers => {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip, deflate', 'Foo'=>'bar'}).to_return(:body => 'foo', :status => 200) + stub_request(:get, 'http://some/resource?a=b&c=d').with(:headers => {'Accept'=>'*/*', 'Foo'=>'bar'}).to_return(:body => 'foo', :status => 200) expect(RestClient::Request.execute(:url => 'http://some/resource?a=b', :method => :get, :headers => {:foo => :bar, :params => {:c => 'd'}}).body).to eq 'foo' end it 'encodes nested GET params' do - stub_request(:get, 'http://some/resource?a[foo][]=1&a[foo][]=2&a[bar]&b=foo+bar&math=2+%2B+2+%3D%3D+4').with(:headers => {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip, deflate'}).to_return(:body => 'foo', :status => 200) + stub_request(:get, 'http://some/resource?a[foo][]=1&a[foo][]=2&a[bar]&b=foo+bar&math=2+%2B+2+%3D%3D+4').with(:headers => {'Accept'=>'*/*',}).to_return(:body => 'foo', :status => 200) expect(RestClient::Request.execute(url: 'http://some/resource', method: :get, headers: { params: { a: { @@ -37,7 +37,7 @@ describe RestClient::Request do block = proc do |http_response| response_value = http_response.body end - stub_request(:get, 'http://some/resource?a=b&c=d').with(:headers => {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip, deflate', 'Foo'=>'bar'}).to_return(:body => 'foo', :status => 200) + stub_request(:get, 'http://some/resource?a=b&c=d').with(:headers => {'Accept'=>'*/*', 'Foo'=>'bar'}).to_return(:body => 'foo', :status => 200) RestClient::Request.execute(:url => 'http://some/resource', :method => :get, :headers => {:foo => :bar, :params => {:a => :b, 'c' => 'd'}}, :block_response => block) expect(response_value).to eq "foo" end @@ -45,7 +45,7 @@ describe RestClient::Request do it 'closes payload if not nil' do test_file = File.new(File.join( File.dirname(File.expand_path(__FILE__)), 'master_shake.jpg')) - stub_request(:post, 'http://some/resource').with(:headers => {'Accept'=>'*/*', 'Accept-Encoding'=>'gzip, deflate'}).to_return(:body => 'foo', :status => 200) + stub_request(:post, 'http://some/resource').with(:headers => {'Accept'=>'*/*'}).to_return(:body => 'foo', :status => 200) RestClient::Request.execute(:url => 'http://some/resource', :method => :post, :payload => {:file => test_file}) expect(test_file.closed?).to be true diff --git a/spec/unit/request_spec.rb b/spec/unit/request_spec.rb index e28740e..47e5299 100644 --- a/spec/unit/request_spec.rb +++ b/spec/unit/request_spec.rb @@ -27,30 +27,6 @@ describe RestClient::Request, :include_helpers do expect(@request.default_headers[:accept]).to eq '*/*' end - describe "compression" do - - it "decodes an uncompressed result body by passing it straight through" do - expect(RestClient::Request.decode(nil, 'xyz')).to eq 'xyz' - end - - it "doesn't fail for nil bodies" do - expect(RestClient::Request.decode('gzip', nil)).to be_nil - end - - - it "decodes a gzip body" do - expect(RestClient::Request.decode('gzip', "\037\213\b\b\006'\252H\000\003t\000\313T\317UH\257\312,HM\341\002\000G\242(\r\v\000\000\000")).to eq "i'm gziped\n" - end - - it "ingores gzip for empty bodies" do - expect(RestClient::Request.decode('gzip', '')).to be_empty - end - - it "decodes a deflated body" do - expect(RestClient::Request.decode('deflate', "x\234+\316\317MUHIM\313I,IMQ(I\255(\001\000A\223\006\363")).to eq "some deflated text" - end - end - it "processes a successful result" do res = res_double allow(res).to receive(:code).and_return("200") @@ -315,7 +291,7 @@ describe RestClient::Request, :include_helpers do describe "user headers" do it "merges user headers with the default headers" do - expect(@request).to receive(:default_headers).and_return({ :accept => '*/*', :accept_encoding => 'gzip, deflate' }) + expect(@request).to receive(:default_headers).and_return({:accept => '*/*'}) headers = @request.make_headers("Accept" => "application/json", :accept_encoding => 'gzip') expect(headers).to have_key "Accept-Encoding" expect(headers["Accept-Encoding"]).to eq "gzip" @@ -644,25 +620,25 @@ describe RestClient::Request, :include_helpers do it "logs a get request" do log = RestClient.log = [] RestClient::Request.new(:method => :get, :url => 'http://url', :headers => {:user_agent => 'rest-client'}).log_request - expect(log[0]).to eq %Q{RestClient.get "http://url", "Accept"=>"*/*", "Accept-Encoding"=>"gzip, deflate", "User-Agent"=>"rest-client"\n} + expect(log[0]).to eq %Q{RestClient.get "http://url", "Accept"=>"*/*", "User-Agent"=>"rest-client"\n} end it "logs a post request with a small payload" do log = RestClient.log = [] RestClient::Request.new(:method => :post, :url => 'http://url', :payload => 'foo', :headers => {:user_agent => 'rest-client'}).log_request - expect(log[0]).to eq %Q{RestClient.post "http://url", "foo", "Accept"=>"*/*", "Accept-Encoding"=>"gzip, deflate", "Content-Length"=>"3", "User-Agent"=>"rest-client"\n} + expect(log[0]).to eq %Q{RestClient.post "http://url", "foo", "Accept"=>"*/*", "Content-Length"=>"3", "User-Agent"=>"rest-client"\n} end it "logs a post request with a large payload" do log = RestClient.log = [] RestClient::Request.new(:method => :post, :url => 'http://url', :payload => ('x' * 1000), :headers => {:user_agent => 'rest-client'}).log_request - expect(log[0]).to eq %Q{RestClient.post "http://url", 1000 byte(s) length, "Accept"=>"*/*", "Accept-Encoding"=>"gzip, deflate", "Content-Length"=>"1000", "User-Agent"=>"rest-client"\n} + expect(log[0]).to eq %Q{RestClient.post "http://url", 1000 byte(s) length, "Accept"=>"*/*", "Content-Length"=>"1000", "User-Agent"=>"rest-client"\n} end it "logs input headers as a hash" do log = RestClient.log = [] RestClient::Request.new(:method => :get, :url => 'http://url', :headers => { :accept => 'text/plain', :user_agent => 'rest-client' }).log_request - expect(log[0]).to eq %Q{RestClient.get "http://url", "Accept"=>"text/plain", "Accept-Encoding"=>"gzip, deflate", "User-Agent"=>"rest-client"\n} + expect(log[0]).to eq %Q{RestClient.get "http://url", "Accept"=>"text/plain", "User-Agent"=>"rest-client"\n} end it "logs a response including the status code, content type, and result body size in bytes" do @@ -695,7 +671,7 @@ describe RestClient::Request, :include_helpers do it 'does not log request password' do log = RestClient.log = [] RestClient::Request.new(:method => :get, :url => 'http://user:password@url', :headers => {:user_agent => 'rest-client'}).log_request - expect(log[0]).to eq %Q{RestClient.get "http://user:REDACTED@url", "Accept"=>"*/*", "Accept-Encoding"=>"gzip, deflate", "User-Agent"=>"rest-client"\n} + expect(log[0]).to eq %Q{RestClient.get "http://user:REDACTED@url", "Accept"=>"*/*", "User-Agent"=>"rest-client"\n} end it 'logs to a passed logger, if provided' do