Remove the automatic loading of URI Adapters

Remove the URI adapters. Few people use them by default and they can
allow insight into the internal networks of the server. If you want to
enable them, add (for example) `Paperclip.DataUriAdapter.register` to
your `config/initializers/paperclip.rb` file.

This is related to CVE-2017-0889.

Elsewhere fix CI: it's `s3.us-west-2` now, with a dot.

.travis.yml

@@ -7,11 +7,6 @@ rvm:
   - 2.3
   - 2.4
 
-# FIXME: fails with modern bundler
-before_install:
-  - rvm @global do gem uninstall bundler -a -x
-  - rvm @global do gem install bundler -v 1.12.5
-
 script: "bundle exec rake clean spec cucumber"
 
 addons:

Gemfile

@@ -12,4 +12,5 @@ group :development, :test do
   gem 'mime-types'
   gem 'builder'
   gem 'rubocop', require: false
+  gem 'rspec'
 end

README.md

@@ -17,7 +17,6 @@ https://github.com/thoughtbot/paperclip/releases
 <!-- START doctoc generated TOC please keep comment here to allow auto update -->
 <!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
 
-
 - [Requirements](#requirements)
   - [Ruby and Rails](#ruby-and-rails)
   - [Image Processor](#image-processor)
@@ -43,6 +42,7 @@ https://github.com/thoughtbot/paperclip/releases
   - [Vintage Syntax](#vintage-syntax)
 - [Storage](#storage)
   - [Understanding Storage](#understanding-storage)
+- [IO Adapters](#io-adapters)
 - [Post Processing](#post-processing)
 - [Custom Attachment Processors](#custom-attachment-processors)
 - [Events](#events)
@@ -608,6 +608,34 @@ variables.
 
 ---
 
+IO Adapters
+-----------
+
+When a file is uploaded or attached, it can be in one of a few different input
+forms, from Rails' UploadedFile object to a StringIO to a Tempfile or even a
+simple String that is a URL that points to an image.
+
+Paperclip will accept, by default, many of these sources. It also is capable of
+handling even more with a little configuration. The IO Adapters that handle
+images from non-local sources are not enabled by default. They can be enabled by
+adding a line similar to the following into `config/initializers/paperclip.rb`:
+
+```ruby
+Paperclip::DataUriAdapter.register
+```
+
+It's best to only enable a remote-loading adapter if you need it. Otherwise
+there's a chance that someone can gain insight into your internal network
+structure using it as a vector.
+
+The following adapters are *not* loaded by default:
+
+* `Paperclip::UriAdapter` - which accepts a `URI` instance.
+* `Paperclip::HttpUrlProxyAdapter` - which accepts a `http` string.
+* `Paperclip::DataUriAdapter` - which accepts a Base64-encoded `data:` string.
+
+---
+
 Post Processing
 ---------------
 

Rakefile

@@ -9,7 +9,7 @@ task :default => [:clean, :all]
 desc 'Test the paperclip plugin under all supported Rails versions.'
 task :all do |t|
   if ENV['BUNDLE_GEMFILE']
-    exec('rake spec cucumber')
+    exec('rake spec && cucumber')
   else
     exec("rm -f gemfiles/*.lock")
     Rake::Task["appraisal:gemfiles"].execute

features/step_definitions/rails_steps.rb

@@ -22,6 +22,7 @@ Given /^I generate a new rails application$/ do
       gem "rubysl", :platform => :rbx
       """
     And I remove turbolinks
+    And I comment out lines that contain "action_mailer" in "config/environments/*.rb"
     And I empty the application.js file
     And I configure the application to use "paperclip" from this project
   }
@@ -49,6 +50,16 @@ Given "I remove turbolinks" do
   end
 end
 
+Given /^I comment out lines that contain "([^"]+)" in "([^"]+)"$/ do |contains, glob|
+  cd(".") do
+    Dir.glob(glob).each do |file|
+      transform_file(file) do |content|
+        content.gsub(/^(.*?#{contains}.*?)$/) { |line| "# #{line}" }
+      end
+    end
+  end
+end
+
 Given /^I attach :attachment$/ do
   attach_attachment("attachment")
 end
@@ -138,8 +149,10 @@ end
 
 Given /^I start the rails application$/ do
   cd(".") do
+    require "rails"
     require "./config/environment"
-    require "capybara/rails"
+    require "capybara"
+    Capybara.app = Rails.application
   end
 end
 

features/step_definitions/s3_steps.rb

 When /^I attach the file "([^"]*)" to "([^"]*)" on S3$/ do |file_path, field|
   definition = Paperclip::AttachmentRegistry.definitions_for(User)[field.downcase.to_sym]
-  path = "https://paperclip.s3-us-west-2.amazonaws.com#{definition[:path]}"
+  path = "https://paperclip.s3.us-west-2.amazonaws.com#{definition[:path]}"
   path.gsub!(':filename', File.basename(file_path))
   path.gsub!(/:([^\/\.]+)/) do |match|
     "([^\/\.]+)"

features/support/env.rb

@@ -7,5 +7,6 @@ $CUCUMBER=1
 World(RSpec::Matchers)
 
 Before do
+  aruba.config.command_launcher = ENV.fetch("DEBUG", nil) ? :debug : :spawn
   @aruba_timeout_seconds = 120
 end

gemfiles/4.2.gemfile

@@ -11,6 +11,7 @@ group :development, :test do
   gem "mime-types"
   gem "builder"
   gem "rubocop", :require => false
+  gem "rspec"
 end
 
 gemspec :path => "../"

gemfiles/5.0.gemfile

@@ -11,6 +11,7 @@ group :development, :test do
   gem "mime-types"
   gem "builder"
   gem "rubocop", :require => false
+  gem "rspec"
 end
 
 gemspec :path => "../"

lib/paperclip/io_adapters/attachment_adapter.rb

 module Paperclip
   class AttachmentAdapter < AbstractAdapter
+    def self.register
+      Paperclip.io_adapters.register self do |target|
+        Paperclip::Attachment === target || Paperclip::Style === target
+      end
+    end
+
     def initialize(target, options = {})
       super
       @target, @style = case target
@@ -32,6 +38,4 @@ module Paperclip
   end
 end
 
-Paperclip.io_adapters.register Paperclip::AttachmentAdapter do |target|
-  Paperclip::Attachment === target || Paperclip::Style === target
-end
+Paperclip::AttachmentAdapter.register

lib/paperclip/io_adapters/data_uri_adapter.rb

 module Paperclip
   class DataUriAdapter < StringioAdapter
+    def self.register
+      Paperclip.io_adapters.register self do |target|
+        String === target && target =~ REGEXP
+      end
+    end
 
     REGEXP = /\Adata:([-\w]+\/[-\w\+\.]+)?;base64,(.*)/m
 
@@ -11,12 +16,7 @@ module Paperclip
 
     def extract_target(uri)
       data_uri_parts = uri.match(REGEXP) || []
-      StringIO.new(Base64.decode64(data_uri_parts[2] || ''))
+      StringIO.new(Base64.decode64(data_uri_parts[2] || ""))
     end
-
   end
 end
-
-Paperclip.io_adapters.register Paperclip::DataUriAdapter do |target|
-  String === target && target =~ Paperclip::DataUriAdapter::REGEXP
-end

lib/paperclip/io_adapters/empty_string_adapter.rb

 module Paperclip
   class EmptyStringAdapter < AbstractAdapter
+    def self.register
+      Paperclip.io_adapters.register self do |target|
+        target.is_a?(String) && target.empty?
+      end
+    end
+
     def nil?
       false
     end
@@ -10,6 +16,4 @@ module Paperclip
   end
 end
 
-Paperclip.io_adapters.register Paperclip::EmptyStringAdapter do |target|
-  target.is_a?(String) && target.empty?
-end
+Paperclip::EmptyStringAdapter.register

lib/paperclip/io_adapters/file_adapter.rb

 module Paperclip
   class FileAdapter < AbstractAdapter
+    def self.register
+      Paperclip.io_adapters.register self do |target|
+        File === target || ::Tempfile === target
+      end
+    end
+
     def initialize(target, options = {})
       super
       cache_current_values
@@ -8,7 +14,9 @@ module Paperclip
     private
 
     def cache_current_values
-      self.original_filename = @target.original_filename if @target.respond_to?(:original_filename)
+      if @target.respond_to?(:original_filename)
+        self.original_filename = @target.original_filename
+      end
       self.original_filename ||= File.basename(@target.path)
       @tempfile = copy_to_tempfile(@target)
       @content_type = ContentTypeDetector.new(@target.path).detect
@@ -17,6 +25,4 @@ module Paperclip
   end
 end
 
-Paperclip.io_adapters.register Paperclip::FileAdapter do |target|
-  File === target || Tempfile === target
-end
+Paperclip::FileAdapter.register

lib/paperclip/io_adapters/http_url_proxy_adapter.rb

 module Paperclip
   class HttpUrlProxyAdapter < UriAdapter
+    def self.register
+      Paperclip.io_adapters.register self do |target|
+        String === target && target =~ REGEXP
+      end
+    end
 
     REGEXP = /\Ahttps?:\/\//
 
     def initialize(target, options = {})
       super(URI(URI.escape(target)), options)
     end
-
   end
 end
-
-Paperclip.io_adapters.register Paperclip::HttpUrlProxyAdapter do |target|
-  String === target && target =~ Paperclip::HttpUrlProxyAdapter::REGEXP
-end

lib/paperclip/io_adapters/identity_adapter.rb

 module Paperclip
   class IdentityAdapter < AbstractAdapter
+    def self.register
+      Paperclip.io_adapters.register Paperclip::IdentityAdapter.new do |target|
+        Paperclip.io_adapters.registered?(target)
+      end
+    end
+
     def initialize
     end
 
@@ -9,7 +15,4 @@ module Paperclip
   end
 end
 
-Paperclip.io_adapters.register Paperclip::IdentityAdapter.new do |target|
-  Paperclip.io_adapters.registered?(target)
-end
-
+Paperclip::IdentityAdapter.register

lib/paperclip/io_adapters/nil_adapter.rb

 module Paperclip
   class NilAdapter < AbstractAdapter
-    def initialize(_target, _options = {})
+    def self.register
+      Paperclip.io_adapters.register self do |target|
+        target.nil? || ((Paperclip::Attachment === target) && !target.present?)
       end
+    end
+
+    def initialize(_target, _options = {}); end
 
     def original_filename
       ""
@@ -19,7 +24,7 @@ module Paperclip
       true
     end
 
-    def read(*args)
+    def read(*_args)
       nil
     end
 
@@ -29,6 +34,4 @@ module Paperclip
   end
 end
 
-Paperclip.io_adapters.register Paperclip::NilAdapter do |target|
-  target.nil? || ( (Paperclip::Attachment === target) && !target.present? )
-end
+Paperclip::NilAdapter.register

lib/paperclip/io_adapters/registry.rb

@@ -12,6 +12,10 @@ module Paperclip
       @registered_handlers << [block, handler_class]
     end
 
+    def unregister(handler_class)
+      @registered_handlers.reject! { |_, klass| klass == handler_class }
+    end
+
     def handler_for(target)
       @registered_handlers.each do |tester, handler|
         return handler if tester.call(target)

lib/paperclip/io_adapters/stringio_adapter.rb

 module Paperclip
   class StringioAdapter < AbstractAdapter
+    def self.register
+      Paperclip.io_adapters.register self do |target|
+        StringIO === target
+      end
+    end
+
     def initialize(target, options = {})
       super
       cache_current_values
@@ -24,10 +30,7 @@ module Paperclip
       destination.rewind
       destination
     end
-
   end
 end
 
-Paperclip.io_adapters.register Paperclip::StringioAdapter do |target|
-  StringIO === target
-end
+Paperclip::StringioAdapter.register

lib/paperclip/io_adapters/uploaded_file_adapter.rb

 module Paperclip
   class UploadedFileAdapter < AbstractAdapter
+    def self.register
+      Paperclip.io_adapters.register self do |target|
+        target.class.name.include?("UploadedFile")
+      end
+    end
+
     def initialize(target, options = {})
       super
       cache_current_values
@@ -37,6 +43,4 @@ module Paperclip
   end
 end
 
-Paperclip.io_adapters.register Paperclip::UploadedFileAdapter do |target|
-  target.class.name.include?("UploadedFile")
-end
+Paperclip::UploadedFileAdapter.register

lib/paperclip/io_adapters/uri_adapter.rb

-require 'open-uri'
+require "open-uri"
 
 module Paperclip
   class UriAdapter < AbstractAdapter
     attr_writer :content_type
 
+    def self.register
+      Paperclip.io_adapters.register self do |target|
+        target.is_a?(URI)
+      end
+    end
+
     def initialize(target, options = {})
       super
       @content = download_content
@@ -28,9 +34,8 @@ module Paperclip
     end
 
     def filename_from_content_disposition
-      if @content.meta.has_key?("content-disposition")
-        matches = @content.meta["content-disposition"].
-          match(/filename="([^"]*)"/)
+      if @content.meta.key?("content-disposition")
+        matches = @content.meta["content-disposition"].match(/filename="([^"]*)"/)
         matches[1] if matches
       end
     end
@@ -50,7 +55,7 @@ module Paperclip
     end
 
     def copy_to_tempfile(src)
-      while data = src.read(16*1024)
+      while data = src.read(16 * 1024)
         destination.write(data)
       end
       src.close
@@ -59,7 +64,3 @@ module Paperclip
     end
   end
 end
-
-Paperclip.io_adapters.register Paperclip::UriAdapter do |target|
-  target.kind_of?(URI)
-end

spec/paperclip/io_adapters/data_uri_adapter_spec.rb

 require 'spec_helper'
 
 describe Paperclip::DataUriAdapter do
+  before do
+    Paperclip::DataUriAdapter.register
+  end
+
   after do
+    Paperclip.io_adapters.unregister(described_class)
+
     if @subject
       @subject.close
     end

spec/paperclip/io_adapters/http_url_proxy_adapter_spec.rb

@@ -7,6 +7,11 @@ describe Paperclip::HttpUrlProxyAdapter do
     @open_return.stubs(:meta).returns({})
     Paperclip::HttpUrlProxyAdapter.any_instance.
       stubs(:download_content).returns(@open_return)
+    Paperclip::HttpUrlProxyAdapter.register
+  end
+
+  after do
+    Paperclip.io_adapters.unregister(described_class)
   end
 
   context "a new instance" do

spec/paperclip/io_adapters/uri_adapter_spec.rb

@@ -8,6 +8,11 @@ describe Paperclip::UriAdapter do
     @open_return = StringIO.new("xxx")
     @open_return.stubs(:content_type).returns(content_type)
     @open_return.stubs(:meta).returns(meta)
+    Paperclip::UriAdapter.register
+  end
+
+  after do
+    Paperclip.io_adapters.unregister(described_class)
   end
 
   context "a new instance" do

spec/paperclip/storage/s3_spec.rb

@@ -256,7 +256,7 @@ describe Paperclip::Storage::S3 do
     end
 
     it "uses the S3 bucket with the correct host name" do
-      assert_equal "s3-ap-northeast-1.amazonaws.com",
+      assert_equal "s3.ap-northeast-1.amazonaws.com",
         @dummy.avatar.s3_bucket.client.config.endpoint.host
     end
   end
@@ -821,8 +821,9 @@ describe Paperclip::Storage::S3 do
 
     it "gets the right s3_host_name in development" do
       rails_env("development") do
-        assert_match %r{^s3-ap-northeast-1.amazonaws.com}, @dummy.avatar.s3_host_name
-        assert_match %r{^s3-ap-northeast-1.amazonaws.com},
+        assert_match %r{^s3.ap-northeast-1.amazonaws.com},
+          @dummy.avatar.s3_host_name
+        assert_match %r{^s3.ap-northeast-1.amazonaws.com},
           @dummy.avatar.s3_bucket.client.config.endpoint.host
       end
     end