Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions google-cloud-storage/lib/google/cloud/storage/bucket.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1628,19 +1628,20 @@ def file path,
# changed to a time in the future. If custom_time must be unset, you
# must either perform a rewrite operation, or upload the data again
# and create a new file.
# @param [Symbol, nil] checksum The type of checksum for the client to
# @param [Symbol, nil, Boolean] checksum The type of checksum for the client to
# automatically calculate and send with the create request to verify
# the integrity of the object. If provided, Cloud Storage will only
# create the file if the value calculated by the client matches the
# value calculated by the service.
#
# Acceptable values are:
#
# * `true` [Boolean] - Calculate and provide a checksum using the CRC32c hash.
# * `false` [Boolean] - Do not calculate or provide a checksum.
# * `md5` - Calculate and provide a checksum using the MD5 hash.
# * `crc32c` - Calculate and provide a checksum using the CRC32c hash.
# * `all` - Calculate and provide checksums for all available verifications.
#
# Optional. The default is `nil`. Do not provide if also providing a
# Optional. The default is `crc32c`. Do not provide if also providing a
# corresponding `crc32c` or `md5` argument. See
# [Validation](https://cloud.google.com/storage/docs/hashes-etags)
# for more information.
Expand Down Expand Up @@ -1805,6 +1806,11 @@ def create_file file,
path ||= file.path if file.respond_to? :path
path ||= file if file.is_a? String
raise ArgumentError, "must provide path" if path.nil?
# If no checksum type or specific value is provided, the default will be set to crc32c.
# If the checksum is set to false, it will be disabled.
if [checksum, crc32c, md5].all?(&:nil?) || checksum == true
checksum = :crc32c
end
crc32c = crc32c_for file, checksum, crc32c
md5 = md5_for file, checksum, md5

Expand Down
48 changes: 33 additions & 15 deletions google-cloud-storage/lib/google/cloud/storage/file/verifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,29 +49,47 @@ def self.verify_crc32c gcloud_file, local_file
gcloud_file.crc32c == crc32c_for(local_file)
end

# Calculates MD5 digest using either file path or open stream.
def self.md5_for local_file
if local_file.respond_to? :to_path
::File.open Pathname(local_file).to_path, "rb" do |f|
::Digest::MD5.file(f).base64digest
end
else # StringIO
local_file.rewind
md5 = ::Digest::MD5.base64digest local_file.read
local_file.rewind
md5
end
_digest_for local_file, ::Digest::MD5
end

# Calculates CRC32c digest using either file path or open stream.
def self.crc32c_for local_file
if local_file.respond_to? :to_path
_digest_for local_file, ::Digest::CRC32c
end

# @private
# Computes a base64-encoded digest for a local file or IO stream.
#
# This method handles two types of inputs for `local_file`:
# 1. A file path (String or Pathname): It efficiently streams the file
# to compute the digest without loading the entire file into memory.
# 2. An IO-like stream (e.g., File, StringIO): It reads the stream's
# content to compute the digest. The stream is rewound before and after
# reading to ensure its position is not permanently changed.
#
# @param local_file [String, Pathname, IO] The local file path or IO
# stream for which to compute the digest.
# @param digest_class [Class] The digest class to use for the
# calculation (e.g., `Digest::MD5`). It must respond to `.file` and
# `.base64digest`.
#
# @return [String] The base64-encoded digest of the file's content.
#
def self._digest_for local_file, digest_class

if local_file.respond_to?(:to_path) || local_file.is_a?(String)
# Case 1: Input is a file path (String, Pathname, or object that responds to :to_path).
::File.open Pathname(local_file).to_path, "rb" do |f|
::Digest::CRC32c.file(f).base64digest
digest_class.file(f).base64digest
end
else # StringIO
else
# Case 2: Input is an open stream (File or StringIO).
local_file.rewind
crc32c = ::Digest::CRC32c.base64digest local_file.read
digest = digest_class.base64digest local_file.read
local_file.rewind
crc32c
digest
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,12 @@ def create_file_gapi bucket=nil, name = nil
def empty_file_gapi cache_control: nil, content_disposition: nil,
content_encoding: nil, content_language: nil,
content_type: nil, crc32c: nil, md5: nil, metadata: nil,
storage_class: nil
storage_class: nil, checksum: nil

# If no checksum type or specific value is provided, the default will be set to crc32c.
# If the checksum is set to false, it will be disabled.
crc32c ||= set_crc32c_as_default md5, crc32c, checksum

