Skip to content
Browse files

Merge branch 'CHEF-3140'

  • Loading branch information...
2 parents 536bc2c + 69cc44a commit 305958772e52f453fcf8d27636e7515d4dfbb696 @danielsdeleo danielsdeleo committed May 24, 2012
View
18 chef/lib/chef/provider/remote_file.rb
@@ -39,7 +39,7 @@ def action_create
if current_resource_matches_target_checksum?
Chef::Log.debug("#{@new_resource} checksum matches target checksum (#{@new_resource.checksum}) - not updating")
else
- Chef::REST.new(@new_resource.source, nil, nil).fetch(@new_resource.source) do |raw_file|
+ Chef::REST.new(@new_resource.source, nil, nil, http_client_opts).fetch(@new_resource.source) do |raw_file|
if matches_current_checksum?(raw_file)
Chef::Log.debug "#{@new_resource} target and source checksums are the same - not updating"
else
@@ -99,6 +99,22 @@ def source_file(source, current_checksum, &block)
end
end
+ def http_client_opts
+ opts={}
+ # CHEF-3140
+ # 1. If it's already compressed, trying to compress it more will
+ # probably be counter-productive.
+ # 2. Some servers are misconfigured so that you GET $URL/file.tgz but
+ # they respond with content type of tar and content encoding of gzip,
+ # which tricks Chef::REST into decompressing the response body. In this
+ # case you'd end up with a tar archive (no gzip) named, e.g., foo.tgz,
+ # which is not what you wanted.
+ if @new_resource.path =~ /gz$/ or @new_resource.source =~ /gz$/
+ opts[:disable_gzip] = true
+ end
+ opts
+ end
+
private
def absolute_uri?(source)
View
65 chef/lib/chef/rest.rb
@@ -62,6 +62,8 @@ def initialize(url, client_name=Chef::Config[:node_name], signing_key_filename=C
@sign_on_redirect, @sign_request = true, true
@redirects_followed = 0
@redirect_limit = 10
+ @disable_gzip = false
+ handle_options(options)
end
def signing_key_filename
@@ -267,15 +269,19 @@ def api_request(method, url, headers={}, data=false)
end
def decompress_body(response)
- case response[CONTENT_ENCODING]
- when GZIP
- Chef::Log.debug "decompressing gzip response"
- Zlib::Inflate.new(Zlib::MAX_WBITS + 16).inflate(response.body)
- when DEFLATE
- Chef::Log.debug "decompressing deflate response"
- Zlib::Inflate.inflate(response.body)
- else
+ if gzip_disabled?
response.body
+ else
+ case response[CONTENT_ENCODING]
+ when GZIP
+ Chef::Log.debug "decompressing gzip response"
+ Zlib::Inflate.new(Zlib::MAX_WBITS + 16).inflate(response.body)
+ when DEFLATE
+ Chef::Log.debug "decompressing deflate response"
+ Zlib::Inflate.inflate(response.body)
+ else
+ response.body
+ end
end
end
@@ -403,7 +409,7 @@ def build_headers(method, url, headers={}, json_body=false, raw=false)
headers['Accept'] = "application/json" unless raw
headers["Content-Type"] = 'application/json' if json_body
headers['Content-Length'] = json_body.bytesize.to_s if json_body
- headers[RESTRequest::ACCEPT_ENCODING] = RESTRequest::ENCODING_GZIP_DEFLATE
+ headers[RESTRequest::ACCEPT_ENCODING] = RESTRequest::ENCODING_GZIP_DEFLATE unless gzip_disabled?
headers.merge!(authentication_headers(method, url, json_body)) if sign_requests?
headers.merge!(Chef::Config[:custom_http_headers]) if Chef::Config[:custom_http_headers]
headers
@@ -419,15 +425,19 @@ def stream_to_tempfile(url, response)
# Kudos to _why!
size, total = 0, response.header['Content-Length'].to_i
- inflater = case response[CONTENT_ENCODING]
- when GZIP
- Chef::Log.debug "decompressing gzip stream"
- Zlib::Inflate.new(Zlib::MAX_WBITS + 16)
- when DEFLATE
- Chef::Log.debug "decompressing inflate stream"
- Zlib::Inflate.new
- else
+ inflater = if gzip_disabled?
NoopInflater.new
+ else
+ case response[CONTENT_ENCODING]
+ when GZIP
+ Chef::Log.debug "decompressing gzip stream"
+ Zlib::Inflate.new(Zlib::MAX_WBITS + 16)
+ when DEFLATE
+ Chef::Log.debug "decompressing inflate stream"
+ Zlib::Inflate.new
+ else
+ NoopInflater.new
+ end
end
response.read_body do |chunk|
@@ -441,5 +451,26 @@ def stream_to_tempfile(url, response)
raise
end
+ # gzip is disabled using the disable_gzip => true option in the
+ # constructor. When gzip is disabled, no 'Accept-Encoding' header will be
+ # set, and the response will not be decompressed, no matter what the
+ # Content-Encoding header of the response is. The intended use case for
+ # this is to work around situations where you request +file.tar.gz+, but
+ # the server responds with a content type of tar and a content encoding of
+ # gzip, tricking the client into decompressing the response so you end up
+ # with a tar archive (no gzip) named file.tar.gz
+ def gzip_disabled?
+ @disable_gzip
+ end
+
+ def handle_options(opts)
+ opts.each do |name, value|
+ case name.to_s
+ when 'disable_gzip'
+ @disable_gzip = value
+ end
+ end
+ end
+
end
end
View
32 chef/spec/unit/provider/remote_file_spec.rb
@@ -134,6 +134,38 @@
end
end
+ # CHEF-3140
+ # Some servers return tarballs as content type tar and encoding gzip, which
+ # is totally wrong. When this happens and gzip isn't disabled, Chef::REST
+ # will decompress the file for you, which is not at all what you expected
+ # to happen (you end up with an uncomressed tar archive instead of the
+ # gzipped tar archive you expected). To work around this behavior, we
+ # detect when users are fetching gzipped files and turn off gzip in
+ # Chef::REST.
+
+ context "and the target file is a tarball" do
+ before do
+ @resource.path(File.expand_path(File.join(CHEF_SPEC_DATA, "seattle.tar.gz")))
+ Chef::REST.should_receive(:new).with("http://opscode.com/seattle.txt", nil, nil, :disable_gzip => true).and_return(@rest)
+ end
+
+ it "disables gzip in the http client" do
+ @provider.action_create
+ end
+
+ end
+
+ context "and the source appears to be a tarball" do
+ before do
+ @resource.source("http://example.com/tarball.tgz")
+ Chef::REST.should_receive(:new).with("http://example.com/tarball.tgz", nil, nil, :disable_gzip => true).and_return(@rest)
+ end
+
+ it "disables gzip in the http client" do
+ @provider.action_create
+ end
+ end
+
it "should raise an exception if it's any other kind of retriable response than 304" do
r = Net::HTTPMovedPermanently.new("one", "two", "three")
e = Net::HTTPRetriableError.new("301", r)
View
19 chef/spec/unit/rest_spec.rb
@@ -221,6 +221,25 @@
end
end
+ # CHEF-3140
+ context "when configured to disable compression" do
+ before do
+ @rest = Chef::REST.new(@base_url, nil, nil, :disable_gzip => true)
+ end
+
+ it "does not accept encoding gzip" do
+ @rest.send(:build_headers, :GET, @url, {}).should_not have_key("Accept-Encoding")
+ end
+
+ it "does not decompress a response encoded as gzip" do
+ @http_response.add_field("content-encoding", "gzip")
+ request = Net::HTTP::Get.new(@url.path)
+ Net::HTTP::Get.should_receive(:new).and_return(request)
+ # will raise a Zlib error if incorrect
+ @rest.api_request(:GET, @url, {}).should == "ninja"
+ end
+ end
+
it "should show the JSON error message on an unsuccessful request" do
http_response = Net::HTTPServerError.new("1.1", "500", "drooling from inside of mouth")
http_response.add_field("content-type", "application/json")

0 comments on commit 3059587

Please sign in to comment.
Something went wrong with that request. Please try again.