diff --git a/lib/carrierwave/processing/rmagick.rb b/lib/carrierwave/processing/rmagick.rb index 129fb4de4..39223f124 100644 --- a/lib/carrierwave/processing/rmagick.rb +++ b/lib/carrierwave/processing/rmagick.rb @@ -378,9 +378,15 @@ def manipulate!(options={}, &block) def create_info_block(options) return nil unless options - assignments = options.map { |k, v| "self.#{k} = #{v}" } - code = "lambda { |img| " + assignments.join(";") + "}" - eval code + proc do |img| + options.each do |k, v| + if v.is_a?(String) && (matches = v.match(/^["'](.+)["']/)) + ActiveSupport::Deprecation.warn "Passing quoted strings like #{v} to #manipulate! is deprecated, pass them without quoting." + v = matches[1] + end + img.public_send(:"#{k}=", v) + end + end end def destroy_image(image) diff --git a/spec/processing/rmagick_spec.rb b/spec/processing/rmagick_spec.rb index 2c5d95fde..19037749b 100644 --- a/spec/processing/rmagick_spec.rb +++ b/spec/processing/rmagick_spec.rb @@ -208,9 +208,41 @@ instance.manipulate! :read => { :density => 10, - :size => %{"200x200"} + :size => "200x200" } end + + it 'shows deprecation but still accepts strings enclosed with double quotes' do + expect_any_instance_of(::Magick::Image::Info).to receive(:size=).once.with("200x200") + expect(ActiveSupport::Deprecation).to receive(:warn).with(any_args) + instance.manipulate! :read => {:size => %{"200x200"}} + end + + it 'shows deprecation but still accepts strings enclosed with single quotes' do + expect_any_instance_of(::Magick::Image::Info).to receive(:size=).once.with("200x200") + expect(ActiveSupport::Deprecation).to receive(:warn).with(any_args) + instance.manipulate! :read => {:size => %{'200x200'}} + end + + it 'does not allow arbitrary code execution' do + expect_any_instance_of(Kernel).not_to receive(:puts) + expect do + instance.manipulate! :read => { + :density => "1 }; raise; {" + } + end.to raise_error ArgumentError, /invalid density geometry/ + end + + it 'does not allow invocation of non-public methods' do + module Kernel + private def foo=(value); raise; end + end + expect do + instance.manipulate! :read => { + :foo => "1" + } + end.to raise_error NoMethodError, /private method `foo=' called/ + end end describe "#width and #height" do