Possible bug: validations run after we've calculated metadata on original image

#1

I’m not sure if it’s a bug or not, but sure seems like undesirable behavior.

Here’s my use-case:

  1. I’m uploading an image
  2. I generate a placeholder for the image and store it as metadata
  3. I unpack the placeholder to model using metadata_attributes

However, the metadata gets calculated before we’ve run the validations, which is critical in my case — it runs an exception if the data is not valid.

Here’s a script to reproduce it:

# frozen_string_literal: true

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem 'sequel'
  gem 'shrine'
  gem 'sqlite3'
  gem 'image_processing'
  gem 'mini_magick'
end

require 'sequel'
require 'mini_magick'
require 'image_processing'
require 'shrine'
require 'shrine/storage/memory'

Shrine.storages = {
  cache: Shrine::Storage::Memory.new,
  store: Shrine::Storage::Memory.new
}

Shrine.plugin :sequel

class MyUploader < Shrine
  plugin :add_metadata
  plugin :metadata_attributes, placeholder: :placeholder
  plugin :validation_helpers

  Attacher.validate do
    if validate_mime_type_inclusion(%w[image/jpg image/jpeg image/png])
      validate_max_width 20_000
      validate_max_height 20_000
    end
  end

  add_metadata :placeholder, skip_nil: true do |io, action:, **_context|
    placeholder = ::ImageProcessing::MiniMagick
                  .source(MiniMagick::Image.read(io))
                  .loader(page: 0)
                  .resize_to_fill(4, 4)
                  .convert('png')
    Base64.strict_encode64(placeholder)
  end
end

File.write('image.html', <<~HTML)
  <!doctype html />
  <html>
    <head></head>
    <body></body>
  </html>
HTML

DB = Sequel.sqlite # SQLite memory database
DB.create_table :posts do
  primary_key :id
  String :image_data
  String :image_placeholder
end

class Post < Sequel::Model
  include MyUploader::Attachment(:image)
end

post = Post.create(image: File.open('image.html'))

It’ll fail because HTML is not an image. I’ve tried changing order of the plugins to no avail.

There’s another problem: I’m using derivatives plugin, which won’t let me compute metadata for original unless action is cache. Which runs before validations.

If I compute the same metadata on a derivative, I won’t be able to use it with extract_metadata, as I understand from docs and my feeble attempts.

So here’s that.

Is it a bug that metadata gets calculated before validations? How do we solve it?

#2

If I understand correctly, the problem is that the metadata extractor relies on the incoming file being an image. The thing is, metadata extraction needs to happen before validation, because validation is validating the extracted metadata. This order cannot be changed.

In your case, I would recommend extracting and saving the placeholder metadata outside of the Shrine lifecycle. You can use Attacher#add_metadata the add_metadata plugin provides.

class MyUploader < Shrine
  plugin :add_metadata
  plugin :metadata_attributes, placeholder: :placeholder
  plugin :validation_helpers

  Attacher.validate do ... end

  metadata_method :placeholder # if you want the file method
end

class ImagePlaceholder
  def self.call(file)
    file = ImageProcessing::MiniMagick
      .source(file)
      .loader(page: 0)
      .resize_to_fill(4, 4)
      .convert('png')
      .call

    result = Base64.strict_encode64(file.read)

    file.close!

    result
  end
end

# ...

post = Post.new(image: image_file)

if post.valid?
  placeholder = post.image.download { |file| ImagePlaceholder.call(file) }
  post.image_attacher.add_metadata("placeholder" => placeholder)
  post.save
end