Skip to content

Commit

Permalink
feat: explicitly raise error when trying to use unsupported operation
Browse files Browse the repository at this point in the history
That should prevent accidental, silent omission of a desired transformation
  • Loading branch information
knarewski committed Dec 12, 2024
1 parent 0c5f3b2 commit 181bc3a
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Vips straighten
- Vips gamma
- Vips stripping alpha
- Explicit error when trying to use VipsProcessor with unsupported options

### Removed
- [BREAKING] dropped support for a broken 'dominant' border colour
Expand Down
6 changes: 5 additions & 1 deletion lib/morandi.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,14 @@ module Morandi
# @param target_path [String] target location for image
# @param local_options [Hash] Hash of options other than desired transformations
# @option local_options [String] 'path.icc' A path to store the input after converting to sRGB colour space
# @options local_options [String] 'processor' ('pixbuf') Name of the image processing library ('pixbuf', 'vips')
# @option local_options [String] 'processor' ('pixbuf') Name of the image processing library ('pixbuf', 'vips')
# NOTE: vips processor only handles subset of operations,
# see `Morandi::VipsImageProcessor.supports?` for details
def process(source, options, target_path, local_options = {})
case local_options['processor']
when 'vips'
raise(ArgumentError, 'Requested unsupported Vips operation') unless VipsImageProcessor.supports?(source, options)

# Cache saves time in expense of RAM when performing the same processing multiple times
# Cache is also created for files based on their names, which can lead to leaking files data, so in terms
# of security it feels prudent to disable it. Latest libvips supports "revalidate" option to prevent that risk
Expand Down
12 changes: 12 additions & 0 deletions lib/morandi/vips_image_processor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ class VipsImageProcessor
}.freeze
SUPPORTED_FILTERS = COLOUR_FILTER_MODIFIERS.keys + ['greyscale']

def self.supports?(input, options)
return false unless input.is_a?(String)
return false if options['brighten'].to_f != 0
return false if options['contrast'].to_f != 0
return false if options['sharpen'].to_f != 0
return false if options['redeye']&.any?
return false if options['border-style']
return false if options['background-style']

true
end

# Vips options are global, this method sets them for yielding, then restores to original
def self.with_global_options(cache_max:, concurrency:)
previous_cache_max = Vips.cache_max
Expand Down
29 changes: 29 additions & 0 deletions spec/morandi_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,35 @@
end

context 'vips processor' do
subject(:process_image) do
Morandi.process(file_arg, options, file_out, 'processor' => 'vips')
end

it_behaves_like 'an image processor', 'vips'

context 'with pixbuf object given as input' do
let(:file_arg) { GdkPixbuf::Pixbuf.new(file: file_in) }

it 'raises ArgumentError' do
expect { process_image }.to raise_error(ArgumentError, 'Requested unsupported Vips operation')
end
end

{
'brighten' => 1.0,
'contrast' => 1.0,
'sharpen' => 1.0,
'redeye' => [[540, 650]],
'border-style' => 'square',
'background-style' => 'retro'
}.each do |option_name, option_value|
context "with unsupported operation `#{option_name}` requested" do
let(:options) { { option_name => option_value } }

it 'raises ArgumentError' do
expect { process_image }.to raise_error(ArgumentError, 'Requested unsupported Vips operation')
end
end
end
end
end

0 comments on commit 181bc3a

Please sign in to comment.