uploads: fix corrupted image detection.

* Fix corrupted image detection. We were shelling out to vips and trying
  to grep for error messages, but the error message for jpeg files changed.
  Now we load the file in ruby vips, which raises an error on failure.

* Don't attempt to redownload corrupted images. If a download completes
  without any errors yet the downloaded file is corrupt, then something is
  wrong at the source and redownloading is unlikely to help. Let the
  upload fail and the user retry if necessary.

* Validate that all uploads are uncorrupted, including files uploaded
  from a computer, not just files uploaded from a source.

app/logical/danbooru_image_resizer.rb

@@ -35,19 +35,4 @@ module DanbooruImageResizer
 
     output_file
   end
-
-  def validate_shell(file)
-    temp = Tempfile.new("validate")
-    output, status = Open3.capture2e("vips stats #{file.path} #{temp.path}.v")
-
-    # png | jpeg | gif
-    if output =~ /Read Error|Premature end of JPEG file|Failed to read from given file/m
-      return false
-    end
-
-    return true
-  ensure
-    temp.close
-    temp.unlink
-  end
 end

app/logical/upload_service/utils.rb

@@ -2,8 +2,6 @@ class UploadService
   module Utils
     module_function
 
-    class CorruptFileError < RuntimeError; end
-
     def file_header_to_file_ext(file)
       case File.read(file.path, 16)
       when /^\xff\xd8/n
@@ -25,6 +23,14 @@ class UploadService
       end
     end
 
+    def corrupt?(filename)
+      image = Vips::Image.new_from_file(filename, fail: true)
+      image.stats
+      false
+    rescue Vips::Error
+      true
+    end
+
     def calculate_ugoira_dimensions(source_path)
       folder = Zip::File.new(source_path)
       Tempfile.open("ugoira-dim-") do |tempfile|
@@ -180,23 +186,8 @@ class UploadService
       return file if file.present?
       raise "No file or source URL provided" if upload.source_url.blank?
 
-      attempts = 0
+      download = Downloads::File.new(upload.source_url, upload.referer_url)
-
+      file, strategy = download.download!
-      begin
-        download = Downloads::File.new(upload.source_url, upload.referer_url)
-        file, strategy = download.download!
-
-        if !DanbooruImageResizer.validate_shell(file)
-          raise CorruptFileError.new("File is corrupted")
-        end
-      rescue StandardError
-        if attempts == 3
-          raise
-        end
-
-        attempts += 1
-        retry
-      end
 
       if download.data[:ugoira_frame_data].present?
         upload.context = {

app/models/upload.rb

@@ -6,6 +6,7 @@ class Upload < ApplicationRecord
   class FileValidator < ActiveModel::Validator
     def validate(record)
       validate_file_ext(record)
+      validate_integrity(record)
       validate_md5_uniqueness(record)
       validate_video_duration(record)
       validate_resolution(record)
@@ -17,6 +18,12 @@ class Upload < ApplicationRecord
       end
     end
 
+    def validate_integrity(record)
+      if record.file_ext.in?(["jpg", "gif", "png"]) && UploadService::Utils.corrupt?(record.file.path)
+        record.errors[:file] << "File is corrupted"
+      end
+    end
+
     def validate_md5_uniqueness(record)
       if record.md5.nil?
         return

test/test_helpers/upload_test_helper.rb

@@ -1,4 +1,8 @@
 module UploadTestHelper
+  def upload_from_file(filepath)
+    UploadService.new(file: upload_file(filepath)).start!
+  end
+
   def upload_file(path)
     file = Tempfile.new(binmode: true)
     IO.copy_stream("#{Rails.root}/#{path}", file.path)

test/unit/upload_service_test.rb

@@ -34,28 +34,6 @@ class UploadServiceTest < ActiveSupport::TestCase
         end
       end
 
-      context "for a corrupt jpeg" do
-        setup do
-          @source = "https://cdn.donmai.us/original/93/f4/93f4dd66ef1eb11a89e56d31f9adc8d0.jpg"
-          @mock_upload = mock("upload")
-          @mock_upload.stubs(:source_url).returns(@source)
-          @mock_upload.stubs(:referer_url).returns(nil)
-          @bad_file = File.open("#{Rails.root}/test/files/test-corrupt.jpg", "rb")
-          Downloads::File.any_instance.stubs(:download!).returns([@bad_file, nil])
-        end
-
-        teardown do
-          @bad_file.close
-        end
-
-        should "retry three times" do
-          DanbooruImageResizer.expects(:validate_shell).times(4).returns(false)
-          assert_raise(UploadService::Utils::CorruptFileError) do
-            subject.get_file_for_upload(@mock_upload)
-          end
-        end
-      end
-
       context "for a pixiv" do
         setup do
           @source = "http://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247350"
@@ -1133,6 +1111,33 @@ class UploadServiceTest < ActiveSupport::TestCase
         assert_equal("http://www.example.com", @upload.source)
       end
     end
+
+    context "for a corrupted image" do
+      should "fail for a corrupted jpeg" do
+        @bad_jpeg_path = "test/files/test-corrupt.jpg"
+
+        upload = upload_from_file(@bad_jpeg_path)
+        assert_match(/corrupt/, upload.status)
+        assert_equal(true, UploadService::Utils.corrupt?(@bad_jpeg_path))
+      end
+
+      should "fail for a corrupted gif" do
+        @bad_gif_path = "test/files/test-corrupt.gif"
+
+        upload = upload_from_file(@bad_gif_path)
+        assert_match(/corrupt/, upload.status)
+        assert_equal(true, UploadService::Utils.corrupt?(@bad_gif_path))
+      end
+
+      # https://schaik.com/pngsuite/pngsuite_xxx_png.html
+      should "fail for a corrupted png" do
+        @bad_png_path = "test/files/test-corrupt.png"
+
+        upload = upload_from_file(@bad_png_path)
+        assert_match(/corrupt/, upload.status)
+        assert_equal(true, UploadService::Utils.corrupt?(@bad_png_path))
+      end
+    end
   end
 
   context "#create_post_from_upload" do