From e5f4162b6178489181e3d7e7163ac12b7e0efc9d Mon Sep 17 00:00:00 2001
From: Julik Tarkhanov <me@julik.nl>
Date: Sun, 30 Dec 2018 17:56:22 +0100
Subject: Make Active Storage blob keys lowercase

Accommodate case-insensitive filesystems and database collations.
---
 activestorage/CHANGELOG.md                      |  6 ++++++
 activestorage/app/models/active_storage/blob.rb | 16 +++++++++++++---
 activestorage/test/models/blob_test.rb          |  4 ++++
 activestorage/test/models/variant_test.rb       |  2 +-
 4 files changed, 24 insertions(+), 4 deletions(-)

(limited to 'activestorage')

diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md
index 51890f308b..b643895893 100644
--- a/activestorage/CHANGELOG.md
+++ b/activestorage/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Use base36 (all lowercase) for all new Blob keys to prevent
+    collisions and undefined behavior with case-insensitive filesystems and
+    database indices.
+
+    *Julik Tarkhanov*
+
 *   It doesn’t include an `X-CSRF-Token` header if a meta tag is not found on
     the page. It previously included one with a value of `undefined`.
 
diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb
index 04f9dbff9f..6ca7d49bc1 100644
--- a/activestorage/app/models/active_storage/blob.rb
+++ b/activestorage/app/models/active_storage/blob.rb
@@ -79,6 +79,15 @@ class ActiveStorage::Blob < ActiveRecord::Base
     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.base36(28)
+    end
   end
 
   # Returns a signed ID for this blob that's suitable for reference on the client-side without fear of tampering.
@@ -87,9 +96,10 @@ class ActiveStorage::Blob < ActiveRecord::Base
     ActiveStorage.verifier.generate(id, purpose: :blob_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.
+  # Returns the key pointing to the file on the service that's associated with this blob. The key is the
+  # secure-token format from Rails in lower case. 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
diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb
index 1503f5fc50..54cf9e2b8a 100644
--- a/activestorage/test/models/blob_test.rb
+++ b/activestorage/test/models/blob_test.rb
@@ -47,6 +47,10 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
     assert_equal "text/csv", blob.content_type
   end
 
+  test "create after upload generates a 28-character base36 key" do
+    assert_match(/^[a-z0-9]{28}$/, create_blob.key)
+  end
+
   test "image?" do
     blob = create_file_blob filename: "racecar.jpg"
     assert_predicate blob, :image?
diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb
index 4f88440e54..012aee3cc9 100644
--- a/activestorage/test/models/variant_test.rb
+++ b/activestorage/test/models/variant_test.rb
@@ -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
-- 
cgit v1.2.3