From 311af752cfd98b534a3d5dbf30e4a202693f32dc Mon Sep 17 00:00:00 2001
From: George Claghorn <george@basecamp.com>
Date: Fri, 15 Dec 2017 10:45:00 -0500
Subject: [PATCH] Restrict variants to variable image blobs

---
 .../app/models/active_storage/blob.rb         | 22 +++++++++++++++----
 activestorage/lib/active_storage.rb           |  1 +
 activestorage/lib/active_storage/engine.rb    |  2 ++
 activestorage/test/models/variant_test.rb     |  6 +++++
 4 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb
index acaf22fac1..fdf17ac67e 100644
--- a/activestorage/app/models/active_storage/blob.rb
+++ b/activestorage/app/models/active_storage/blob.rb
@@ -16,6 +16,7 @@ require "active_storage/analyzer/null_analyzer"
 # update a blob's metadata on a subsequent pass, but you should not update the key or change the uploaded file.
 # If you need to create a derivative or otherwise change the blob, simply create a new blob and purge the old one.
 class ActiveStorage::Blob < ActiveRecord::Base
+  class InvariableError < StandardError; end
   class UnpreviewableError < StandardError; end
   class UnrepresentableError < StandardError; end
 
@@ -110,6 +111,7 @@ class ActiveStorage::Blob < ActiveRecord::Base
     content_type.start_with?("text")
   end
 
+
   # Returns an ActiveStorage::Variant instance with the set of +transformations+ provided. This is only relevant for image
   # files, and it allows any image to be transformed for size, colors, and the like. Example:
   #
@@ -125,8 +127,20 @@ class ActiveStorage::Blob < ActiveRecord::Base
   #
   # This will create a URL for that specific blob with that specific variant, which the ActiveStorage::VariantsController
   # can then produce on-demand.
+  #
+  # Raises ActiveStorage::Blob::InvariableError if ImageMagick cannot transform the blob. To determine whether a blob is
+  # variable, call ActiveStorage::Blob#previewable?.
   def variant(transformations)
-    ActiveStorage::Variant.new(self, ActiveStorage::Variation.wrap(transformations))
+    if variable?
+      ActiveStorage::Variant.new(self, ActiveStorage::Variation.wrap(transformations))
+    else
+      raise InvariableError
+    end
+  end
+
+  # Returns true if ImageMagick can transform the blob (its content type is in +ActiveStorage.variable_content_types+).
+  def variable?
+    ActiveStorage.variable_content_types.include?(content_type)
   end
 
 
@@ -170,16 +184,16 @@ class ActiveStorage::Blob < ActiveRecord::Base
     case
     when previewable?
       preview transformations
-    when image?
+    when variable?
       variant transformations
     else
       raise UnrepresentableError
     end
   end
 
-  # Returns true if the blob is an image or is previewable.
+  # Returns true if the blob is variable or is previewable.
   def representable?
-    image? || previewable?
+    variable? || previewable?
   end
 
 
diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb
index d1ff6b7032..3b28113dc7 100644
--- a/activestorage/lib/active_storage.rb
+++ b/activestorage/lib/active_storage.rb
@@ -41,4 +41,5 @@ module ActiveStorage
   mattr_accessor :queue
   mattr_accessor :previewers, default: []
   mattr_accessor :analyzers, default: []
+  mattr_accessor :variable_content_types, default: []
 end
diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb
index b870e6d4d6..b41d8bb4d7 100644
--- a/activestorage/lib/active_storage/engine.rb
+++ b/activestorage/lib/active_storage/engine.rb
@@ -16,6 +16,7 @@ module ActiveStorage
     config.active_storage = ActiveSupport::OrderedOptions.new
     config.active_storage.previewers = [ ActiveStorage::Previewer::PDFPreviewer, ActiveStorage::Previewer::VideoPreviewer ]
     config.active_storage.analyzers = [ ActiveStorage::Analyzer::ImageAnalyzer, ActiveStorage::Analyzer::VideoAnalyzer ]
+    config.active_storage.variable_content_types = [ "image/png", "image/gif", "image/jpg", "image/jpeg", "image/vnd.adobe.photoshop" ]
     config.active_storage.paths = ActiveSupport::OrderedOptions.new
 
     config.eager_load_namespaces << ActiveStorage
@@ -26,6 +27,7 @@ module ActiveStorage
         ActiveStorage.queue      = app.config.active_storage.queue
         ActiveStorage.previewers = app.config.active_storage.previewers || []
         ActiveStorage.analyzers  = app.config.active_storage.analyzers || []
+        ActiveStorage.variable_content_types = app.config.active_storage.variable_content_types || []
       end
     end
 
diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb
index b7d20ab55a..83ebce4446 100644
--- a/activestorage/test/models/variant_test.rb
+++ b/activestorage/test/models/variant_test.rb
@@ -27,6 +27,12 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
     assert_match(/Gray/, image.colorspace)
   end
 
+  test "variation of invariable blob" do
+    assert_raises ActiveStorage::Blob::InvariableError do
+      create_file_blob(filename: "report.pdf", content_type: "application/pdf").variant(resize: "100x100")
+    end
+  end
+
   test "service_url doesn't grow in length despite long variant options" do
     variant = @blob.variant(font: "a" * 10_000).processed
     assert_operator variant.service_url.length, :<, 500