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

 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 @@ module Paperclip
         :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 @@ module Paperclip
     # 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 @@ module Paperclip
       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 @@ module Paperclip
     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

 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 @@ module Paperclip
 
     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

 module Paperclip
   class EmptyStringAdapter < AbstractAdapter
-    def initialize(target)
-    end
-
     def nil?
       false
     end

lib/paperclip/io_adapters/file_adapter.rb

 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 @@ module Paperclip
 
     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

 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

 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 @@ module Paperclip
       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

 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

 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 @@ describe Paperclip::Attachment do
         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

spec/paperclip/io_adapters/abstract_adapter_spec.rb

@@ -9,70 +9,93 @@ describe Paperclip::AbstractAdapter do
     end
   end
 
+  subject { TestAdapter.new(nil) }
+
   context "content type from file contents" do
     before do
-      @adapter = TestAdapter.new
-      @adapter.stubs(:path).returns("image.png")
+      subject.stubs(:path).returns("image.png")
       Paperclip.stubs(:run).returns("image/png\n")
       Paperclip::ContentTypeDetector.any_instance.stubs(:type_from_mime_magic).returns("image/png")
     end
 
     it "returns the content type without newline" do
-      assert_equal "image/png", @adapter.content_type
+      assert_equal "image/png", subject.content_type
     end
   end
 
   context "nil?" do
     it "returns false" do
-      assert !TestAdapter.new.nil?
+      assert !subject.nil?
     end
   end
 
   context "delegation" do
     before do
-      @adapter = TestAdapter.new
-      @adapter.tempfile = stub("Tempfile")
+      subject.tempfile = stub("Tempfile")
     end
 
     [:binmode, :binmode?, :close, :close!, :closed?, :eof?, :path, :readbyte, :rewind, :unlink].each do |method|
       it "delegates #{method} to @tempfile" do
-        @adapter.tempfile.stubs(method)
-        @adapter.public_send(method)
-        assert_received @adapter.tempfile, method
+        subject.tempfile.stubs(method)
+        subject.public_send(method)
+        assert_received subject.tempfile, method
       end
     end
   end
 
   it 'gets rid of slashes and colons in filenames' do
-    @adapter = TestAdapter.new
-    @adapter.original_filename = "awesome/file:name.png"
+    subject.original_filename = "awesome/file:name.png"
 
-    assert_equal "awesome_file_name.png", @adapter.original_filename
+    assert_equal "awesome_file_name.png", subject.original_filename
   end
 
   it 'is an assignment' do
-    assert TestAdapter.new.assignment?
+    assert subject.assignment?
   end
 
   it 'is not nil' do
-    assert !TestAdapter.new.nil?
+    assert !subject.nil?
   end
 
   it "generates a destination filename with no original filename" do
-    @adapter = TestAdapter.new
-    expect(@adapter.send(:destination).path).to_not be_nil
+    expect(subject.send(:destination).path).to_not be_nil
   end
 
   it 'uses the original filename to generate the tempfile' do
-    @adapter = TestAdapter.new
-    @adapter.original_filename = "file.png"
-    expect(@adapter.send(:destination).path).to end_with(".png")
+    subject.original_filename = "file.png"
+    expect(subject.send(:destination).path).to end_with(".png")
+  end
+
+  context "generates a fingerprint" do
+    subject { TestAdapter.new(nil, options) }
+
+    before do
+      subject.stubs(:path).returns(fixture_file("50x50.png"))
+    end
+
+    context "MD5" do
+      let(:options) { { hash_digest: Digest::MD5 } }
+
+      it "returns a fingerprint" do
+        expect(subject.fingerprint).to be_a String
+        expect(subject.fingerprint).to eq "a790b00c9b5d58a8fd17a1ec5a187129"
+      end
+    end
+
+    context "SHA256" do
+      let(:options) { { hash_digest: Digest::SHA256 } }
+
+      it "returns a fingerprint" do
+        expect(subject.fingerprint).to be_a String
+        expect(subject.fingerprint).
+          to eq "243d7ce1099719df25f600f1c369c629fb979f88d5a01dbe7d0d48c8e6715bb1"
+      end
+    end
   end
 
   context "#original_filename=" do
     it "should not fail with a nil original filename" do
-      adapter = TestAdapter.new
-      expect{ adapter.original_filename = nil }.not_to raise_error
+      expect { subject.original_filename = nil }.not_to raise_error
     end
   end
 end

spec/paperclip/io_adapters/attachment_adapter_spec.rb

@@ -13,7 +13,8 @@ describe Paperclip::AttachmentAdapter do
 
       @attachment.assign(@file)
       @attachment.save
-      @subject = Paperclip.io_adapters.for(@attachment)
+      @subject = Paperclip.io_adapters.for(@attachment,
+                                           hash_digest: Digest::MD5)
     end
 
     after do
@@ -65,7 +66,8 @@ describe Paperclip::AttachmentAdapter do
 
       @attachment.assign(@file)
       @attachment.save
-      @subject = Paperclip.io_adapters.for(@attachment)
+      @subject = Paperclip.io_adapters.for(@attachment,
+                                           hash_digest: Digest::MD5)
     end
 
     after do
@@ -92,7 +94,8 @@ describe Paperclip::AttachmentAdapter do
       FileUtils.cp @attachment.queued_for_write[:thumb].path, @thumb.path
 
       @attachment.save
-      @subject = Paperclip.io_adapters.for(@attachment.styles[:thumb])
+      @subject = Paperclip.io_adapters.for(@attachment.styles[:thumb],
+                                           hash_digest: Digest::MD5)
     end
 
     after do

