From 30c11286eeed54778eff491c54eb68a322ea6f73 Mon Sep 17 00:00:00 2001 From: Shubhangi Singh Date: Sun, 7 Sep 2025 21:45:35 +0000 Subject: [PATCH 1/6] handling missing upload_id in resumable upload --- .../lib/google/apis/core/storage_upload.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/google-apis-core/lib/google/apis/core/storage_upload.rb b/google-apis-core/lib/google/apis/core/storage_upload.rb index da31878d3a3..e9f6d7720de 100644 --- a/google-apis-core/lib/google/apis/core/storage_upload.rb +++ b/google-apis-core/lib/google/apis/core/storage_upload.rb @@ -149,7 +149,9 @@ def initiate_resumable_upload(client) # Reinitiating resumable upload def reinitiate_resumable_upload(client) logger.debug { sprintf('Restarting resumable upload command to %s', url) } - check_resumable_upload client + unless check_resumable_upload client + return false + end upload_io.pos = @offset end @@ -224,6 +226,10 @@ def check_resumable_upload(client) # Initiating call response = client.put(@upload_url, "", request_header) handle_resumable_upload_http_response_codes(response) + if response.status.to_i == 404 + logger.debug { sprintf("Failed to fetch upload session. Response: #{response.status.to_i} - #{response.body}") } + false + end end # Cancel resumable upload @@ -235,11 +241,12 @@ def cancel_resumable_upload(client) response = client.delete(@upload_url, nil, request_header) handle_resumable_upload_http_response_codes(response) - if !@upload_incomplete && (400..499).include?(response.status.to_i) + if !@upload_incomplete && (400..499).include?(response.status.to_i) && response.status.to_i != 404 @close_io_on_finish = true true # method returns true if upload is successfully cancelled else logger.debug { sprintf("Failed to cancel upload session. Response: #{response.status.to_i} - #{response.body}") } + false end end From b7de7d28434f528e203ddd05ab376df4bcce2c9b Mon Sep 17 00:00:00 2001 From: Shubhangi Singh Date: Mon, 8 Sep 2025 11:59:48 +0000 Subject: [PATCH 2/6] adding test cases --- .../lib/google/apis/core/storage_upload.rb | 5 +-- .../spec/google/apis/core/service_spec.rb | 36 +++++++++++++++++++ .../google/apis/core/storage_upload_spec.rb | 17 +++++++++ 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/google-apis-core/lib/google/apis/core/storage_upload.rb b/google-apis-core/lib/google/apis/core/storage_upload.rb index e9f6d7720de..7be62b0034c 100644 --- a/google-apis-core/lib/google/apis/core/storage_upload.rb +++ b/google-apis-core/lib/google/apis/core/storage_upload.rb @@ -149,9 +149,7 @@ def initiate_resumable_upload(client) # Reinitiating resumable upload def reinitiate_resumable_upload(client) logger.debug { sprintf('Restarting resumable upload command to %s', url) } - unless check_resumable_upload client - return false - end + return false unless check_resumable_upload client upload_io.pos = @offset end @@ -248,7 +246,6 @@ def cancel_resumable_upload(client) logger.debug { sprintf("Failed to cancel upload session. Response: #{response.status.to_i} - #{response.body}") } false end - end def handle_resumable_upload_http_response_codes(response) diff --git a/google-apis-core/spec/google/apis/core/service_spec.rb b/google-apis-core/spec/google/apis/core/service_spec.rb index a4403f6cc3f..f96e0bd5fdd 100644 --- a/google-apis-core/spec/google/apis/core/service_spec.rb +++ b/google-apis-core/spec/google/apis/core/service_spec.rb @@ -287,6 +287,24 @@ expect(a_request(:put, upload_url)).to have_been_made end end + context 'Should return false if wrong upload id is provided' do + let(:upload_id) { 'wrong_id' } + let(:command) do + service.send( + :restart_resumable_upload, + bucket_name, file, upload_id, + options: { upload_chunk_size: 11} + ) + end + before(:example) do + stub_request(:put, upload_url) + .to_return(status: 404) + end + it 'should return false' do + expect(command).to be_falsey + end + end + end context 'delete resumable upload with upload_id' do @@ -312,6 +330,24 @@ expect(a_request(:delete, upload_url)).to have_been_made expect(command).to be_truthy end + + context 'Should return false if wrong upload id is provided' do + let(:upload_id) { 'wrong_id' } + let(:command) do + service.send( + :delete_resumable_upload, + bucket_name, upload_id, + options: { upload_chunk_size: 11} + ) + end + before(:example) do + stub_request(:delete, upload_url) + .to_return(status: 404) + end + it 'should return false' do + expect(command).to be_falsey + end + end end context 'with batch' do diff --git a/google-apis-core/spec/google/apis/core/storage_upload_spec.rb b/google-apis-core/spec/google/apis/core/storage_upload_spec.rb index 7435f9cafe8..4b4117c70c2 100644 --- a/google-apis-core/spec/google/apis/core/storage_upload_spec.rb +++ b/google-apis-core/spec/google/apis/core/storage_upload_spec.rb @@ -161,6 +161,14 @@ expect(a_request(:put, upload_url) .with(body: 'Hello world')).to have_been_made end + + it 'should return false if wrong upload id is provided' do + stub_request(:put, upload_url) + .to_return(status: 404) + command.options.upload_chunk_size = 11 + command.upload_id = "wrong_upload_id" + expect(command.execute(client)).to be_falsy + end end context('should not restart resumable upload if upload is completed') do @@ -235,6 +243,15 @@ expect(a_request(:put, upload_url) .with(body: 'Hello world')).to have_not_been_made end + + it 'should return false if wrong upload id is provided' do + stub_request(:delete, upload_url) + .to_return(status: 404) + command.options.upload_chunk_size = 11 + command.upload_id = "wrong_upload_id" + command.delete_upload = true + expect(command.execute(client)).to be_falsy + end end context('with chunking disabled') do From 392b0ee29df0a03596c0a2693c0e5e8f5ff8b517 Mon Sep 17 00:00:00 2001 From: Shubhangi Singh Date: Mon, 8 Sep 2025 13:34:01 +0000 Subject: [PATCH 3/6] update tests --- .../google/apis/core/storage_upload_spec.rb | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/google-apis-core/spec/google/apis/core/storage_upload_spec.rb b/google-apis-core/spec/google/apis/core/storage_upload_spec.rb index 4b4117c70c2..d528a83a0ef 100644 --- a/google-apis-core/spec/google/apis/core/storage_upload_spec.rb +++ b/google-apis-core/spec/google/apis/core/storage_upload_spec.rb @@ -15,7 +15,7 @@ require 'spec_helper' require 'google/apis/core/storage_upload' require 'google/apis/core/json_representation' - +require 'pry' RSpec.describe Google::Apis::Core::StorageUploadCommand do include TestHelpers include_context 'HTTP client' @@ -162,11 +162,23 @@ .with(body: 'Hello world')).to have_been_made end + end + + context('restart resumable upload with wrong upload_id') do + let(:file) { StringIO.new('Hello world' * 3) } + let(:upload_id) { 'wrong_upload_id' } + let(:upload_url) { "https://www.googleapis.com/zoo/animals?uploadType=resumable&upload_id=#{upload_id}" } + + before(:example) do + stub_request(:put, upload_url) + .to_return(status: 404) + end + it 'should return false if wrong upload id is provided' do stub_request(:put, upload_url) .to_return(status: 404) command.options.upload_chunk_size = 11 - command.upload_id = "wrong_upload_id" + command.upload_id = upload_id expect(command.execute(client)).to be_falsy end end @@ -244,11 +256,21 @@ .with(body: 'Hello world')).to have_not_been_made end - it 'should return false if wrong upload id is provided' do + end + + context('delete resumable upload with upload_id') do + let(:file) { StringIO.new('Hello world' * 3) } + let(:upload_id) { 'wrong_upload_id' } + let(:upload_url) { "https://www.googleapis.com/zoo/animals?uploadType=resumable&upload_id=#{upload_id}" } + + before(:example) do stub_request(:delete, upload_url) .to_return(status: 404) + end + + it 'should return false if wrong upload id is provided' do command.options.upload_chunk_size = 11 - command.upload_id = "wrong_upload_id" + command.upload_id = upload_id command.delete_upload = true expect(command.execute(client)).to be_falsy end From 6d8bd82731b42de52f2e2753300d903bda49c72f Mon Sep 17 00:00:00 2001 From: Shubhangi Singh Date: Mon, 8 Sep 2025 13:39:16 +0000 Subject: [PATCH 4/6] removing pry --- google-apis-core/spec/google/apis/core/storage_upload_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-apis-core/spec/google/apis/core/storage_upload_spec.rb b/google-apis-core/spec/google/apis/core/storage_upload_spec.rb index d528a83a0ef..607089569df 100644 --- a/google-apis-core/spec/google/apis/core/storage_upload_spec.rb +++ b/google-apis-core/spec/google/apis/core/storage_upload_spec.rb @@ -15,7 +15,7 @@ require 'spec_helper' require 'google/apis/core/storage_upload' require 'google/apis/core/json_representation' -require 'pry' + RSpec.describe Google::Apis::Core::StorageUploadCommand do include TestHelpers include_context 'HTTP client' From 5eda2873e0986b960d1e312d46f8471ec706e594 Mon Sep 17 00:00:00 2001 From: Shubhangi Singh Date: Tue, 28 Oct 2025 10:56:51 +0000 Subject: [PATCH 5/6] fix --- .../lib/google/apis/core/storage_upload.rb | 12 ++++++++---- .../spec/google/apis/core/service_spec.rb | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/google-apis-core/lib/google/apis/core/storage_upload.rb b/google-apis-core/lib/google/apis/core/storage_upload.rb index 7be62b0034c..dd11ba98948 100644 --- a/google-apis-core/lib/google/apis/core/storage_upload.rb +++ b/google-apis-core/lib/google/apis/core/storage_upload.rb @@ -239,12 +239,16 @@ def cancel_resumable_upload(client) response = client.delete(@upload_url, nil, request_header) handle_resumable_upload_http_response_codes(response) - if !@upload_incomplete && (400..499).include?(response.status.to_i) && response.status.to_i != 404 + is_successful_cancellation = + !@upload_incomplete && + (400..499).cover?(response.status.to_i) && + response.status.to_i != 404 + if is_successful_cancellation @close_io_on_finish = true - true # method returns true if upload is successfully cancelled + return true # method returns true if upload is successfully cancelled else - logger.debug { sprintf("Failed to cancel upload session. Response: #{response.status.to_i} - #{response.body}") } - false + logger.debug { "Failed to cancel upload session. Response: #{response.status.to_i} - #{response.body}" } + return false end end diff --git a/google-apis-core/spec/google/apis/core/service_spec.rb b/google-apis-core/spec/google/apis/core/service_spec.rb index f96e0bd5fdd..ad156c4cc6a 100644 --- a/google-apis-core/spec/google/apis/core/service_spec.rb +++ b/google-apis-core/spec/google/apis/core/service_spec.rb @@ -301,7 +301,7 @@ .to_return(status: 404) end it 'should return false' do - expect(command).to be_falsey + expect(command).to be(false) end end @@ -345,7 +345,7 @@ .to_return(status: 404) end it 'should return false' do - expect(command).to be_falsey + expect(command).to be(false) end end end From b5c1d105b01264c59a18bd70074e73f58829550f Mon Sep 17 00:00:00 2001 From: Shubhangi Singh Date: Tue, 28 Oct 2025 11:02:59 +0000 Subject: [PATCH 6/6] fix --- google-apis-core/lib/google/apis/core/storage_upload.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-apis-core/lib/google/apis/core/storage_upload.rb b/google-apis-core/lib/google/apis/core/storage_upload.rb index dd11ba98948..6a2a547e9f9 100644 --- a/google-apis-core/lib/google/apis/core/storage_upload.rb +++ b/google-apis-core/lib/google/apis/core/storage_upload.rb @@ -225,7 +225,7 @@ def check_resumable_upload(client) response = client.put(@upload_url, "", request_header) handle_resumable_upload_http_response_codes(response) if response.status.to_i == 404 - logger.debug { sprintf("Failed to fetch upload session. Response: #{response.status.to_i} - #{response.body}") } + logger.debug { "Failed to fetch upload session. Response: #{response.status.to_i} - #{response.body}" } false end end