Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ActiveStorage::Blob keys lowercase #34818

Merged
merged 12 commits into from Dec 30, 2018
14 changes: 12 additions & 2 deletions activestorage/app/models/active_storage/blob.rb
Expand Up @@ -79,6 +79,15 @@ def create_after_upload!(io:, filename:, content_type: nil, metadata: nil, ident
def create_before_direct_upload!(filename:, byte_size:, checksum:, content_type: nil, metadata: nil)
create! filename: filename, byte_size: byte_size, checksum: checksum, content_type: content_type, metadata: metadata
end

# To prevent problems with case-insensitive filesystems, especially in combination
# with databases which treat indices as case-sensitive, all blob keys generated are going
# to only contain the base-36 character alphabet and will therefore be lowercase. To maintain
# the same or higher amount of entropy as in the base-58 encoding used by `has_secure_token`
# the number of bytes used is increased to 28 from the standard 24
def generate_unique_secure_token
SecureRandom.base58(28).downcase
end
end

# Returns a signed ID for this blob that's suitable for reference on the client-side without fear of tampering.
Expand All @@ -88,8 +97,9 @@ def signed_id
end

# Returns the key pointing to the file on the service that's associated with this blob. The key is in the
# standard secure-token format from Rails. So it'll look like: XTAPjJCJiuDrLk3TmwyJGpUo. This key is not intended
# to be revealed directly to the user. Always refer to blobs using the signed_id or a verified form of the key.
# standard secure-token format from Rails. So it'll look like: xtapjjcjiudrlk3tmwyjgpuobabd.
# This key is not intended to be revealed directly to the user.
# Always refer to blobs using the signed_id or a verified form of the key.
def key
# We can't wait until the record is first saved to have a key for it
self[:key] ||= self.class.generate_unique_secure_token
Expand Down
10 changes: 10 additions & 0 deletions activestorage/test/models/blob_test.rb
Expand Up @@ -47,6 +47,16 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
assert_equal "text/csv", blob.content_type
end

test "create after upload only assigns lowercase keys with 28 bytes of data" do
blobs = 20.times.map do
create_blob data: Random.new.bytes(20), filename: "rand.bin", content_type: "binary/octet-stream", identify: false
end
keys = blobs.map(&:key)
keys.each do |key|
assert_match /^[a-z0-9]{28}$/, key
end
end
georgeclaghorn marked this conversation as resolved.
Show resolved Hide resolved

test "image?" do
blob = create_file_blob filename: "racecar.jpg"
assert_predicate blob, :image?
Expand Down
2 changes: 1 addition & 1 deletion activestorage/test/models/variant_test.rb
Expand Up @@ -150,7 +150,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
test "service_url doesn't grow in length despite long variant options" do
blob = create_file_blob(filename: "racecar.jpg")
variant = blob.variant(font: "a" * 10_000).processed
assert_operator variant.service_url.length, :<, 726
assert_operator variant.service_url.length, :<, 730
end

test "works for vips processor" do
Expand Down