Fix Paperclip's io_adapters to properly sanitize original_filename

The ":" is a special character in file paths passed to ImageMagick's
"identify" command. Even though Cocaine (and everything else in
paperclip) escapes special characters correctly, we need to strip it out
of filenames to ensure identify (and probably other ImageMagick
commands) can run correctly.

lib/paperclip/io_adapters/attachment_adapter.rb

@@ -14,7 +14,7 @@ module Paperclip
     private
     private
 
 
     def cache_current_values
     def cache_current_values
-      @original_filename = @target.original_filename
+      self.original_filename = @target.original_filename
       @content_type = @target.content_type
       @content_type = @target.content_type
       @tempfile = copy_to_tempfile(@target)
       @tempfile = copy_to_tempfile(@target)
       @size = @tempfile.size || @target.size
       @size = @tempfile.size || @target.size

lib/paperclip/io_adapters/file_adapter.rb

@@ -8,8 +8,8 @@ module Paperclip
     private
     private
 
 
     def cache_current_values
     def cache_current_values
-      @original_filename = @target.original_filename if @target.respond_to?(:original_filename)
-      @original_filename ||= File.basename(@target.path)
+      self.original_filename = @target.original_filename if @target.respond_to?(:original_filename)
+      self.original_filename ||= File.basename(@target.path)
       @tempfile = copy_to_tempfile(@target)
       @tempfile = copy_to_tempfile(@target)
       @content_type = ContentTypeDetector.new(@target.path).detect
       @content_type = ContentTypeDetector.new(@target.path).detect
       @size = File.size(@target)
       @size = File.size(@target)

lib/paperclip/io_adapters/stringio_adapter.rb

@@ -13,7 +13,7 @@ module Paperclip
     def cache_current_values
     def cache_current_values
       @original_filename = @target.original_filename if @target.respond_to?(:original_filename)
       @original_filename = @target.original_filename if @target.respond_to?(:original_filename)
       @original_filename ||= "stringio.txt"
       @original_filename ||= "stringio.txt"
-      @original_filename = @original_filename.strip
+      self.original_filename = @original_filename.strip
 
 
       @content_type = @target.content_type if @target.respond_to?(:content_type)
       @content_type = @target.content_type if @target.respond_to?(:content_type)
       @content_type ||= "text/plain"
       @content_type ||= "text/plain"

lib/paperclip/io_adapters/uploaded_file_adapter.rb

@@ -18,7 +18,7 @@ module Paperclip
     private
     private
 
 
     def cache_current_values
     def cache_current_values
-      @original_filename = @target.original_filename
+      self.original_filename = @target.original_filename
       @content_type = determine_content_type
       @content_type = determine_content_type
       @size = File.size(@target.path)
       @size = File.size(@target.path)
     end
     end

lib/paperclip/io_adapters/uri_adapter.rb

@@ -20,7 +20,7 @@ module Paperclip
     def cache_current_values
     def cache_current_values
       @original_filename = @target.path.split("/").last
       @original_filename = @target.path.split("/").last
       @original_filename ||= "index.html"
       @original_filename ||= "index.html"
-      @original_filename = @original_filename.strip
+      self.original_filename = @original_filename.strip
 
 
       @content_type = @content.content_type if @content.respond_to?(:content_type)
       @content_type = @content.content_type if @content.respond_to?(:content_type)
       @content_type ||= "text/html"
       @content_type ||= "text/html"

test/io_adapters/attachment_adapter_test.rb

@@ -56,6 +56,31 @@ class AttachmentAdapterTest < Test::Unit::TestCase
 
 
   end
   end
 
 
+  context "for a file with restricted characters in the name" do
+    setup do
+      file_contents = File.new(fixture_file("animated.gif"))
+      @file = StringIO.new(file_contents.read)
+      @file.stubs(:original_filename).returns('image:restricted.gif')
+      @file.binmode
+
+      @attachment.assign(@file)
+      @attachment.save
+      @subject = Paperclip.io_adapters.for(@attachment)
+    end
+
+    teardown do
+      @file.close
+    end
+
+    should "not generate paths that include restricted characters" do
+      assert_no_match /:/, @subject.path
+    end
+
+    should "not generate filenames that include restricted characters" do
+      assert_equal 'image_restricted.gif', @subject.original_filename
+    end
+  end
+
   context "for a style" do
   context "for a style" do
     setup do
     setup do
       @file = File.new(fixture_file("5k.png"))
       @file = File.new(fixture_file("5k.png"))

