How can I avoid accepting broken images in ActiveStorage?

Question

I am currently migrating a Rails application (that drives screenshots.debian.net) from Paperclip to ActiveStorage. It allows users to upload PNG images that are then shown to all users.

TL;DR: The upload form accepts broken images. Imagemagick "accepts" them. ActiveStorage does not instantly validate that. I end up with broken files on disk.

This is the model for the images. I am using the 'active_storage_validations' gem:

class Screenshot < ApplicationRecord
  has_one_attached :image
  validates :image, attached: true, content_type: [:png]

  def medium_image
    if self.image.attached?
      self.image.variant(
        resize_to_limit: [670, 600]
      ).processed
    end
  end
end

The controller logic that receives uploads from an HTML form with file field:

def upload_receive
  @package = Package.find_by!(name: params[:name])
  params[:file].each do |img|
    new_screenshot = @package.screenshots.new(image: img)
    
    if new_screenshot.valid?  
      new_screenshot.save
    end
  end
end

This works well. PNG files are accepted. JPG files are rejected. Nice.

However when I upload a deliberately broken file I run into trouble. I have created a file called "broken.png" that consists of 5 KB of zero bytes. ActiveStorage accepts that file so I end up with an invalid file.

Right after the upload the image is supposed to be shown. The Screenshot.medium_image method is called that tries to create a variant. ImageMagick tries to convert resize the broken image and fails:

convert /tmp/ActiveStorage-21670-… failed with error:
convert-im6.q16: improper image header `/tmp/ActiveStorage-21670-…' @ error/png.c/ReadPNGImage/4092.
convert-im6.q16: no images defined `/tmp/image_processing….png' @ error/convert.c/ConvertImageCommand/3258.

So once such a broken image was uploaded the application will fail every time it tries to display it. Quite an easy denial-of-service.

What I understand:

The file is accepted and then the ".save" method succeeds. The broken file has just been saved to disk.
Briefly after that ActiveStorage starts a background job to analyze the file: [ActiveJob] Enqueued ActiveStorage::AnalyzeJob (Job ID: 91055878-f516-4f2f-98ff-e39573980b45) to Async(active_storage_analysis) with arguments
The job runs and prints "Skipping image analysis because ImageMagick doesn't support the file" (Source: https://github.com/rails/rails/blob/fcb5f9035fd1307c300f4ab31fda353bd6365fc3/activestorage/lib/active_storage/analyzer/image_analyzer.rb#L39)
The job writes the metadata and flags the file as identified and analyzed. ActiveStorage::Blob Update (1.0ms) UPDATE "active_storage_blobs" SET "metadata" = $1 WHERE "active_storage_blobs"."id" = $2 [["metadata", "{\"identified\":true,\"analyzed\":true}"], ["id", 21670]]

What I actually wanted:

The if new_screenshot.valid? line runs validations on the file and finds that it is broken.
No ".save" will happen. An error message is added to the flash messages.

After hours of frustrations I would be so grateful for any hint. Is this a bug in ActiveStorage or the active_storage_validations gem? Thanks.

[I'm missing Paperclip a lot. ActiveStorage doesn't seem to offer the same functionality yet.]

Alexander Gorg Alexander Gorg · Accepted Answer · 2021-09-02T13:32:35

I guess you are just missing imagemagick.

On Ubuntu you can make the magic via

sudo apt install imagemagick

For another OS just google for an appropriate solution, fortunately, imagemagick is quite popular.

Also keep in mind that on some OS it could be changed to GraphicsMagick which is a fork of ImageMagick

How can I avoid accepting broken images in ActiveStorage?

1 Answers