From e73064faa39863c0446ebb611ffa345a41c68db1 Mon Sep 17 00:00:00 2001 From: Shubhangi Singh Date: Sun, 7 Sep 2025 21:45:35 +0000 Subject: [PATCH 1/3] 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 e6884c0e357db86bbd86afbc3e006bb6317f2a35 Mon Sep 17 00:00:00 2001 From: Shubhangi Singh Date: Mon, 8 Sep 2025 06:20:54 +0000 Subject: [PATCH 2/3] adding test cases --- .../lib/google/apis/core/storage_upload.rb | 2 -- .../spec/google/apis/core/service_spec.rb | 36 +++++++++++++++++++ .../google/apis/core/storage_upload_spec.rb | 17 +++++++++ 3 files changed, 53 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 e9f6d7720de..ef07fc8adf8 100644 --- a/google-apis-core/lib/google/apis/core/storage_upload.rb +++ b/google-apis-core/lib/google/apis/core/storage_upload.rb @@ -240,7 +240,6 @@ def cancel_resumable_upload(client) # Initiating call 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 @close_io_on_finish = true true # method returns true if upload is successfully cancelled @@ -248,7 +247,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 1c4c192477c2cf4bac6951600357868f86b66aa5 Mon Sep 17 00:00:00 2001 From: Shubhangi Singh Date: Mon, 8 Sep 2025 11:50:25 +0000 Subject: [PATCH 3/3] refactor --- google-apis-core/lib/google/apis/core/storage_upload.rb | 4 +--- 1 file changed, 1 insertion(+), 3 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 ef07fc8adf8..8eb18ca2a27 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