test/io_adapters/file_adapter_test.rb

@@ -84,6 +84,25 @@ class FileAdapterTest < Test::Unit::TestCase
       end
       end
     end
     end
 
 
+    context "filename with restricted characters" do
+      setup do
+        file_contents = File.new(fixture_file("animated.gif"))
+        @file = StringIO.new(file_contents.read)
+        @file.stubs(:original_filename).returns('image:restricted.gif')
+        @subject = Paperclip.io_adapters.for(@file)
+      end
+
+      teardown { @file.close }
+
+      should "not generate filenames that include restricted characters" do
+        assert_equal 'image_restricted.gif', @subject.original_filename
+      end
+
+      should "not generate paths that include restricted characters" do
+        assert_no_match /:/, @subject.path
+      end
+    end
+
     context "empty file" do
     context "empty file" do
       setup do
       setup do
         @file = Tempfile.new("file_adapter_test")
         @file = Tempfile.new("file_adapter_test")

test/io_adapters/stringio_adapter_test.rb

@@ -42,10 +42,20 @@ class StringioFileProxyTest < Test::Unit::TestCase
       assert_equal 'image/png', @subject.content_type
       assert_equal 'image/png', @subject.content_type
     end
     end
 
 
-    should 'accept an orgiginal_filename' do
+    should 'accept an original_filename' do
       @subject.original_filename = 'image.png'
       @subject.original_filename = 'image.png'
       assert_equal 'image.png', @subject.original_filename
       assert_equal 'image.png', @subject.original_filename
     end
     end
 
 
+    should "not generate filenames that include restricted characters" do
+      @subject.original_filename = 'image:restricted.png'
+      assert_equal 'image_restricted.png', @subject.original_filename
+    end
+
+    should "not generate paths that include restricted characters" do
+      @subject.original_filename = 'image:restricted.png'
+      assert_no_match /:/, @subject.path
+    end
+
   end
   end
 end
 end

test/io_adapters/uploaded_file_adapter_test.rb

@@ -52,6 +52,29 @@ class UploadedFileAdapterTest < Test::Unit::TestCase
       end
       end
     end
     end
 
 
+    context "with UploadedFile that has restricted characters" do
+      setup do
+        Paperclip::UploadedFileAdapter.content_type_detector = nil
+
+        class UploadedFile < OpenStruct; end
+        @file = UploadedFile.new(
+          :original_filename => "image:restricted.gif",
+          :content_type => "image/x-png-by-browser",
+          :head => "",
+          :path => fixture_file("5k.png")
+        )
+        @subject = Paperclip.io_adapters.for(@file)
+      end
+
+      should "not generate paths that include restricted characters" do
+        assert_no_match /:/, @subject.path
+      end
+
+      should "not generate filenames that include restricted characters" do
+        assert_equal 'image_restricted.gif', @subject.original_filename
+      end
+    end
+
     context "with UploadFile responding to #path" do
     context "with UploadFile responding to #path" do
       setup do
       setup do
         Paperclip::UploadedFileAdapter.content_type_detector = nil
         Paperclip::UploadedFileAdapter.content_type_detector = nil

test/io_adapters/uri_adapter_test.rb

@@ -83,4 +83,20 @@ class UriProxyTest < Test::Unit::TestCase
     end
     end
   end
   end
 
 
+  context "a url with restricted characters in the filename" do
+    setup do
+      Paperclip::UriAdapter.any_instance.stubs(:download_content).returns(StringIO.new("xxx"))
+      @uri = URI.parse("https://github.com/thoughtbot/paper:clip.jpg")
+      @subject = Paperclip.io_adapters.for(@uri)
+    end
+
+    should "not generate filenames that include restricted characters" do
+      assert_equal "paper_clip.jpg", @subject.original_filename
+    end
+
+    should "not generate paths that include restricted characters" do
+      assert_no_match /:/, @subject.path
+    end
+  end
+
 end
 end