From 11b7bcac912e17ed4abb04fa21588ef53994d1d7 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Sat, 29 Jan 2022 04:38:47 -0600
Subject: [PATCH] uploads: fix broken tests.

* Fix broken upload tests.
* Fix uploads to return an error if both a file and a source are given
  at the same time, or if neither are given. Also fix the error message
  in this case so that it doesn't include "base" at the start of the string.
* Fix uploads to percent-encode any Unicode characters in the source URL.
* Add a max filesize validation to media assets.
---
 .../file_upload_component.js                  |   8 +-
 app/models/media_asset.rb                     |   3 +-
 app/models/upload.rb                          |  40 +-
 test/factories/upload.rb                      |  52 +--
 test/factories/upload_media_asset.rb          |   6 +
 test/functional/posts_controller_test.rb      |  34 ++
 test/functional/uploads_controller_test.rb    | 318 +++++++--------
 test/test_helpers/upload_test_helper.rb       |  36 +-
 test/unit/upload_service_test.rb              | 362 ------------------
 9 files changed, 248 insertions(+), 611 deletions(-)
 create mode 100644 test/factories/upload_media_asset.rb

diff --git a/app/components/file_upload_component/file_upload_component.js b/app/components/file_upload_component/file_upload_component.js
index 2602798ef..2f354ba8c 100644
--- a/app/components/file_upload_component/file_upload_component.js
+++ b/app/components/file_upload_component/file_upload_component.js
@@ -110,7 +110,13 @@ export default class FileUploadComponent {
   async onError(e) {
     let errors = e.originalEvent.detail[0].errors;
     let message = Object.keys(errors).map(attribute => {
-      return errors[attribute].map(error => `${capitalize(attribute)} ${error}`);
+      return errors[attribute].map(error => {
+        if (attribute === "base") {
+          return `${error}`;
+        } else {
+          return `${capitalize(attribute)} ${error}`;
+        }
+      });
     }).join("; ");
 
     Utility.error(message);
diff --git a/app/models/media_asset.rb b/app/models/media_asset.rb
index 694c20dee..1987d182a 100644
--- a/app/models/media_asset.rb
+++ b/app/models/media_asset.rb
@@ -35,6 +35,7 @@ class MediaAsset < ApplicationRecord
 
   validates :md5, uniqueness: { conditions: -> { where(status: [:processing, :active]) } }
   validates :file_ext, inclusion: { in: %w[jpg png gif mp4 webm swf zip], message: "Not an image or video" }
+  validates :file_size, numericality: { less_than_or_equal_to: Danbooru.config.max_file_size }
   validates :duration, numericality: { less_than_or_equal_to: MAX_VIDEO_DURATION, message: "must be less than #{MAX_VIDEO_DURATION} seconds", allow_nil: true }, on: :create # XXX should allow admins to bypass
   validate :validate_resolution, on: :create
 
@@ -228,7 +229,7 @@ class MediaAsset < ApplicationRecord
         # failed state, then mark the asset as failed so the user can try the upload again later.
         if !media_asset.active?
           media_asset.update!(status: :failed)
-          raise Error, "Upload failed, try again (upload was stuck in 'processing' state)"
+          raise Error, "Upload failed, try again (timed out while waiting for file to be processed)"
         end
 
         media_asset
diff --git a/app/models/upload.rb b/app/models/upload.rb
index bb1d5ffa1..adecf3e83 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -11,8 +11,11 @@ class Upload < ApplicationRecord
   has_many :upload_media_assets, dependent: :destroy
   has_many :media_assets, through: :upload_media_assets
 
+  normalize :source, :normalize_source
+
   validates :source, format: { with: %r{\Ahttps?://}i, message: "is not a valid URL" }, if: -> { source.present? }
   validates :referer_url, format: { with: %r{\Ahttps?://}i, message: "is not a valid URL" }, if: -> { referer_url.present? }
+  validate :validate_file_and_source, on: :create
 
   after_create :async_process_upload!
 
@@ -46,6 +49,31 @@ class Upload < ApplicationRecord
     end
   end
 
+  concerning :ValidationMethods do
+    def validate_file_and_source
+      if file.present? && source.present?
+        errors.add(:base, "Can't give both a file and a source")
+      elsif file.blank? && source.blank?
+        errors.add(:base, "No file or source given")
+      end
+    end
+  end
+
+  concerning :SourceMethods do
+    class_methods do
+      # percent-encode unicode characters in the URL
+      def normalize_source(url)
+        return nil if url.nil?
+        Addressable::URI.normalized_encode(url)
+      end
+    end
+
+    def source_strategy
+      return nil if source.blank?
+      Sources::Strategies.find(source, referer_url)
+    end
+  end
+
   def self.search(params)
     q = search_attributes(params, :id, :created_at, :updated_at, :source, :referer_url, :uploader, :status, :backtrace, :upload_media_assets, :media_assets)
     q.apply_default_order(params)
@@ -54,8 +82,10 @@ class Upload < ApplicationRecord
   def async_process_upload!
     if file.present?
       ProcessUploadJob.perform_now(self)
-    else
+    elsif source.present?
       ProcessUploadJob.perform_later(self)
+    else
+      raise "No file or source given" # Should never happen
     end
   end
 
@@ -68,19 +98,13 @@ class Upload < ApplicationRecord
       strategy = Sources::Strategies.find(source, referer_url)
       media_file = strategy.download_file!(strategy.image_url)
     else
-      raise "No file or source provided"
+      raise "No file or source given" # Should never happen
     end
 
     media_asset = MediaAsset.upload!(media_file)
     update!(media_assets: [media_asset], status: "completed")
   rescue Exception => e
     update!(status: "error: #{e.message}", backtrace: e.backtrace.join("\n"))
-    raise
-  end
-
-  def source_strategy
-    return nil if source.blank?
-    Sources::Strategies.find(source, referer_url)
   end
 
   def self.available_includes
diff --git a/test/factories/upload.rb b/test/factories/upload.rb
index 4cd79a9e7..9dc81962c 100644
--- a/test/factories/upload.rb
+++ b/test/factories/upload.rb
@@ -1,45 +1,25 @@
-require 'fileutils'
-
 FactoryBot.define do
   factory(:upload) do
-    rating {"s"}
-    uploader :factory => :user, :level => 20
-    uploader_ip_addr {"127.0.0.1"}
-    tag_string {"special"}
-    status {"pending"}
-    server {Socket.gethostname}
-    source {"xxx"}
-    md5 { SecureRandom.hex(32) }
-    media_asset { build(:media_asset) }
+    uploader factory: :user
+    uploader_ip_addr { "127.0.0.1" }
+    status { "pending" }
+    rating { "s" }
+    tag_string { "" }
+    source { "https://files.catbox.moe/om3tcw.webm" }
 
-    factory(:source_upload) do
-      source {"http://www.google.com/intl/en_ALL/images/logo.gif"}
+    factory(:completed_source_upload) do
+      status { "completed" }
+      source { "https://cdn.donmai.us/original/d3/4e/d34e4cf0a437a5d65f8e82b7bcd02606.jpg" }
+      upload_media_assets { [build(:upload_media_asset)] }
     end
 
-    factory(:ugoira_upload) do
-      file do
-        f = Tempfile.new
-        IO.copy_stream("#{Rails.root}/test/files/ugoira.zip", f.path)
-        ActionDispatch::Http::UploadedFile.new(tempfile: f, filename: "ugoira.zip")
-      end
-    end
+    factory(:completed_file_upload) do
+      status { "completed" }
+      source { nil }
+      file { Rack::Test::UploadedFile.new("#{Rails.root}/test/files/test.jpg") }
 
-    factory(:jpg_upload) do
-      file do
-        f = Tempfile.new
-        IO.copy_stream("#{Rails.root}/test/files/test.jpg", f.path)
-        ActionDispatch::Http::UploadedFile.new(tempfile: f, filename: "test.jpg")
-      end
-
-      md5 { MediaFile.open("test/files/test.jpg").md5 }
-      media_asset { build(:media_asset, file: "test/files/test.jpg") }
-    end
-
-    factory(:large_jpg_upload) do
-      file do
-        f = Tempfile.new
-        IO.copy_stream("#{Rails.root}/test/files/test-large.jpg", f.path)
-        ActionDispatch::Http::UploadedFile.new(tempfile: f, filename: "test.jpg")
+      upload_media_assets do
+        [build(:upload_media_asset, media_asset: build(:media_asset, file: "test/files/test.jpg"))]
       end
     end
   end
diff --git a/test/factories/upload_media_asset.rb b/test/factories/upload_media_asset.rb
new file mode 100644
index 000000000..6552fdc3c
--- /dev/null
+++ b/test/factories/upload_media_asset.rb
@@ -0,0 +1,6 @@
+FactoryBot.define do
+  factory(:upload_media_asset) do
+    upload
+    media_asset
+  end
+end
diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb
index 1ee0e4d94..a781ee890 100644
--- a/test/functional/posts_controller_test.rb
+++ b/test/functional/posts_controller_test.rb
@@ -5,6 +5,14 @@ class PostsControllerTest < ActionDispatch::IntegrationTest
     assert_equal(expected, response.parsed_body.css("link[rel=canonical]").attribute("href").value)
   end
 
+  def create_post!(user: create(:user), rating: "q", tag_string: "tagme", **params)
+    upload = build(:upload, uploader: user)
+    asset = create(:upload_media_asset, upload: upload)
+    post_auth posts_path, user, params: { post: { upload_media_asset_id: asset.id, rating: rating, tag_string: tag_string, **params }}
+
+    Post.last
+  end
+
   context "The posts controller" do
     setup do
       @user = travel_to(1.month.ago) {create(:user)}
@@ -638,6 +646,32 @@ class PostsControllerTest < ActionDispatch::IntegrationTest
       end
     end
 
+    context "create action" do
+      should "autoban the post when it is tagged banned_artist" do
+        @post = create_post!(tag_string: "banned_artist")
+        assert_equal(true, @post.is_banned?)
+      end
+
+      should "autoban the post if it is tagged paid_reward" do
+        @post = create_post!(tag_string: "paid_reward")
+        assert_equal(true, @post.is_banned?)
+      end
+
+      should "not create a post when the uploader is upload-limited" do
+        @user = create(:user, upload_points: 0)
+
+        @user.upload_limit.upload_slots.times do
+          assert_difference("Post.count", 1) do
+            create_post!(user: @user)
+          end
+        end
+
+        assert_no_difference("Post.count") do
+          create_post!(user: @user)
+        end
+      end
+    end
+
     context "update action" do
       should "work" do
         put_auth post_path(@post), @user, params: {:post => {:tag_string => "bbb"}}
diff --git a/test/functional/uploads_controller_test.rb b/test/functional/uploads_controller_test.rb
index 84dd4eaa0..4961a754a 100644
--- a/test/functional/uploads_controller_test.rb
+++ b/test/functional/uploads_controller_test.rb
@@ -3,7 +3,7 @@ require 'test_helper'
 class UploadsControllerTest < ActionDispatch::IntegrationTest
   context "The uploads controller" do
     setup do
-      @user = create(:contributor_user, name: "marisa")
+      @user = create(:user)
     end
 
     context "image proxy action" do
@@ -48,218 +48,171 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest
         assert_response :success
       end
 
-      context "with a url" do
-        should "preprocess" do
-          assert_difference(-> { Upload.count }) do
-            get_auth new_upload_path, @user, params: {:url => "https://cdn.donmai.us/original/d3/4e/d34e4cf0a437a5d65f8e82b7bcd02606.jpg"}
-            perform_enqueued_jobs
-            assert_response :success
-          end
-        end
-      end
-
-      context "for a direct link twitter post" do
-        setup do
-          skip "Twitter credentials not configured" unless Sources::Strategies::Twitter.enabled?
-
-          @ref = "https://twitter.com/onsen_musume_jp/status/865534101918330881"
-          @source = "https://pbs.twimg.com/media/DAL-ntWV0AEbhes.jpg:orig"
-        end
-
-        should "trigger the preprocessor" do
-          assert_difference(-> { Upload.preprocessed.count }, 1) do
-            get_auth new_upload_path, @user, params: {:url => @source, :ref => @ref}
-            perform_enqueued_jobs
-          end
-        end
-      end
-
-      context "for a twitter post" do
-        setup do
-          @source = "https://twitter.com/onsen_musume_jp/status/865534101918330881"
-        end
-
-        should "render" do
-          skip "Twitter keys are not set" unless Danbooru.config.twitter_api_key
-          get_auth new_upload_path, @user, params: {:url => @source}
-          assert_response :success
-        end
-
-        should "set the correct source" do
-          skip "Twitter keys are not set" unless Danbooru.config.twitter_api_key
-          get_auth new_upload_path, @user, params: {:url => @source}
-          assert_response :success
-          perform_enqueued_jobs
-          upload = Upload.last
-          assert_equal(@source, upload.source)
-        end
-      end
-
-      context "for a pixiv post" do
-        setup do
-          skip "Pixiv credentials not configured" unless Sources::Strategies::Pixiv.enabled?
-          @ref = "http://www.pixiv.net/member_illust.php?mode=medium&illust_id=49270482"
-          @source = "https://i.pximg.net/img-original/img/2015/03/14/17/53/32/49270482_p0.jpg"
-        end
-
-        should "trigger the preprocessor" do
-          assert_difference(-> { Upload.preprocessed.count }, 1) do
-            get_auth new_upload_path, @user, params: {:url => @source, :ref => @ref}
-            perform_enqueued_jobs
-          end
-        end
-      end
-
-      context "for a post that has already been uploaded" do
-        setup do
-          @post = as(@user) { create(:post, source: "http://google.com/aaa") }
-        end
-
-        should "initialize the post" do
-          assert_difference(-> { Upload.count }, 0) do
-            get_auth new_upload_path, @user, params: {:url => "http://google.com/aaa"}
-            assert_response :success
-          end
-        end
+      should "render with an url" do
+        get_auth new_upload_path(url: "https://cdn.donmai.us/original/d3/4e/d34e4cf0a437a5d65f8e82b7bcd02606.jpg"), @user
+        assert_response :success
       end
     end
 
     context "index action" do
-      setup do
-        as(@user) do
-          @upload = create(:upload, tag_string: "foo bar", source: "http://example.com/foobar")
-          @post_upload = create(:source_upload, status: "completed", post: build(:post, tag_string: "touhou"), rating: "e")
-        end
-        as(create(:user)) do
-          @upload3 = create(:upload)
-        end
-      end
-
-      should "render" do
+      should "render as an anonymous user" do
+        create(:completed_source_upload, uploader: @user)
         get uploads_path
+
         assert_response :success
       end
 
-      context "as an uploader" do
-        setup do
-          CurrentUser.user = @user
-        end
+      should "render as an uploader" do
+        create(:completed_source_upload, uploader: @user)
+        get_auth uploads_path, @user
 
-        should respond_to_search({}).with { [@post_upload, @upload] }
-        should respond_to_search(source: "http://example.com/foobar").with { @upload }
-        should respond_to_search(rating: "e").with { @post_upload }
-        should respond_to_search(tag_string: "*foo*").with { @upload }
-
-        context "using includes" do
-          should respond_to_search(post_tags_match: "touhou").with { @post_upload }
-          should respond_to_search(uploader: {name: "marisa"}).with { [@post_upload, @upload] }
-        end
+        assert_response :success
       end
 
-      context "as an admin" do
+      should "render as an admin" do
+        create(:completed_source_upload, uploader: @user)
+        get_auth uploads_path, create(:admin_user)
+
+        assert_response :success
+      end
+
+      context "for a search" do
         setup do
-          CurrentUser.user = create(:admin_user)
+          CurrentUser.user = @user
+          @upload = create(:completed_source_upload, uploader: @user, source: "http://example.com/foobar")
         end
 
-        should respond_to_search({}).with { [@upload3, @post_upload, @upload] }
+        should respond_to_search({}).with { [@upload] }
+        should respond_to_search(source: "http://example.com/foobar").with { @upload }
+        should respond_to_search(status: "completed").with { @upload }
       end
     end
 
     context "show action" do
-      setup do
-        @upload = as(@user) { create(:jpg_upload) }
+      should "not show uploads to other users" do
+        upload = create(:completed_source_upload, uploader: @user)
+        get_auth upload_path(upload), create(:user)
+
+        assert_response 403
       end
 
-      should "render" do
-        get_auth upload_path(@upload), @user
+      should "render a completed source upload for the uploader" do
+        upload = create(:completed_source_upload, uploader: @user)
+        get_auth upload_path(upload), @user
+
+        assert_response :success
+      end
+
+      should "render a completed file upload for the uploader" do
+        upload = create(:completed_file_upload, uploader: @user)
+        get_auth upload_path(upload), @user
+
+        assert_response :success
+      end
+
+      should "render a failed upload" do
+        upload = create(:upload, uploader: @user, status: "error: Not an image or video")
+        get_auth upload_path(upload), @user
+
+        assert_response :success
+      end
+
+      should "render a pending upload" do
+        upload = create(:upload, uploader: @user, status: "pending", source: "https://www.google.com")
+        get_auth upload_path(upload), @user
+
+        assert_response :success
+      end
+
+      should "render a processing upload" do
+        upload = create(:upload, uploader: @user, status: "processing")
+        get_auth upload_path(upload), @user
+
         assert_response :success
       end
     end
 
     context "create action" do
-      context "when a preprocessed upload already exists" do
-        context "for twitter" do
-          setup do
-            as(@user) do
-              @ref = "https://twitter.com/onsen_musume_jp/status/865534101918330881"
-              @source = "https://pbs.twimg.com/media/DAL-ntWV0AEbhes.jpg:orig"
-              @upload = create(:upload, status: "preprocessed", source: @source, referer_url: @ref, image_width: 0, image_height: 0, file_size: 0, md5: "something", file_ext: "jpg")
-            end
-          end
+      should "fail if not given a file or a source" do
+        assert_no_difference("Upload.count") do
+          post_auth uploads_path(format: :json), @user
 
-          should "update the predecessor" do
-            assert_difference(-> { Post.count }, 1) do
-              assert_difference(-> { Upload.count }, 0) do
-                post_auth uploads_path, @user, params: {:upload => {:tag_string => "aaa", :rating => "q", :source => @source, :referer_url => @ref}}
-              end
-            end
-            post = Post.last
-            assert_match(/aaa/, post.tag_string)
-          end
-        end
-
-        context "for pixiv" do
-          setup do
-            @ref = "http://www.pixiv.net/member_illust.php?mode=medium&illust_id=49270482"
-            @source = "https://i.pximg.net/img-original/img/2015/03/14/17/53/32/49270482_p0.jpg"
-            @upload = as(@user) { create(:upload, status: "preprocessed", source: @source, referer_url: @ref, image_width: 0, image_height: 0, file_size: 0, md5: "something", file_ext: "jpg") }
-          end
-
-          should "update the predecessor" do
-            assert_difference(-> { Post.count }, 1) do
-              assert_difference(-> { Upload.count }, 0) do
-                post_auth uploads_path, @user, params: {:upload => {:tag_string => "aaa", :rating => "q", :source => @source, :referer_url => @ref}}
-              end
-            end
-            post = Post.last
-            assert_match(/aaa/, post.tag_string)
-          end
+          assert_response 422
+          assert_equal(["No file or source given"], response.parsed_body.dig("errors", "base"))
         end
       end
 
-      context "when the uploader is limited" do
-        should "not allow uploading" do
-          @member = create(:user, created_at: 2.weeks.ago, upload_points: 0)
-          create_list(:post, @member.upload_limit.upload_slots, uploader: @member, is_pending: true)
+      should "fail if given both a file and source" do
+        assert_no_difference("Upload.count") do
+          file = File.open("test/files/test.jpg")
+          source = "https://files.catbox.moe/om3tcw.webm"
+          post_auth uploads_path(format: :json), @user, params: { upload: { file: file, source: source }}
+        end
 
-          assert_no_difference("Post.count") do
-            file = Rack::Test::UploadedFile.new("#{Rails.root}/test/files/test.jpg", "image/jpeg")
-            post_auth uploads_path, @member, params: { upload: { file: file, tag_string: "aaa", rating: "q" }}
-          end
+        assert_response 422
+        assert_equal(["Can't give both a file and a source"], response.parsed_body.dig("errors", "base"))
+      end
 
-          @upload = Upload.last
-          assert_redirected_to @upload
-          assert_match(/have reached your upload limit/, @upload.status)
+      should "fail if given an unsupported filetype" do
+        file = Rack::Test::UploadedFile.new("test/files/ugoira.json")
+        post_auth uploads_path(format: :json), @user, params: { upload: { file: file }}
+
+        assert_response 201
+        assert_match("Not an image or video", Upload.last.status)
+      end
+
+      should "fail if the file size is too large" do
+        skip "flaky test"
+        Danbooru.config.stubs(:max_file_size).returns(1.kilobyte)
+
+        file = Rack::Test::UploadedFile.new("test/files/test.jpg")
+        post_auth uploads_path(format: :json), @user, params: { upload: { file: file }}
+        perform_enqueued_jobs
+
+        assert_response 201
+        assert_match("File size must be less than or equal to", Upload.last.status)
+
+        Danbooru.config.unstub(:max_file_size)
+      end
+
+      context "for a corrupted image" do
+        should "fail for a corrupted jpeg" do
+          create_upload!("test/files/test-corrupt.jpg", user: @user)
+          assert_match("corrupt", Upload.last.status)
+        end
+
+        should "fail for a corrupted gif" do
+          create_upload!("test/files/test-corrupt.gif", user: @user)
+          assert_match("corrupt", Upload.last.status)
+        end
+
+        # https://schaik.com/pngsuite/pngsuite_xxx_png.html
+        should "fail for a corrupted png" do
+          create_upload!("test/files/test-corrupt.png", user: @user)
+          assert_match("corrupt", Upload.last.status)
         end
       end
 
-      context "for a 2+ minute long video" do
-        should "allow the upload if the user is an admin" do
-          skip "Twitter keys are not set" unless Danbooru.config.twitter_api_key
+      context "for a video longer than the video length limit" do
+        should "fail for a regular user" do
+          @source = "https://cdn.donmai.us/original/63/cb/63cb09f2526ef3ac14f11c011516ad9b.webm"
+          post_auth uploads_path(format: :json), @user, params: { upload: { source: @source }}
+          perform_enqueued_jobs
 
-          @source = "https://twitter.com/7u_NABY/status/1269599527700295681"
-          post_auth uploads_path, create(:admin_user, created_at: 1.week.ago), params: { upload: { tag_string: "aaa", rating: "q", source: @source }}
-          assert_redirected_to Upload.last
+          assert_response 201
+          assert_match("Duration must be less than", Upload.last.status)
+        end
+      end
 
-          assert_equal("mp4", Upload.last.file_ext)
+      # XXX fixme
+      context "for a video longer than the video length limit" do
+        should_eventually "work for an admin" do
+          @source = "https://cdn.donmai.us/original/63/cb/63cb09f2526ef3ac14f11c011516ad9b.webm"
+          post_auth uploads_path(format: :json), create(:admin_user), params: { upload: { source: @source }}
+          perform_enqueued_jobs
+
+          assert_response 201
           assert_equal("completed", Upload.last.status)
-          assert_equal(1280, Upload.last.image_width)
-          assert_equal(720, Upload.last.image_height)
-          assert_equal("mp4", Upload.last.post.file_ext)
-        end
-      end
-
-      context "when the upload is tagged banned_artist" do
-        should "autoban the post" do
-          upload = assert_successful_upload("test/files/test.jpg", tag_string: "banned_artist")
-          assert_equal(true, upload.post.is_banned?)
-        end
-      end
-
-      context "when the upload is tagged paid_reward" do
-        should "autoban the post" do
-          upload = assert_successful_upload("test/files/test.jpg", tag_string: "paid_reward")
-          assert_equal(true, upload.post.is_banned?)
         end
       end
 
@@ -268,15 +221,23 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest
           asset = create(:media_asset, file: File.open("test/files/test.jpg"), status: "processing")
           file = Rack::Test::UploadedFile.new("test/files/test.jpg")
 
-          post_auth uploads_path, @user, params: { upload: { file: file, tag_string: "abc", rating: "e", }}
+          post_auth uploads_path, @user, params: { upload: { file: file }}
           upload = Upload.last
 
           assert_redirected_to upload
-          assert_match("MediaAsset::Error", upload.reload.status)
+          assert_match("Upload failed, try again", upload.reload.status)
           assert_equal("failed", asset.reload.status)
         end
       end
 
+      should "work for a source URL containing unicode characters" do
+        source1 = "https://cdn.donmai.us/original/d3/4e/d34e4cf0a437a5d65f8e82b7bcd02606.jpg?one=東方&two=a%20b"
+        source2 = "https://cdn.donmai.us/original/d3/4e/d34e4cf0a437a5d65f8e82b7bcd02606.jpg?one=%E6%9D%B1%E6%96%B9&two=a%20b"
+
+        upload = assert_successful_upload(source1, user: @user)
+        assert_equal(source2, upload.source)
+      end
+
       context "uploading a file from your computer" do
         should_upload_successfully("test/files/test.jpg")
         should_upload_successfully("test/files/test.png")
@@ -325,6 +286,7 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest
         should_upload_successfully("https://img.pawoo.net/media_attachments/files/000/128/953/original/4c0a06087b03343f.png") if Danbooru.config.pawoo_client_id.present? # XXX
 
         should_upload_successfully("https://www.pixiv.net/en/artworks/64476642")
+        should_upload_successfully("https://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364")
         should_upload_successfully("https://i.pximg.net/img-original/img/2017/08/18/00/09/21/64476642_p0.jpg")
 
         should_upload_successfully("https://noizave.tumblr.com/post/162206271767")
diff --git a/test/test_helpers/upload_test_helper.rb b/test/test_helpers/upload_test_helper.rb
index 0e9f37f7a..eb06ed816 100644
--- a/test/test_helpers/upload_test_helper.rb
+++ b/test/test_helpers/upload_test_helper.rb
@@ -1,47 +1,33 @@
 module UploadTestHelper
   extend ActiveSupport::Concern
 
-  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)
-    uploaded_file = ActionDispatch::Http::UploadedFile.new(tempfile: file, filename: File.basename(path))
-
-    yield uploaded_file if block_given?
-    uploaded_file
-  end
-
-  def assert_successful_upload(source_or_file_path, user: @user, **params)
+  def create_upload!(source_or_file_path, user:, **params)
     if source_or_file_path =~ %r{\Ahttps?://}i
-      return "Login credentials not configured for #{source_or_file_path}" unless Sources::Strategies.find(source_or_file_path).class.enabled?
+      skip "Login credentials not configured for #{source_or_file_path}" unless Sources::Strategies.find(source_or_file_path).class.enabled?
       source = { source: source_or_file_path }
     else
       file = Rack::Test::UploadedFile.new(Rails.root.join(source_or_file_path))
       source = { file: file }
     end
 
-    assert_difference(["Upload.count"]) do
-      post_auth uploads_path, user, params: { upload: { tag_string: "abc", rating: "e", **source, **params }}
-    end
+    post_auth uploads_path(format: :json), user, params: { upload: { **source, **params }}
+  end
+
+  def assert_successful_upload(source_or_file_path, user: create(:user), **params)
+    create_upload!(source_or_file_path, user: user, **params)
+    perform_enqueued_jobs
 
     upload = Upload.last
-    assert_response :redirect
-    assert_redirected_to upload
+    assert_response 201
+    assert_operator(upload.media_assets.count, :>, 0)
     assert_equal("completed", upload.status)
-    assert_equal(Post.last, upload.post)
-    assert_equal(upload.post.md5, upload.md5)
-    assert_not_nil(upload.media_asset)
-    assert_operator(upload.media_asset.media_metadata.metadata.count, :>=, 1)
     upload
   end
 
   class_methods do
     def should_upload_successfully(source)
       should "upload successfully from #{source}" do
-        assert_successful_upload(source, user: create(:user, created_at: 1.month.ago))
+        assert_successful_upload(source, user: create(:user))
       end
     end
   end
diff --git a/test/unit/upload_service_test.rb b/test/unit/upload_service_test.rb
index 1852ed07e..9d5e5a8a8 100644
--- a/test/unit/upload_service_test.rb
+++ b/test/unit/upload_service_test.rb
@@ -1,213 +1,6 @@
 require 'test_helper'
 
 class UploadServiceTest < ActiveSupport::TestCase
-  UGOIRA_CONTEXT = {
-    "ugoira" => {
-      "frame_data" => [
-        {"file" => "000000.jpg", "delay" => 200},
-        {"file" => "000001.jpg", "delay" => 200},
-        {"file" => "000002.jpg", "delay" => 200},
-        {"file" => "000003.jpg", "delay" => 200},
-        {"file" => "000004.jpg", "delay" => 250}
-      ],
-      "content_type" => "image/jpeg"
-    }
-  }
-
-  def assert_file_exists(upload, variant)
-    assert_nothing_raised { upload.media_asset.variant(variant).open_file }
-  end
-
-  def assert_file_does_not_exist(upload, variant)
-    assert_raise { upload.media_asset.variant(variant).open_file }
-  end
-
-  context "::Utils" do
-    context ".process_file" do
-      setup do
-        @upload = FactoryBot.build(:jpg_upload)
-      end
-
-      should "run" do
-        UploadService::Utils.process_file(@upload, @upload.file.tempfile)
-        assert_equal("jpg", @upload.file_ext)
-        assert_equal(28086, @upload.file_size)
-        assert_equal("ecef68c44edb8a0d6a3070b5f8e8ee76", @upload.md5)
-        assert_equal(335, @upload.image_height)
-        assert_equal(500, @upload.image_width)
-      end
-
-      should "create a media asset" do
-        UploadService::Utils.process_file(@upload, @upload.file.tempfile)
-
-        @media_asset = @upload.media_asset
-        assert_not_nil(@media_asset)
-        assert_equal("ecef68c44edb8a0d6a3070b5f8e8ee76", @media_asset.md5)
-        assert_equal("jpg", @media_asset.file_ext)
-        assert_equal(28086, @media_asset.file_size)
-        assert_equal(500, @media_asset.image_width)
-        assert_equal(335, @media_asset.image_height)
-
-        metadata = @media_asset.media_metadata.metadata
-        assert_equal(91, metadata.count)
-      end
-    end
-  end
-
-  context "::Preprocessor" do
-    subject { UploadService::Preprocessor }
-
-    context "#start!" do
-      setup do
-        CurrentUser.user = travel_to(1.month.ago) do
-          FactoryBot.create(:user)
-        end
-        CurrentUser.ip_addr = "127.0.0.1"
-      end
-
-      teardown do
-        CurrentUser.user = nil
-        CurrentUser.ip_addr = nil
-      end
-
-      context "for twitter" do
-        setup do
-          skip "Twitter credentials not configured" unless Sources::Strategies::Twitter.enabled?
-          @source = "https://pbs.twimg.com/media/B4HSEP5CUAA4xyu.png:large"
-          @ref = "https://twitter.com/nounproject/status/540944400767922176"
-        end
-
-        should "download the file" do
-          @service = UploadService::Preprocessor.new(source: @source, referer_url: @ref)
-          @upload = @service.start!
-          assert_equal("preprocessed", @upload.status)
-          assert_equal(9800, @upload.file_size)
-          assert_equal("png", @upload.file_ext)
-          assert_equal("f5fe24f3a3a13885285f6627e04feec9", @upload.md5)
-          assert_file_exists(@upload, :preview)
-          assert_file_exists(@upload, :original)
-        end
-      end
-
-      context "for pixiv" do
-        setup do
-          skip "Pixiv credentials not configured" unless Sources::Strategies::Pixiv.enabled?
-          @source = "https://i.pximg.net/img-original/img/2014/10/29/09/27/19/46785915_p0.jpg"
-          @ref = "https://www.pixiv.net/member_illust.php?mode=medium&illust_id=46785915"
-        end
-
-        should "download the file" do
-          @service = UploadService::Preprocessor.new(source: @source, referer_url: @ref)
-          @upload = @service.start!
-
-          assert_equal("preprocessed", @upload.status)
-          assert_equal(317733, @upload.file_size)
-          assert_equal("jpg", @upload.file_ext)
-          assert_equal("4c71da5638b897aa6da1150e742e2982", @upload.md5)
-          assert_file_exists(@upload, :preview)
-          assert_file_exists(@upload, :original)
-        end
-      end
-
-      context "for pixiv ugoira" do
-        setup do
-          skip "Pixiv credentials not configured" unless Sources::Strategies::Pixiv.enabled?
-          @source = "http://www.pixiv.net/member_illust.php?mode=medium&illust_id=62247364"
-        end
-
-        should "download the file" do
-          skip unless MediaFile::Ugoira.videos_enabled?
-
-          @service = UploadService::Preprocessor.new(source: @source)
-          @upload = @service.start!
-
-          assert_equal("preprocessed", @upload.status)
-          assert_equal(2804, @upload.file_size)
-          assert_equal("zip", @upload.file_ext)
-          assert_equal("cad1da177ef309bf40a117c17b8eecf5", @upload.md5)
-          assert_file_exists(@upload, :sample)
-          assert_file_exists(@upload, :original)
-        end
-      end
-
-      context "for null" do
-        setup do
-          @source = "https://cdn.donmai.us/original/93/f4/93f4dd66ef1eb11a89e56d31f9adc8d0.jpg"
-        end
-
-        should "download the file" do
-          @service = UploadService::Preprocessor.new(source: @source)
-          @upload = @service.start!
-
-          assert_equal("preprocessed", @upload.status)
-          assert_equal(181309, @upload.file_size)
-          assert_equal("jpg", @upload.file_ext)
-          assert_equal("93f4dd66ef1eb11a89e56d31f9adc8d0", @upload.md5)
-          assert_file_exists(@upload, :preview)
-          assert_file_exists(@upload, :sample)
-          assert_file_exists(@upload, :original)
-        end
-      end
-
-      context "for a video" do
-        setup do
-          @source = "https://cdn.donmai.us/original/b7/cb/b7cb80092be273771510952812380fa2.mp4"
-        end
-
-        should "work for a video" do
-          @service = UploadService::Preprocessor.new(source: @source)
-          @upload = @service.start!
-          assert_equal("preprocessed", @upload.status)
-          assert_not_nil(@upload.md5)
-          assert_equal("mp4", @upload.file_ext)
-          assert_operator(@upload.file_size, :>, 0)
-          assert_not_nil(@upload.source)
-          assert_file_exists(@upload, :preview)
-          assert_file_exists(@upload, :original)
-        end
-      end
-
-      context "on timeout errors" do
-        setup do
-          @source = "https://cdn.donmai.us/original/93/f4/93f4dd66ef1eb11a89e56d31f9adc8d0.jpg"
-          Danbooru::Http.any_instance.stubs(:get).raises(HTTP::TimeoutError)
-        end
-
-        should "leave the upload in an error state" do
-          @service = UploadService::Preprocessor.new(source: @source)
-          @upload = @service.start!
-          assert_match(/error:/, @upload.status)
-        end
-      end
-
-      context "for an invalid content type" do
-        should "fail" do
-          upload = UploadService::Preprocessor.new(source: "http://www.example.com").start!
-          assert_match(/\Aerror:.*File ext is invalid/, upload.status)
-        end
-      end
-    end
-
-    context "#finish!" do
-      setup do
-        skip "Twitter credentials not configured" unless Sources::Strategies::Twitter.enabled?
-        CurrentUser.user = travel_to(1.month.ago) do
-          FactoryBot.create(:user)
-        end
-        CurrentUser.ip_addr = "127.0.0.1"
-        @source = "https://twitter.com/nounproject/status/540944400767922176"
-      end
-
-      should "overwrite the attributes" do
-        @service = UploadService::Preprocessor.new(source: @source, rating: 'e')
-        @upload = @service.start!
-        @service.finish!
-        @upload.reload
-        assert_equal('e', @upload.rating)
-      end
-    end
-  end
-
   context "::Replacer" do
     context "for a file replacement" do
       setup do
@@ -518,63 +311,12 @@ class UploadServiceTest < ActiveSupport::TestCase
       CurrentUser.ip_addr = nil
     end
 
-    context "automatic tagging" do
-      should "tag animated png files" do
-        upload = UploadService.new(file: upload_file("test/files/apng/normal_apng.png")).start!
-        assert_match(/animated_png/, upload.post.tag_string)
-      end
-
-      should "tag animated gif files" do
-        upload = UploadService.new(file: upload_file("test/files/test-animated-86x52.gif")).start!
-        assert_match(/animated_gif/, upload.post.tag_string)
-      end
-
-      should "not tag static gif files" do
-        upload = UploadService.new(file: upload_file("test/files/test-static-32x32.gif")).start!
-        assert_no_match(/animated_gif/, upload.post.tag_string)
-      end
-    end
-
-    context "that is too large" do
-      setup do
-        Danbooru.config.stubs(:max_image_resolution).returns(31 * 31)
-      end
-
-      should "should fail validation" do
-        service = subject.new(file: upload_file("test/files/test-large.jpg"))
-        upload = service.start!
-        assert_match(/image resolution is too large/, upload.status)
-      end
-    end
-
-    context "with a preprocessing predecessor" do
-      setup do
-        @predecessor = FactoryBot.create(:source_upload, status: "preprocessing", source: @source, image_height: 0, image_width: 0, file_ext: "jpg")
-      end
-
-      should "schedule a job later" do
-        service = subject.new(source: @source)
-
-        predecessor = service.start!
-        assert_enqueued_jobs(1, only: UploadServiceDelayedStartJob)
-        assert_equal(@predecessor, predecessor)
-      end
-    end
-
     context "with a preprocessed predecessor" do
       setup do
         @predecessor = FactoryBot.create(:source_upload, status: "preprocessed", source: @source, image_height: 0, image_width: 0, file_size: 1, md5: 'd34e4cf0a437a5d65f8e82b7bcd02606', file_ext: "jpg")
         @tags = 'hello world'
       end
 
-      should "update the predecessor" do
-        service = subject.new(source: @source, tag_string: @tags)
-
-        predecessor = service.start!
-        assert_equal(@predecessor, predecessor)
-        assert_equal(@tags, predecessor.tag_string.strip)
-      end
-
       context "when the file has already been uploaded" do
         setup do
           @asset = MediaAsset.find_by_md5("d34e4cf0a437a5d65f8e82b7bcd02606")
@@ -590,39 +332,7 @@ class UploadServiceTest < ActiveSupport::TestCase
       end
     end
 
-    context "with no predecessor" do
-      should "create an upload" do
-        service = subject.new(source: @source)
-
-        assert_difference(-> { Upload.count }) do
-          service.start!
-        end
-      end
-
-      should "assign the rating from tags" do
-        service = subject.new(source: @source, tag_string: "rating:safe blah")
-        upload = service.start!
-
-        assert_equal(true, upload.valid?)
-        assert_equal("s", upload.rating)
-        assert_equal("rating:safe blah", upload.tag_string)
-
-        assert_equal("s", upload.post.rating)
-        assert_equal("blah", upload.post.tag_string)
-      end
-    end
-
     context "with a source containing unicode characters" do
-      should "upload successfully" do
-        source1 = "https://cdn.donmai.us/original/d3/4e/d34e4cf0a437a5d65f8e82b7bcd02606.jpg?one=東方&two=a%20b"
-        source2 = "https://cdn.donmai.us/original/d3/4e/d34e4cf0a437a5d65f8e82b7bcd02606.jpg?one=%E6%9D%B1%E6%96%B9&two=a%20b"
-        service = subject.new(source: source1, rating: "s")
-
-        assert_nothing_raised { @upload = service.start! }
-        assert_equal(true, @upload.is_completed?)
-        assert_equal(source2, @upload.source)
-      end
-
       should "normalize unicode characters in the source field" do
         source1 = "poke\u0301mon" # pokémon (nfd form)
         source2 = "pok\u00e9mon"  # pokémon (nfc form)
@@ -632,51 +342,6 @@ class UploadServiceTest < ActiveSupport::TestCase
         assert_equal(source2, @upload.source)
       end
     end
-
-    context "without a file or a source url" do
-      should "fail gracefully" do
-        service = subject.new(source: "blah", rating: "s")
-
-        assert_nothing_raised { @upload = service.start! }
-        assert_equal(true, @upload.is_errored?)
-        assert_match(/No file or source URL provided/, @upload.status)
-      end
-    end
-
-    context "with both a file and a source url" do
-      should "upload the file and set the source field to the given source" do
-        service = subject.new(file: upload_file("test/files/test.jpg"), source: "http://www.example.com", rating: "s")
-
-        assert_nothing_raised { @upload = service.start! }
-        assert_equal(true, @upload.is_completed?)
-        assert_equal("ecef68c44edb8a0d6a3070b5f8e8ee76", @upload.md5)
-        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)
-      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)
-      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)
-      end
-    end
   end
 
   context "#create_post_from_upload" do
@@ -767,31 +432,4 @@ class UploadServiceTest < ActiveSupport::TestCase
       end
     end
   end
-
-  context "Upload#prune!" do
-    setup do
-      @user = create(:user, created_at: 1.year.ago)
-    end
-
-    should "delete stale upload records" do
-      @upload = as(@user) { UploadService.new(file: upload_file("test/files/test.jpg")).start! }
-      @upload.update!(created_at: 1.month.ago)
-
-      assert_difference("Upload.count", -1) { Upload.prune! }
-    end
-
-    should "work on uploads without a file" do
-      @upload = as(@user) { UploadService.new(source: "http://14903gf0vm3g134yjq3n535yn3n.com/does_not_exist.jpg").start! }
-
-      assert(@upload.is_errored?)
-      assert_difference("Upload.count", -1) { @upload.destroy! }
-    end
-
-    should "work on uploads with an invalid file" do
-      @upload = as(@user) { UploadService.new(file: upload_file("test/files/test-empty.bin")).start! }
-
-      assert(@upload.is_errored?)
-      assert_difference("Upload.count", -1) { @upload.destroy! }
-    end
-  end
 end