From 3c848ca01d9ffdb9c1ce4e1e548ba04704683ea9 Mon Sep 17 00:00:00 2001 From: Ghouse Mohamed Date: Tue, 31 May 2022 17:28:31 +0530 Subject: [PATCH] Added validation for global active storage service configuration --- activestorage/CHANGELOG.md | 10 +++++++++- activestorage/lib/active_storage/attached/model.rb | 8 ++++++++ activestorage/test/dummy/config/environments/test.rb | 3 +++ activestorage/test/models/attached/many_test.rb | 12 ++++++++++++ activestorage/test/models/attached/one_test.rb | 12 ++++++++++++ 5 files changed, 44 insertions(+), 1 deletion(-) diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index e77689cb10371..87dd6e0b69c6d 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,4 +1,12 @@ -* Fixes proxy downloads of files over 5mb +* Raise an exception if `config.active_storage.service` is not set. + + If Active Storage is configured and `config.active_storage.service` is not + set in the respective environment's configuration file, then an exception + is raised with a meaningful message when attempting to use Active Storage. + + *Ghouse Mohamed* + +* Fixes proxy downloads of files over 5mb Previously, trying to view and/or download files larger than 5mb stored in services like S3 via proxy mode could return corrupted files at around diff --git a/activestorage/lib/active_storage/attached/model.rb b/activestorage/lib/active_storage/attached/model.rb index 06f335f0cccc4..13fe96ef8edd0 100644 --- a/activestorage/lib/active_storage/attached/model.rb +++ b/activestorage/lib/active_storage/attached/model.rb @@ -215,6 +215,14 @@ def validate_service_configuration(association_name, service) ActiveStorage::Blob.services.fetch(service) do raise ArgumentError, "Cannot configure service :#{service} for #{name}##{association_name}" end + else + validate_global_service_configuration + end + end + + def validate_global_service_configuration + if connected? && ActiveStorage::Blob.table_exists? && Rails.configuration.active_storage.service.nil? + raise RuntimeError, "Missing Active Storage service name. Specify Active Storage service name for config.active_storage.service in config/environments/#{Rails.env}.rb" end end end diff --git a/activestorage/test/dummy/config/environments/test.rb b/activestorage/test/dummy/config/environments/test.rb index 568462023cb4c..0b58d4108c2d3 100644 --- a/activestorage/test/dummy/config/environments/test.rb +++ b/activestorage/test/dummy/config/environments/test.rb @@ -33,6 +33,9 @@ # Disable request forgery protection in test environment. config.action_controller.allow_forgery_protection = false + # Store uploaded files on the local file system in a temporary directory + config.active_storage.service = :test + # Raises error for missing translations # config.i18n.raise_on_missing_translations = true diff --git a/activestorage/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb index 75983588ce8c4..3debf57c0511e 100644 --- a/activestorage/test/models/attached/many_test.rb +++ b/activestorage/test/models/attached/many_test.rb @@ -761,6 +761,18 @@ def highlights assert_equal highlights.third.filename.to_s, @user.highlights.third.filename.to_s end + test "raises error when global service configuration is missing" do + Rails.configuration.active_storage.stub(:service, nil) do + error = assert_raises RuntimeError do + User.class_eval do + has_one_attached :featured_photos + end + end + + assert_match(/Missing Active Storage service name. Specify Active Storage service name for config.active_storage.service in config\/environments\/test.rb/, error.message) + end + end + test "raises error when misconfigured service is passed" do error = assert_raises ArgumentError do User.class_eval do diff --git a/activestorage/test/models/attached/one_test.rb b/activestorage/test/models/attached/one_test.rb index 22672b2cda103..0a5027e47e5cf 100644 --- a/activestorage/test/models/attached/one_test.rb +++ b/activestorage/test/models/attached/one_test.rb @@ -667,6 +667,18 @@ def avatar end end + test "raises error when global service configuration is missing" do + Rails.configuration.active_storage.stub(:service, nil) do + error = assert_raises RuntimeError do + User.class_eval do + has_one_attached :featured_photos + end + end + + assert_match(/Missing Active Storage service name. Specify Active Storage service name for config.active_storage.service in config\/environments\/test.rb/, error.message) + end + end + test "raises error when misconfigured service is passed" do error = assert_raises ArgumentError do User.class_eval do