spec/paperclip/io_adapters/data_uri_adapter_spec.rb

@@ -20,7 +20,7 @@ describe Paperclip::DataUriAdapter do
   context "a new instance" do
     before do
       @contents = "data:image/png;base64,#{original_base64_content}"
-      @subject = Paperclip.io_adapters.for(@contents)
+      @subject = Paperclip.io_adapters.for(@contents, hash_digest: Digest::MD5)
     end
 
     it "returns a nondescript file name" do

spec/paperclip/io_adapters/file_adapter_spec.rb

@@ -15,7 +15,7 @@ describe Paperclip::FileAdapter do
 
       context 'doing normal things' do
         before do
-          @subject = Paperclip.io_adapters.for(@file)
+          @subject = Paperclip.io_adapters.for(@file, hash_digest: Digest::MD5)
         end
 
         it 'uses the original filename to generate the tempfile' do
@@ -61,7 +61,7 @@ describe Paperclip::FileAdapter do
       context "file with multiple possible content type" do
         before do
           MIME::Types.stubs(:type_for).returns([MIME::Type.new('image/x-png'), MIME::Type.new('image/png')])
-          @subject = Paperclip.io_adapters.for(@file)
+          @subject = Paperclip.io_adapters.for(@file, hash_digest: Digest::MD5)
         end
 
         it "prefers officially registered mime type" do

spec/paperclip/io_adapters/http_url_proxy_adapter_spec.rb

@@ -12,7 +12,7 @@ describe Paperclip::HttpUrlProxyAdapter do
   context "a new instance" do
     before do
       @url = "http://thoughtbot.com/images/thoughtbot-logo.png"
-      @subject = Paperclip.io_adapters.for(@url)
+      @subject = Paperclip.io_adapters.for(@url, hash_digest: Digest::MD5)
     end
 
     after do

spec/paperclip/io_adapters/identity_adapter_spec.rb

@@ -3,6 +3,6 @@ require 'spec_helper'
 describe Paperclip::IdentityAdapter do
   it "responds to #new by returning the argument" do
     adapter = Paperclip::IdentityAdapter.new
-    assert_equal :target, adapter.new(:target)
+    assert_equal :target, adapter.new(:target, nil)
   end
 end

spec/paperclip/io_adapters/registry_spec.rb

@@ -4,7 +4,7 @@ describe Paperclip::AttachmentRegistry do
   context "for" do
     before do
       class AdapterTest
-        def initialize(target); end
+        def initialize(_target, _ = {}); end
       end
       @subject = Paperclip::AdapterRegistry.new
       @subject.register(AdapterTest){|t| Symbol === t }
@@ -18,7 +18,7 @@ describe Paperclip::AttachmentRegistry do
   context "registered?" do
     before do
       class AdapterTest
-        def initialize(target); end
+        def initialize(_target, _ = {}); end
       end
       @subject = Paperclip::AdapterRegistry.new
       @subject.register(AdapterTest){|t| Symbol === t }

spec/paperclip/io_adapters/stringio_adapter_spec.rb

@@ -5,7 +5,7 @@ describe Paperclip::StringioAdapter do
     before do
       @contents = "abc123"
       @stringio = StringIO.new(@contents)
-      @subject = Paperclip.io_adapters.for(@stringio)
+      @subject = Paperclip.io_adapters.for(@stringio, hash_digest: Digest::MD5)
     end
 
     it "returns a file name" do

spec/paperclip/io_adapters/uploaded_file_adapter_spec.rb

@@ -17,7 +17,7 @@ describe Paperclip::UploadedFileAdapter do
           tempfile: tempfile,
           path: tempfile.path
         )
-        @subject = Paperclip.io_adapters.for(@file)
+        @subject = Paperclip.io_adapters.for(@file, hash_digest: Digest::MD5)
       end
 
       it "gets the right filename" do
@@ -63,7 +63,7 @@ describe Paperclip::UploadedFileAdapter do
           head: "",
           path: fixture_file("5k.png")
         )
-        @subject = Paperclip.io_adapters.for(@file)
+        @subject = Paperclip.io_adapters.for(@file, hash_digest: Digest::MD5)
       end
 
       it "does not generate paths that include restricted characters" do
@@ -86,7 +86,7 @@ describe Paperclip::UploadedFileAdapter do
           head: "",
           path: fixture_file("5k.png")
         )
-        @subject = Paperclip.io_adapters.for(@file)
+        @subject = Paperclip.io_adapters.for(@file, hash_digest: Digest::MD5)
       end
 
       it "gets the right filename" do

spec/paperclip/io_adapters/uri_adapter_spec.rb

@@ -16,7 +16,7 @@ describe Paperclip::UriAdapter do
         stubs(:download_content).returns(@open_return)
 
       @uri = URI.parse("http://thoughtbot.com/images/thoughtbot-logo.png")
-      @subject = Paperclip.io_adapters.for(@uri)
+      @subject = Paperclip.io_adapters.for(@uri, hash_digest: Digest::MD5)
     end
 
     it "returns a file name" do

spec/paperclip/storage/s3_spec.rb

@@ -458,7 +458,7 @@ describe Paperclip::Storage::S3 do
           "question?mark.png"
         end
       end
-      file = Paperclip.io_adapters.for(stringio)
+      file = Paperclip.io_adapters.for(stringio, hash_digest: Digest::MD5)
       @dummy = Dummy.new
       @dummy.avatar = file
       @dummy.save