Make fingerprint digest configurable (#2229)

Adapters now accept an options parameter, that currently specifies
the type of hash digest to use.  The default value remains MD5, but
can be specified to be any OpenSSL-supported digest.  The specs are
modified to reflect that.

The task just reassigns all of the attachments, thereby regenerating
their fingerprints.

NEWS

@@ -1,5 +1,22 @@
 master:
 
+* Improvement: make the fingerprint digest configurable per attachment. The
+  default remains MD5 but this will change in a future version because it is
+  not considered secure anymore against intentional file corruption. For more
+  info, see https://en.wikipedia.org/wiki/MD5#Security
+
+  You can change the digest used for an attachment by adding the
+  `:adapter_options` parameter to the `has_attached_file` options like this:
+  `has_attached_file :avatar, adapter_options: { hash_digest: Digest::SHA256 }`
+
+  Use the rake task to regenerate fingerprints with the new digest for a given
+  class. Note that this does **not** check the file integrity using the old
+  fingerprint. Run the following command to regenerate fingerprints for all
+  User attachments:
+  `CLASS=User rake paperclip:refresh:fingerprints`
+  You can optionally limit the attachment that will be processed, e.g:
+  `CLASS=User ATTACHMENT=avatar rake paperclip:refresh:fingerprints`
+
 5.1.0 (2016-08-19):
 
 * Add default `content_type_detector` to `UploadedFileAdapter` (#2270)

README.md

@@ -737,10 +737,10 @@ is specified in `:hash_data`. The default value for `:hash_data` is `":class/:at
 
 For more on this feature, read [the author's own explanation](https://github.com/thoughtbot/paperclip/pull/416)
 
-MD5 Checksum / Fingerprint
+Checksum / Fingerprint
 -------
 
-An MD5 checksum of the original file assigned will be placed in the model if it
+A checksum of the original file assigned will be placed in the model if it
 has an attribute named fingerprint.  Following the user model migration example
 above, the migration would look like the following:
 
@@ -756,6 +756,17 @@ class AddAvatarFingerprintColumnToUser < ActiveRecord::Migration
 end
 ```
 
+The algorithm can be specified using a configuration option; it defaults to MD5
+for backwards compatibility with Paperclip 5 and earlier.
+
+```ruby
+has_attached_file :some_attachment, adapter_options: { hash_digest: Digest::SHA256 }
+```
+
+Run `CLASS=User ATTACHMENT=avatar rake paperclip:refresh:fingerprints` after
+changing the digest on existing attachments to update the fingerprints in the
+database.
+
 File Preservation for Soft-Delete
 -------
 

lib/paperclip/attachment.rb

@@ -33,6 +33,7 @@ def self.default_options
         :use_timestamp         => true,
         :whiny                 => Paperclip.options[:whiny] || Paperclip.options[:whiny_thumbnails],
         :validate_media_type   => true,
+        :adapter_options       => { hash_digest: Digest::MD5 },
         :check_validity_before_processing => true
       }
     end
@@ -97,7 +98,8 @@ def initialize(name, instance, options = {})
     # attachment:
     #   new_user.avatar = old_user.avatar
     def assign(uploaded_file)
-      @file = Paperclip.io_adapters.for(uploaded_file)
+      @file = Paperclip.io_adapters.for(uploaded_file,
+                                        @options[:adapter_options])
       ensure_required_accessors!
       ensure_required_validations!
 
@@ -523,15 +525,18 @@ def post_process_style(name, style) #:nodoc:
       begin
         raise RuntimeError.new("Style #{name} has no processors defined.") if style.processors.blank?
         intermediate_files = []
+        original = @queued_for_write[:original]
 
-        @queued_for_write[name] = style.processors.inject(@queued_for_write[:original]) do |file, processor|
+        @queued_for_write[name] = style.processors.
+          reduce(original) do |file, processor|
           file = Paperclip.processor(processor).make(file, style.processor_options, self)
           intermediate_files << file unless file == @queued_for_write[:original]
           file
         end
 
         unadapted_file = @queued_for_write[name]
-        @queued_for_write[name] = Paperclip.io_adapters.for(@queued_for_write[name])
+        @queued_for_write[name] = Paperclip.io_adapters.
+          for(@queued_for_write[name], @options[:adapter_options])
         unadapted_file.close if unadapted_file.respond_to?(:close)
         @queued_for_write[name]
       rescue Paperclip::Errors::NotIdentifiedByImageMagickError => e

lib/paperclip/io_adapters/abstract_adapter.rb

@@ -8,8 +8,20 @@ class AbstractAdapter
     delegate :binmode, :binmode?, :close, :close!, :closed?, :eof?, :path, :readbyte, :rewind, :unlink, :to => :@tempfile
     alias :length :size
 
+    def initialize(target, options = {})
+      @target = target
+      @options = options
+    end
+
     def fingerprint
-      @fingerprint ||= Digest::MD5.file(path).to_s
+      @fingerprint ||= begin
+        digest = @options.fetch(:hash_digest).new
+        File.open(path, "rb") do |f|
+          buf = ""
+          digest.update(buf) while f.read(16384, buf)
+        end
+        digest.hexdigest
+      end
     end
 
     def read(length = nil, buffer = nil)

lib/paperclip/io_adapters/attachment_adapter.rb

@@ -1,6 +1,7 @@
 module Paperclip
   class AttachmentAdapter < AbstractAdapter
-    def initialize(target)
+    def initialize(target, options = {})
+      super
       @target, @style = case target
       when Paperclip::Attachment
         [target, :original]

lib/paperclip/io_adapters/data_uri_adapter.rb

@@ -3,8 +3,8 @@ class DataUriAdapter < StringioAdapter
 
     REGEXP = /\Adata:([-\w]+\/[-\w\+\.]+)?;base64,(.*)/m
 
-    def initialize(target_uri)
-      super(extract_target(target_uri))
+    def initialize(target_uri, options = {})
+      super(extract_target(target_uri), options)
     end
 
     private

lib/paperclip/io_adapters/empty_string_adapter.rb

@@ -1,8 +1,5 @@
 module Paperclip
   class EmptyStringAdapter < AbstractAdapter
-    def initialize(target)
-    end
-
     def nil?
       false
     end

lib/paperclip/io_adapters/file_adapter.rb

@@ -1,7 +1,7 @@
 module Paperclip
   class FileAdapter < AbstractAdapter
-    def initialize(target)
-      @target = target
+    def initialize(target, options = {})
+      super
       cache_current_values
     end
 

lib/paperclip/io_adapters/http_url_proxy_adapter.rb

@@ -3,8 +3,8 @@ class HttpUrlProxyAdapter < UriAdapter
 
     REGEXP = /\Ahttps?:\/\//
 
-    def initialize(target)
-      super(URI(URI.escape(target)))
+    def initialize(target, options = {})
+      super(URI(URI.escape(target)), options)
     end
 
   end

lib/paperclip/io_adapters/identity_adapter.rb

@@ -1,7 +1,10 @@
 module Paperclip
   class IdentityAdapter < AbstractAdapter
-    def new(adapter)
-      adapter
+    def initialize
+    end
+
+    def new(target, _)
+      target
     end
   end
 end

lib/paperclip/io_adapters/nil_adapter.rb

@@ -1,6 +1,6 @@
 module Paperclip
   class NilAdapter < AbstractAdapter
-    def initialize(target)
+    def initialize(_target, _options = {})
     end
 
     def original_filename

lib/paperclip/io_adapters/registry.rb

@@ -25,8 +25,8 @@ def registered?(target)
       end
     end
 
-    def for(target)
-      handler_for(target).new(target)
+    def for(target, options = {})
+      handler_for(target).new(target, options)
     end
   end
 end

lib/paperclip/io_adapters/stringio_adapter.rb

@@ -1,7 +1,7 @@
 module Paperclip
   class StringioAdapter < AbstractAdapter
-    def initialize(target)
-      @target = target
+    def initialize(target, options = {})
+      super
       cache_current_values
     end
 

lib/paperclip/io_adapters/uploaded_file_adapter.rb

@@ -1,7 +1,7 @@
 module Paperclip
   class UploadedFileAdapter < AbstractAdapter
-    def initialize(target)
-      @target = target
+    def initialize(target, options = {})
+      super
       cache_current_values
 
       if @target.respond_to?(:tempfile)

lib/paperclip/io_adapters/uri_adapter.rb

@@ -4,8 +4,8 @@ module Paperclip
   class UriAdapter < AbstractAdapter
     attr_writer :content_type
 
-    def initialize(target)
-      @target = target
+    def initialize(target, options = {})
+      super
       @content = download_content
       cache_current_values
       @tempfile = copy_to_tempfile(@content)

lib/tasks/paperclip.rake

@@ -64,7 +64,8 @@ namespace :paperclip do
       names = Paperclip::Task.obtain_attachments(klass)
       names.each do |name|
         Paperclip.each_instance_with_attachment(klass, name) do |instance|
-          if file = Paperclip.io_adapters.for(instance.send(name))
+          attachment = instance.send(name)
+          if file = Paperclip.io_adapters.for(attachment, attachment.options[:adapter_options])
             instance.send("#{name}_file_name=", instance.send("#{name}_file_name").strip)
             instance.send("#{name}_content_type=", file.content_type.to_s.strip)
             instance.send("#{name}_file_size=", file.size) if instance.respond_to?("#{name}_file_size")
@@ -90,6 +91,19 @@ namespace :paperclip do
       end
       Paperclip.save_current_attachments_styles!
     end
+
+    desc "Regenerates fingerprints for a given CLASS (and optional ATTACHMENT). Useful when changing digest."
+    task :fingerprints => :environment do
+      klass = Paperclip::Task.obtain_class
+      names = Paperclip::Task.obtain_attachments(klass)
+      names.each do |name|
+        Paperclip.each_instance_with_attachment(klass, name) do |instance|
+          attachment = instance.send(name)
+          attachment.assign(attachment)
+          instance.save(:validate => false)
+        end
+      end
+    end
   end
 
   desc "Cleans out invalid attachments. Useful after you've added new validations."
@@ -109,7 +123,7 @@ namespace :paperclip do
     end
   end
 
- desc "find missing attachments. Useful to know which attachments are broken"
+  desc "find missing attachments. Useful to know which attachments are broken"
   task :find_broken_attachments => :environment do
     klass = Paperclip::Task.obtain_class
     names = Paperclip::Task.obtain_attachments(klass)

spec/paperclip/attachment_spec.rb

@@ -1433,16 +1433,46 @@ def call(filename)
         assert_nothing_raised { @dummy.avatar = @file }
       end
 
-      it "returns the right value when sent #avatar_fingerprint" do
-        @dummy.avatar = @file
-        assert_equal 'aec488126c3b33c08a10c3fa303acf27', @dummy.avatar_fingerprint
+      context "with explicitly set digest" do
+        before do
+          rebuild_class adapter_options: { hash_digest: Digest::SHA256 }
+          @dummy = Dummy.new
+        end
+
+        it "returns the right value when sent #avatar_fingerprint" do
+          @dummy.avatar = @file
+          assert_equal "734016d801a497f5579cdd4ef2ae1d020088c1db754dc434482d76dd5486520a",
+                       @dummy.avatar_fingerprint
+        end
+
+        it "returns the right value when saved, reloaded, and sent #avatar_fingerprint" do
+          @dummy.avatar = @file
+          @dummy.save
+          @dummy = Dummy.find(@dummy.id)
+          assert_equal "734016d801a497f5579cdd4ef2ae1d020088c1db754dc434482d76dd5486520a",
+                       @dummy.avatar_fingerprint
+        end
       end
 
-      it "returns the right value when saved, reloaded, and sent #avatar_fingerprint" do
-        @dummy.avatar = @file
-        @dummy.save
-        @dummy = Dummy.find(@dummy.id)
-        assert_equal 'aec488126c3b33c08a10c3fa303acf27', @dummy.avatar_fingerprint
+      context "with the default digest" do
+        before do
+          rebuild_class # MD5 is the default
+          @dummy = Dummy.new
+        end
+
+        it "returns the right value when sent #avatar_fingerprint" do
+          @dummy.avatar = @file
+          assert_equal "aec488126c3b33c08a10c3fa303acf27",
+                       @dummy.avatar_fingerprint
+        end
+
+        it "returns the right value when saved, reloaded, and sent #avatar_fingerprint" do
+          @dummy.avatar = @file
+          @dummy.save
+          @dummy = Dummy.find(@dummy.id)
+          assert_equal "aec488126c3b33c08a10c3fa303acf27",
+                       @dummy.avatar_fingerprint
+        end
       end
     end
   end