omit accept-encoding from upload

Author: shubhangi-google

google-cloud-storage/acceptance/storage/bucket_test.rb

@@ -267,6 +267,19 @@
     _(bucket.lifecycle.count).must_equal original_count
   end
 
+  it "omits Accept-Encoding: gzip header for file uploads" do
+    # Verify the service's global headers do not contain the gzip encoding
+    global_headers = storage.service.service.request_options.header || {}
+    _(global_headers["Accept-Encoding"]).wont_equal "gzip"
+
+    # By uploading a JSON file, we simulate the bug condition.
+    # If the header were sent globally, the server might return gzipped JSON metadata, causing a parse error.
+    uploaded = bucket.create_file StringIO.new('{"metadata":"test"}'), "test-upload-metadata.json", content_type: "application/json"
+    
+    _(uploaded.name).must_equal "test-upload-metadata.json"
+    uploaded.delete
+  end
+
   it "does not error when getting a file that does not exist" do
     random_bucket = storage.bucket "#{bucket_name}_does_not_exist"
     _(random_bucket).must_be :nil?

google-cloud-storage/acceptance/storage/file_test.rb

@@ -42,6 +42,19 @@
     bucket.files(versions: true).all { |f| f.delete generation: true rescue nil }
   end
 
+  it "does not send Accept-Encoding: gzip globally during file uploads" do
+    # Verify the service's global headers do not contain the gzip encoding
+    global_headers = storage.service.service.request_options.header || {}
+    _(global_headers["Accept-Encoding"]).wont_equal "gzip"
+
+    # By uploading a JSON file, we simulate the bug condition.
+    # If the header was sent, the server might return gzipped JSON metadata,
+    # which the upload method wouldn't know how to parse, causing an error.
+    uploaded = bucket.create_file StringIO.new('{"metadata":"test"}'), "upload-metadata.json", content_type: "application/json"
+    _(uploaded.name).must_equal "upload-metadata.json"
+    uploaded.delete
+  end
+
   it "should upload and download a file" do
     original = File.new files[:logo][:path]
     uploaded = bucket.create_file original, "CloudLogo.png",

google-cloud-storage/lib/google/cloud/storage/file.rb

@@ -985,7 +985,7 @@ def update generation: nil,
         # @param [Boolean] skip_decompress Optional. If `true`, the data for a
         #   Storage object returning a `Content-Encoding: gzip` response header
         #   will *not* be automatically decompressed by this client library. The
-        #   default is `nil`. Note that all requests by this client library send
+        #   default is `nil`. Note that download requests by this client library send
         #   the `Accept-Encoding: gzip` header, so decompressive transcoding is
         #   not performed in the Storage service. (See [Transcoding of
         #   gzip-compressed files](https://cloud.google.com/storage/docs/transcoding))

google-cloud-storage/lib/google/cloud/storage/service.rb

@@ -64,7 +64,6 @@ def initialize project, credentials, retries: nil,
           @service.request_options.header ||= {}
           @service.request_options.header["x-goog-api-client"] =
             "gl-ruby/#{RUBY_VERSION} gccl/#{Google::Cloud::Storage::VERSION}"
-          @service.request_options.header["Accept-Encoding"] = "gzip"
           @service.request_options.quota_project = quota_project if quota_project
           @service.request_options.max_elapsed_time = max_elapsed_time if max_elapsed_time
           @service.request_options.base_interval = base_interval if base_interval
@@ -586,6 +585,8 @@ def download_file bucket_name, file_path, target_path, generation: nil,
                           key: nil, range: nil, user_project: nil, options: {}
           options = key_options(key).merge(options)
           options = range_header options, range
+          options[:header] ||= {}
+          options[:header]["Accept-Encoding"] = "gzip"
 
           execute do
             service.get_object \

google-cloud-storage/test/google/cloud/storage/bucket_test.rb

@@ -694,6 +694,21 @@
     mock.verify
   end
 
