aboutsummaryrefslogtreecommitdiffstats
path: root/activestorage
diff options
context:
space:
mode:
authorGeorge Claghorn <george@basecamp.com>2017-12-15 19:26:53 -0500
committerGeorge Claghorn <george@basecamp.com>2017-12-15 19:26:53 -0500
commitaf0caadb8d9781770399c1804976af4a71d1313b (patch)
tree19fe95a35822990cba61915b3efd38b4ed5f1513 /activestorage
parentca2c4cb7c4ee2f126e65ade6192a0c6b5ec58ffc (diff)
downloadrails-af0caadb8d9781770399c1804976af4a71d1313b.tar.gz
rails-af0caadb8d9781770399c1804976af4a71d1313b.tar.bz2
rails-af0caadb8d9781770399c1804976af4a71d1313b.zip
Handle invalid signed blob IDs gracefully
Diffstat (limited to 'activestorage')
-rw-r--r--activestorage/app/controllers/active_storage/blobs_controller.rb10
-rw-r--r--activestorage/app/controllers/active_storage/previews_controller.rb10
-rw-r--r--activestorage/app/controllers/active_storage/variants_controller.rb10
-rw-r--r--activestorage/app/controllers/concerns/active_storage/set_blob.rb16
-rw-r--r--activestorage/test/controllers/blobs_controller_test.rb5
-rw-r--r--activestorage/test/controllers/previews_controller_test.rb9
-rw-r--r--activestorage/test/controllers/variants_controller_test.rb9
7 files changed, 51 insertions, 18 deletions
diff --git a/activestorage/app/controllers/active_storage/blobs_controller.rb b/activestorage/app/controllers/active_storage/blobs_controller.rb
index a17e3852f9..fa44131048 100644
--- a/activestorage/app/controllers/active_storage/blobs_controller.rb
+++ b/activestorage/app/controllers/active_storage/blobs_controller.rb
@@ -5,12 +5,10 @@
# security-through-obscurity factor of the signed blob references, you'll need to implement your own
# authenticated redirection controller.
class ActiveStorage::BlobsController < ActionController::Base
+ include ActiveStorage::SetBlob
+
def show
- if blob = ActiveStorage::Blob.find_signed(params[:signed_id])
- expires_in ActiveStorage::Blob.service.url_expires_in
- redirect_to blob.service_url(disposition: params[:disposition])
- else
- head :not_found
- end
+ expires_in ActiveStorage::Blob.service.url_expires_in
+ redirect_to @blob.service_url(disposition: params[:disposition])
end
end
diff --git a/activestorage/app/controllers/active_storage/previews_controller.rb b/activestorage/app/controllers/active_storage/previews_controller.rb
index 9e8cf27b6e..aa7ef58ca4 100644
--- a/activestorage/app/controllers/active_storage/previews_controller.rb
+++ b/activestorage/app/controllers/active_storage/previews_controller.rb
@@ -1,12 +1,10 @@
# frozen_string_literal: true
class ActiveStorage::PreviewsController < ActionController::Base
+ include ActiveStorage::SetBlob
+
def show
- if blob = ActiveStorage::Blob.find_signed(params[:signed_blob_id])
- expires_in ActiveStorage::Blob.service.url_expires_in
- redirect_to ActiveStorage::Preview.new(blob, params[:variation_key]).processed.service_url(disposition: params[:disposition])
- else
- head :not_found
- end
+ expires_in ActiveStorage::Blob.service.url_expires_in
+ redirect_to ActiveStorage::Preview.new(@blob, params[:variation_key]).processed.service_url(disposition: params[:disposition])
end
end
diff --git a/activestorage/app/controllers/active_storage/variants_controller.rb b/activestorage/app/controllers/active_storage/variants_controller.rb
index dc5e78ecc0..e8f8dd592d 100644
--- a/activestorage/app/controllers/active_storage/variants_controller.rb
+++ b/activestorage/app/controllers/active_storage/variants_controller.rb
@@ -5,12 +5,10 @@
# security-through-obscurity factor of the signed blob and variation reference, you'll need to implement your own
# authenticated redirection controller.
class ActiveStorage::VariantsController < ActionController::Base
+ include ActiveStorage::SetBlob
+
def show
- if blob = ActiveStorage::Blob.find_signed(params[:signed_blob_id])
- expires_in ActiveStorage::Blob.service.url_expires_in
- redirect_to ActiveStorage::Variant.new(blob, params[:variation_key]).processed.service_url(disposition: params[:disposition])
- else
- head :not_found
- end
+ expires_in ActiveStorage::Blob.service.url_expires_in
+ redirect_to ActiveStorage::Variant.new(@blob, params[:variation_key]).processed.service_url(disposition: params[:disposition])
end
end
diff --git a/activestorage/app/controllers/concerns/active_storage/set_blob.rb b/activestorage/app/controllers/concerns/active_storage/set_blob.rb
new file mode 100644
index 0000000000..b0f3d97a66
--- /dev/null
+++ b/activestorage/app/controllers/concerns/active_storage/set_blob.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+module ActiveStorage::SetBlob
+ extend ActiveSupport::Concern
+
+ included do
+ before_action :set_blob
+ end
+
+ private
+ def set_blob
+ @blob = ActiveStorage::Blob.find_signed(params[:signed_blob_id] || params[:signed_id])
+ rescue ActiveSupport::MessageVerifier::InvalidSignature
+ head :not_found
+ end
+end
diff --git a/activestorage/test/controllers/blobs_controller_test.rb b/activestorage/test/controllers/blobs_controller_test.rb
index 97177e64c2..9c811df895 100644
--- a/activestorage/test/controllers/blobs_controller_test.rb
+++ b/activestorage/test/controllers/blobs_controller_test.rb
@@ -8,6 +8,11 @@ class ActiveStorage::BlobsControllerTest < ActionDispatch::IntegrationTest
@blob = create_file_blob filename: "racecar.jpg"
end
+ test "showing blob with invalid signed ID" do
+ get rails_service_blob_url("invalid", "racecar.jpg")
+ assert_response :not_found
+ end
+
test "showing blob utilizes browser caching" do
get rails_blob_url(@blob)
diff --git a/activestorage/test/controllers/previews_controller_test.rb b/activestorage/test/controllers/previews_controller_test.rb
index c3151a710e..704a466160 100644
--- a/activestorage/test/controllers/previews_controller_test.rb
+++ b/activestorage/test/controllers/previews_controller_test.rb
@@ -21,4 +21,13 @@ class ActiveStorage::PreviewsControllerTest < ActionDispatch::IntegrationTest
assert_equal 77, image.width
assert_equal 100, image.height
end
+
+ test "showing preview with invalid signed blob ID" do
+ get rails_blob_preview_url(
+ filename: @blob.filename,
+ signed_blob_id: "invalid",
+ variation_key: ActiveStorage::Variation.encode(resize: "100x100"))
+
+ assert_response :not_found
+ end
end
diff --git a/activestorage/test/controllers/variants_controller_test.rb b/activestorage/test/controllers/variants_controller_test.rb
index 6c70d73786..a0642f9bed 100644
--- a/activestorage/test/controllers/variants_controller_test.rb
+++ b/activestorage/test/controllers/variants_controller_test.rb
@@ -20,4 +20,13 @@ class ActiveStorage::VariantsControllerTest < ActionDispatch::IntegrationTest
assert_equal 100, image.width
assert_equal 67, image.height
end
+
+ test "showing variant with invalid signed blob ID" do
+ get rails_blob_variation_url(
+ filename: @blob.filename,
+ signed_blob_id: "invalid",
+ variation_key: ActiveStorage::Variation.encode(resize: "100x100"))
+
+ assert_response :not_found
+ end
end