params = {
cache_control: cache_control, content_type: content_type,
content_disposition: content_disposition, md5_hash: md5,
Expand Down
89 changes: 83 additions & 6 deletions google-cloud-storage/test/google/cloud/storage/bucket_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,41 @@
_(bucket_complete.autoclass_enabled).must_equal bucket_autoclass_enabled
_(bucket_complete.autoclass_terminal_storage_class).must_equal bucket_autoclass_terminal_storage_class
end

it "creates a file with checksum: :crc32c by default" do
new_file_name = random_file_path

Tempfile.open ["google-cloud", ".txt"] do |tmpfile|
tmpfile.write "Hello world!"
tmpfile.rewind

crc32c = Google::Cloud::Storage::File::Verifier.crc32c_for tmpfile

mock = Minitest::Mock.new
mock.expect :insert_object, create_file_gapi(bucket.name, new_file_name),
[bucket.name, empty_file_gapi(crc32c: crc32c)], **insert_object_args(name: new_file_name, upload_source: tmpfile, options: {retries: 0})

bucket.service.mocked_service = mock
bucket.create_file tmpfile, new_file_name

mock.verify
end
end

it "creates a file with a StringIO and checksum: :crc32c by default" do
new_file_name = random_file_path
new_file_contents = StringIO.new "Hello world"
crc32c = Google::Cloud::Storage::File::Verifier.crc32c_for new_file_contents
mock = Minitest::Mock.new
mock.expect :insert_object, create_file_gapi(bucket.name, new_file_name),
[bucket.name, empty_file_gapi(crc32c: crc32c)], **insert_object_args(name: new_file_name, upload_source: new_file_contents, options: {retries: 0})

bucket.service.mocked_service = mock

bucket.create_file new_file_contents, new_file_name

mock.verify
end

it "returns frozen cors" do
bucket_complete.cors.each do |cors|
Expand Down Expand Up @@ -405,6 +440,42 @@
end
end

it "creates a file with no checksum" do
new_file_name = random_file_path

Tempfile.open ["google-cloud", ".txt"] do |tmpfile|
tmpfile.write "Hello world!"
tmpfile.rewind

mock = Minitest::Mock.new
mock.expect :insert_object, create_file_gapi(bucket.name, new_file_name),
[bucket.name, empty_file_gapi(checksum: false)], **insert_object_args(name: new_file_name, upload_source: tmpfile, options: {retries: 0})

bucket.service.mocked_service = mock

bucket.create_file tmpfile, new_file_name, checksum: false
mock.verify
end
end

it "creates a file with crc32c if checksum is true" do
new_file_name = random_file_path

Tempfile.open ["google-cloud", ".txt"] do |tmpfile|
tmpfile.write "Hello world!"
tmpfile.rewind

mock = Minitest::Mock.new
mock.expect :insert_object, create_file_gapi(bucket.name, new_file_name),
[bucket.name, empty_file_gapi(checksum: true, crc32c: "e5jnUQ==")], **insert_object_args(name: new_file_name, upload_source: tmpfile, options: {retries: 0})
bucket.service.mocked_service = mock

bucket.create_file tmpfile, new_file_name, checksum: true

mock.verify
end
end

it "creates a file with attributes" do
new_file_name = random_file_path

Expand Down Expand Up @@ -595,9 +666,11 @@
new_file_name = random_file_path

Tempfile.create ["google-cloud", ".txt"] do |tmpfile|

crc32c = Google::Cloud::Storage::File::Verifier.crc32c_for tmpfile
mock = Minitest::Mock.new
mock.expect :insert_object, create_file_gapi(bucket_user_project.name, new_file_name),
[bucket.name, empty_file_gapi], **insert_object_args(name: new_file_name, upload_source: tmpfile, user_project: "test", options: {retries: 0})
[bucket.name, empty_file_gapi(crc32c: crc32c)], **insert_object_args(name: new_file_name, upload_source: tmpfile, user_project: "test", options: {retries: 0})

bucket_user_project.service.mocked_service = mock

Expand All @@ -608,13 +681,13 @@
end
end

it "creates an file with a StringIO" do
it "creates a file with StringIO" do
new_file_name = random_file_path
new_file_contents = StringIO.new

new_file_contents = StringIO.new("Hello world string_io")
crc32c = Google::Cloud::Storage::File::Verifier.crc32c_for new_file_contents
mock = Minitest::Mock.new
mock.expect :insert_object, create_file_gapi(bucket.name, new_file_name),
[bucket.name, empty_file_gapi], **insert_object_args(name: new_file_name, upload_source: new_file_contents, options: {retries: 0})
[bucket.name, empty_file_gapi(crc32c: crc32c)], **insert_object_args(name: new_file_name, upload_source: new_file_contents, options: {retries: 0})

bucket.service.mocked_service = mock

Expand Down Expand Up @@ -1416,7 +1489,11 @@ def empty_file_gapi cache_control: nil, content_disposition: nil,
content_encoding: nil, content_language: nil,
content_type: nil, crc32c: nil, md5: nil, metadata: nil,
storage_class: nil, temporary_hold: nil,
event_based_hold: nil
event_based_hold: nil, checksum: nil

# If no checksum type or specific value is provided, the default will be set to crc32c.
# If the checksum is set to false, it will be disabled.
crc32c ||= set_crc32c_as_default md5, crc32c, checksum

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This change, while intended to support default checksums in tests, will likely cause existing tests for empty file creation (e.g., it "creates an empty file") to fail. When empty_file_gapi is called with no arguments, set_crc32c_as_default is invoked and incorrectly adds a crc32c expectation to the mock. The production code doesn't send a checksum for empty files, so the mock verification will fail.

This logic needs to be more nuanced to avoid applying a default checksum for mocks of empty file uploads. You might need to modify empty_file_gapi to differentiate between uploads with and without content.

params = {
cache_control: cache_control, content_type: content_type,
content_disposition: content_disposition, md5_hash: md5,
Expand Down
66 changes: 63 additions & 3 deletions google-cloud-storage/test/google/cloud/storage/lazy/bucket_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,62 @@
mock.verify
end
end

it "creates a file with checksum: :crc32c by default" do
new_file_name = random_file_path

Tempfile.open ["google-cloud", ".txt"] do |tmpfile|
tmpfile.write "Hello world 123"
tmpfile.rewind

crc32c = Google::Cloud::Storage::File::Verifier.crc32c_for tmpfile
mock = Minitest::Mock.new
mock.expect :insert_object, create_file_gapi(bucket.name, new_file_name),
[bucket.name, empty_file_gapi(crc32c: crc32c)], **insert_object_args(name: new_file_name, upload_source: tmpfile, options: {retries: 0})

bucket.service.mocked_service = mock
bucket.create_file tmpfile, new_file_name

mock.verify
end
end

it "creates a file with no checksum" do
new_file_name = random_file_path

Tempfile.open ["google-cloud", ".txt"] do |tmpfile|
tmpfile.write "Hello world!"
tmpfile.rewind

mock = Minitest::Mock.new
mock.expect :insert_object, create_file_gapi(bucket.name, new_file_name),
[bucket.name, empty_file_gapi(checksum: false)], **insert_object_args(name: new_file_name, upload_source: tmpfile, options: {retries: 0})

bucket.service.mocked_service = mock

bucket.create_file tmpfile, new_file_name, checksum: false
mock.verify
end
end

it "creates a file with crc32c if checksum is true" do
new_file_name = random_file_path

Tempfile.open ["google-cloud", ".txt"] do |tmpfile|
tmpfile.write "Hello world!"
tmpfile.rewind

mock = Minitest::Mock.new
mock.expect :insert_object, create_file_gapi(bucket.name, new_file_name),
[bucket.name, empty_file_gapi(checksum: true, crc32c: "e5jnUQ==")], **insert_object_args(name: new_file_name, upload_source: tmpfile, options: {retries: 0})

bucket.service.mocked_service = mock

bucket.create_file tmpfile, new_file_name, checksum: true

mock.verify
end
end

it "creates a file with attributes" do
new_file_name = random_file_path
Expand Down Expand Up @@ -279,7 +335,6 @@
Tempfile.open ["google-cloud", ".txt"] do |tmpfile|
tmpfile.write "Hello world"
tmpfile.rewind

metadata = {
"player" => "Bob",
score: 10
Expand Down Expand Up @@ -340,9 +395,10 @@
new_file_name = random_file_path

Tempfile.create ["google-cloud", ".txt"] do |tmpfile|
crc32c = Google::Cloud::Storage::File::Verifier.crc32c_for tmpfile
mock = Minitest::Mock.new
mock.expect :insert_object, create_file_gapi(bucket_user_project.name, new_file_name),
[bucket.name, empty_file_gapi], **insert_object_args(name: new_file_name, upload_source: tmpfile, user_project: "test", options: {retries: 0})
[bucket.name, empty_file_gapi(crc32c: crc32c)], **insert_object_args(name: new_file_name, upload_source: tmpfile, user_project: "test", options: {retries: 0})

bucket_user_project.service.mocked_service = mock

Expand Down Expand Up @@ -1090,7 +1146,11 @@ def create_file_gapi bucket=nil, name = nil
def empty_file_gapi cache_control: nil, content_disposition: nil,
content_encoding: nil, content_language: nil,
content_type: nil, crc32c: nil, md5: nil, metadata: nil,
storage_class: nil
storage_class: nil, checksum: nil

# If no checksum type or specific value is provided, the default will be set to crc32c.
# If the checksum is set to false, it will be disabled.
crc32c ||= set_crc32c_as_default md5, crc32c, checksum
params = {
cache_control: cache_control, content_type: content_type,
content_disposition: content_disposition, md5_hash: md5,
Expand Down
8 changes: 8 additions & 0 deletions google-cloud-storage/test/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -612,4 +612,12 @@ def restore_file_gapi bucket, file_name, generation=nil
file_hash = random_file_hash(bucket, file_name, generation).to_json
Google::Apis::StorageV1::Object.from_json file_hash
end

def set_crc32c_as_default md5, crc32c, checksum
# If no checksum type or specific value is provided, the default will be set to crc32c.
# If the checksum is set to false, it will be disabled. if [checksum, crc32c, md5].all?(&:nil?) || checksum == true
crc32c = Google::Cloud::Storage::File::Verifier.crc32c_for(StringIO.new("Hello world"))
end
crc32c
end
end