From 56d14e6cc417572e2967703af9862dd19474cab6 Mon Sep 17 00:00:00 2001 From: blocknotes Date: Tue, 6 Sep 2022 11:36:12 +0200 Subject: [PATCH 1/4] Sanitize SQL like value --- lib/active_storage/service/db_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_storage/service/db_service.rb b/lib/active_storage/service/db_service.rb index d783318..eee08d9 100644 --- a/lib/active_storage/service/db_service.rb +++ b/lib/active_storage/service/db_service.rb @@ -63,7 +63,7 @@ def delete(key) def delete_prefixed(prefix) instrument :delete_prefixed, prefix: prefix do - ::ActiveStorageDB::File.where('ref LIKE ?', "#{prefix}%").destroy_all + ::ActiveStorageDB::File.where('ref LIKE ?', "#{ApplicationRecord.sanitize_sql_like(prefix)}%").destroy_all end end From 2d09f06e8a5538e1c9d28ee9566a38cd7f7ffceb Mon Sep 17 00:00:00 2001 From: blocknotes Date: Tue, 6 Sep 2022 09:40:14 +0200 Subject: [PATCH 2/4] Minor improvements to the files controller --- .../active_storage_db/files_controller.rb | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/app/controllers/active_storage_db/files_controller.rb b/app/controllers/active_storage_db/files_controller.rb index 727c88d..e9618c1 100644 --- a/app/controllers/active_storage_db/files_controller.rb +++ b/app/controllers/active_storage_db/files_controller.rb @@ -8,34 +8,41 @@ def show if (key = decode_verified_key) serve_file(key[:key], content_type: key[:content_type], disposition: key[:disposition]) else - head :not_found + head(:not_found) end rescue ActiveStorage::FileNotFoundError - head :not_found + head(:not_found) end def update if (token = decode_verified_token) - if acceptable_content?(token) - db_service.upload(token[:key], request.body, checksum: token[:checksum]) - else - head :unprocessable_entity - end + file_uploaded = upload_file(token, body: request.body) + head(file_uploaded ? :no_content : :unprocessable_entity) else - head :not_found + head(:not_found) end rescue ActiveStorage::IntegrityError - head :unprocessable_entity + head(:unprocessable_entity) end private + def acceptable_content?(token) + token[:content_type] == request.content_mime_type && token[:content_length] == request.content_length + end + def db_service ActiveStorage::Blob.service end def decode_verified_key - ActiveStorage.verifier.verified(params[:encoded_key], purpose: :blob_key) + key = ActiveStorage.verifier.verified(params[:encoded_key], purpose: :blob_key) + key&.deep_symbolize_keys + end + + def decode_verified_token + token = ActiveStorage.verifier.verified(params[:encoded_token], purpose: :blob_token) + token&.deep_symbolize_keys end def serve_file(key, content_type:, disposition:) @@ -43,15 +50,14 @@ def serve_file(key, content_type:, disposition:) type: content_type || DEFAULT_SEND_FILE_TYPE, disposition: disposition || DEFAULT_SEND_FILE_DISPOSITION } - send_data db_service.download(key), options + send_data(db_service.download(key), options) end - def decode_verified_token - ActiveStorage.verifier.verified(params[:encoded_token], purpose: :blob_token) - end + def upload_file(token, body:) + return false unless acceptable_content?(token) - def acceptable_content?(token) - token[:content_type] == request.content_mime_type && token[:content_length] == request.content_length + db_service.upload(token[:key], request.body, checksum: token[:checksum]) + true end end end From 0b5bb4192f27a6cd58d2f281d97b79e5b06ec99c Mon Sep 17 00:00:00 2001 From: blocknotes Date: Tue, 6 Sep 2022 10:20:17 +0200 Subject: [PATCH 3/4] Minor improvements to the create active storage db files migration --- ...0702202022_create_active_storage_db_files.rb | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/db/migrate/20200702202022_create_active_storage_db_files.rb b/db/migrate/20200702202022_create_active_storage_db_files.rb index 5bb1d57..0636c97 100644 --- a/db/migrate/20200702202022_create_active_storage_db_files.rb +++ b/db/migrate/20200702202022_create_active_storage_db_files.rb @@ -2,12 +2,25 @@ class CreateActiveStorageDBFiles < ActiveRecord::Migration[6.0] def change - create_table :active_storage_db_files do |t| + create_table :active_storage_db_files, id: primary_key_type do |t| t.string :ref, null: false t.binary :data, null: false - t.datetime :created_at, null: false + + if connection.supports_datetime_with_precision? + t.datetime :created_at, precision: 6, null: false + else + t.datetime :created_at, null: false + end t.index [:ref], unique: true end end + + private + + def primary_key_type + config = Rails.configuration.generators + primary_key_type = config.options[config.orm][:primary_key_type] + primary_key_type || :primary_key + end end From eb972e500443aef4947c95a0ee1739175ecdc795 Mon Sep 17 00:00:00 2001 From: blocknotes Date: Tue, 6 Sep 2022 09:29:21 +0200 Subject: [PATCH 4/4] Minor improvements to the DB service component --- lib/active_storage/service/db_service.rb | 27 ++++++++++++++---------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/active_storage/service/db_service.rb b/lib/active_storage/service/db_service.rb index eee08d9..cd488ad 100644 --- a/lib/active_storage/service/db_service.rb +++ b/lib/active_storage/service/db_service.rb @@ -36,18 +36,14 @@ def download(key, &block) end else instrument :download, key: key do - record = ::ActiveStorageDB::File.find_by(ref: key) - raise(ActiveStorage::FileNotFoundError) unless record - - record.data + retrieve_file(key) end end end def download_chunk(key, range) instrument :download_chunk, key: key, range: range do - chunk_select = "SUBSTRING(data FROM #{range.begin + 1} FOR #{range.size}) AS chunk" - record = ::ActiveStorageDB::File.select(chunk_select).find_by(ref: key) + record = object_for(key, fields: "SUBSTRING(data FROM #{range.begin + 1} FOR #{range.size}) AS chunk") raise(ActiveStorage::FileNotFoundError) unless record record.chunk @@ -127,17 +123,26 @@ def generate_url(key, expires_in:, filename:, content_type:, disposition:) end def ensure_integrity_of(key, checksum) - file = ::ActiveStorageDB::File.find_by(ref: key) - return if Digest::MD5.base64digest(file.data) == checksum + return if Digest::MD5.base64digest(object_for(key).data) == checksum delete(key) raise ActiveStorage::IntegrityError end + def retrieve_file(key) + file = object_for(key) + raise(ActiveStorage::FileNotFoundError) unless file + + file.data + end + + def object_for(key, fields: nil) + as_file = fields ? ::ActiveStorageDB::File.select(*fields) : ::ActiveStorageDB::File + as_file.find_by(ref: key) + end + def stream(key) - size = - ::ActiveStorageDB::File.select('OCTET_LENGTH(data) AS size').find_by(ref: key)&.size || - raise(ActiveStorage::FileNotFoundError) + size = object_for(key, fields: 'OCTET_LENGTH(data) AS size')&.size || raise(ActiveStorage::FileNotFoundError) (size / @chunk_size.to_f).ceil.times.each do |i| range = (i * @chunk_size..((i + 1) * @chunk_size) - 1) yield download_chunk(key, range)