+  it "omits Accept-Encoding: gzip header for file uploads" do
+    new_file_name = random_file_path
+    new_file_contents = StringIO.new "Hello world"
+
+    mock = Minitest::Mock.new
+    mock.expect :insert_object, create_file_gapi(bucket.name, new_file_name) do |b_name, file_obj, **kwargs|
+      headers = (kwargs[:options] && kwargs[:options][:header]) || {}
+      b_name == bucket.name && headers["Accept-Encoding"] != "gzip"
+    end
+    
+    bucket.service.mocked_service = mock
+    bucket.create_file new_file_contents, new_file_name
+    mock.verify
+  end
+
   it "raises when given a file that does not exist" do
     bad_file_path = "/this/file/does/not/exist.ext"
 

google-cloud-storage/test/google/cloud/storage/file_test.rb

@@ -221,7 +221,7 @@ def file.md5
 
       mock = Minitest::Mock.new
       mock.expect :get_object, [tmpfile, download_http_resp],
-        [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: {}
+        [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
       bucket.service.mocked_service = mock
 
@@ -247,7 +247,7 @@ def file.md5
 
       mock = Minitest::Mock.new
       mock.expect :get_object, [tmpfile, download_http_resp(gzip: true)],
-        [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: {}
+        [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
       bucket.service.mocked_service = mock
 
@@ -273,7 +273,7 @@ def file.md5
 
       mock = Minitest::Mock.new
       mock.expect :get_object, [tmpfile, download_http_resp(gzip: true)],
-        [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: {}
+        [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
       bucket.service.mocked_service = mock
 
@@ -297,7 +297,7 @@ def file.md5
 
       mock = Minitest::Mock.new
       mock.expect :get_object, [tmpfile, download_http_resp],
-        [bucket.name, file.name], download_dest: tmpfile.path, generation: generation, user_project: nil, options: {}
+        [bucket.name, file.name], download_dest: tmpfile.path, generation: generation, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
       bucket.service.mocked_service = mock
 
@@ -320,7 +320,7 @@ def file_user_project.md5
 
       mock = Minitest::Mock.new
       mock.expect :get_object, [tmpfile, download_http_resp],
-        [bucket.name, file_user_project.name], download_dest: tmpfile, generation: generation, user_project: "test", options: {}
+        [bucket.name, file_user_project.name], download_dest: tmpfile, generation: generation, user_project: "test", options: { header: { "Accept-Encoding" => "gzip" } }
 
       bucket.service.mocked_service = mock
 
@@ -341,7 +341,7 @@ def file.md5
     downloadio = StringIO.new
     mock = Minitest::Mock.new
     mock.expect :get_object, [StringIO.new(data), download_http_resp],
-      [bucket.name, file.name], download_dest: downloadio, generation: generation, user_project: nil, options: {}
+      [bucket.name, file.name], download_dest: downloadio, generation: generation, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
     bucket.service.mocked_service = mock
 
@@ -362,7 +362,7 @@ def file.md5
 
     mock = Minitest::Mock.new
     mock.expect :get_object, [StringIO.new(gzipped_data), download_http_resp(gzip: true)],
-      [bucket.name, file.name], download_dest: downloadio, generation: generation, user_project: nil, options: {}
+      [bucket.name, file.name], download_dest: downloadio, generation: generation, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
     bucket.service.mocked_service = mock
 
@@ -383,7 +383,7 @@ def file.md5
 
     mock = Minitest::Mock.new
     mock.expect :get_object, [StringIO.new(gzipped_data), download_http_resp(gzip: true)],
-      [bucket.name, file.name], download_dest: downloadio, generation: generation, user_project: nil, options: {}
+      [bucket.name, file.name], download_dest: downloadio, generation: generation, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
     bucket.service.mocked_service = mock
 
@@ -404,7 +404,7 @@ def file.md5
 
     mock = Minitest::Mock.new
     mock.expect :get_object, [StringIO.new("yay!"), download_http_resp],
-      [bucket.name, file.name], download_dest: downloadio, generation: generation, user_project: nil, options: {}
+      [bucket.name, file.name], download_dest: downloadio, generation: generation, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
     bucket.service.mocked_service = mock
 
@@ -427,7 +427,7 @@ def file.md5
 
       mock = Minitest::Mock.new
       mock.expect :get_object, [nil, download_http_resp], # using encryption keys seems to return nil
-        [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: key_options
+        [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: { header: key_headers.merge("Accept-Encoding" => "gzip") }
 
       bucket.service.mocked_service = mock
 
@@ -442,7 +442,7 @@ def file.md5
     Tempfile.open "google-cloud" do |tmpfile|
       mock = Minitest::Mock.new
       mock.expect :get_object, [tmpfile, download_http_resp],
-        [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: { header: { 'Range' => 'bytes=3-6' }}
+        [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: { header: { "Range" => "bytes=3-6", "Accept-Encoding" => "gzip" } }
 
       bucket.service.mocked_service = mock
 
@@ -457,7 +457,7 @@ def file.md5
     Tempfile.open "google-cloud" do |tmpfile|
       mock = Minitest::Mock.new
       mock.expect :get_object, [tmpfile, download_http_resp],
-        [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: { header: { 'Range' => 'bytes=-6' }}
+        [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: { header: { "Range" => "bytes=-6", "Accept-Encoding" => "gzip" } }
 
       bucket.service.mocked_service = mock
 
@@ -477,7 +477,7 @@ def file.crc32c; "crc32c="; end
       Tempfile.open "google-cloud" do |tmpfile|
         mock = Minitest::Mock.new
         mock.expect :get_object, [tmpfile, download_http_resp],
-          [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: {}
+          [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
         bucket.service.mocked_service = mock
 
@@ -504,7 +504,7 @@ def file.crc32c; "crc32c="; end
       Tempfile.open "google-cloud" do |tmpfile|
         mock = Minitest::Mock.new
         mock.expect :get_object, [tmpfile, download_http_resp],
-          [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: {}
+          [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
         bucket.service.mocked_service = mock
 
@@ -531,7 +531,7 @@ def file.crc32c; "crc32c="; end
       Tempfile.open "google-cloud" do |tmpfile|
         mock = Minitest::Mock.new
         mock.expect :get_object, [tmpfile, download_http_resp],
-          [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: {}
+          [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
         bucket.service.mocked_service = mock
 
@@ -558,7 +558,7 @@ def file.crc32c; "crc32c="; end
 
       mock = Minitest::Mock.new
       mock.expect :get_object, [StringIO.new(data), download_http_resp],
-        [bucket.name, file.name], download_dest: path, generation: 1234567890, user_project: nil, options: {}
+        [bucket.name, file.name], download_dest: path, generation: 1234567890, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
       bucket.service.mocked_service = mock
 
@@ -577,7 +577,7 @@ def file.crc32c; "crc32c="; end
       Tempfile.open "google-cloud" do |tmpfile|
         mock = Minitest::Mock.new
         mock.expect :get_object, [tmpfile, download_http_resp],
-          [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: {}
+          [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
         bucket.service.mocked_service = mock
 
@@ -608,7 +608,7 @@ def file.crc32c; "crc32c="; end
       Tempfile.open "google-cloud" do |tmpfile|
         mock = Minitest::Mock.new
         mock.expect :get_object, [tmpfile, download_http_resp],
-          [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: {}
+          [bucket.name, file.name], download_dest: tmpfile, generation: generation, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
         bucket.service.mocked_service = mock
 
@@ -633,7 +633,7 @@ def file.crc32c; "crc32c="; end
       Tempfile.open "google-cloud" do |tmpfile|
         mock = Minitest::Mock.new
         mock.expect :get_object, [tmpfile, download_http_resp],
-          [bucket.name, file.name], download_dest: tmpfile.path, generation: generation, user_project: nil, options: {}
+          [bucket.name, file.name], download_dest: tmpfile.path, generation: generation, user_project: nil, options: { header: { "Accept-Encoding" => "gzip" } }
 
         bucket.service.mocked_service = mock