From 87a00a118292584098d9a8611e4aff8a5942a9e4 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Tue, 15 Feb 2022 17:34:49 -0600
Subject: [PATCH] uploads: fix "ArgumentError: string contains null byte" error

Fix an error when trying to upload a file larger than the file size
limit. In this case we tried to dump the whole HTTP response into the
error message, which included the binary file itself, which caused this
exception because it contained null bytes.
---
 app/logical/danbooru/http.rb               |  4 +--
 app/models/media_asset.rb                  |  2 +-
 test/functional/uploads_controller_test.rb | 32 ++++++++++++++++------
 3 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/app/logical/danbooru/http.rb b/app/logical/danbooru/http.rb
index 2508abe86..a0b8944e2 100644
--- a/app/logical/danbooru/http.rb
+++ b/app/logical/danbooru/http.rb
@@ -148,12 +148,12 @@ module Danbooru
         response = get(url)
 
         raise DownloadError, "Downloading #{response.uri} failed with code #{response.status}" if response.status != 200
-        raise FileTooLargeError, response if @max_size && response.content_length.to_i > @max_size
+        raise FileTooLargeError, "File size too large (size: #{response.content_length.to_i.to_formatted_s(:human_size)}; max size: #{@max_size.to_formatted_s(:human_size)})" if @max_size && response.content_length.to_i > @max_size
 
         size = 0
         response.body.each do |chunk|
           size += chunk.size
-          raise FileTooLargeError if @max_size && size > @max_size
+          raise FileTooLargeError, "File size too large (max size: #{@max_size.to_formatted_s(:human_size)})" if @max_size && size > @max_size
           file.write(chunk)
         end
 
diff --git a/app/models/media_asset.rb b/app/models/media_asset.rb
index e9dad1980..79af6f49d 100644
--- a/app/models/media_asset.rb
+++ b/app/models/media_asset.rb
@@ -39,7 +39,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 :file_size, numericality: { less_than_or_equal_to: Danbooru.config.max_file_size, message: ->(asset, _) { "too large (size: #{asset.file_size.to_formatted_s(:human_size)}; max size: #{Danbooru.config.max_file_size.to_formatted_s(:human_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
 
diff --git a/test/functional/uploads_controller_test.rb b/test/functional/uploads_controller_test.rb
index 6d26746b7..ea0ece1e9 100644
--- a/test/functional/uploads_controller_test.rb
+++ b/test/functional/uploads_controller_test.rb
@@ -177,18 +177,32 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest
         assert_match("Not an image or video", Upload.last.error)
       end
 
-      should "fail if the file size is too large" do
-        skip "flaky test"
-        Danbooru.config.stubs(:max_file_size).returns(1.kilobyte)
+      context "for a file larger than the file size limit" do
+        setup do
+          skip "flaky test"
+          Danbooru.config.stubs(:max_file_size).returns(1.kilobyte)
+        end
 
-        file = Rack::Test::UploadedFile.new("test/files/test.jpg")
-        post_auth uploads_path(format: :json), @user, params: { upload: { file: file }}
-        perform_enqueued_jobs
+        should "fail for a direct file upload" do
+          create_upload!("test/files/test.jpg", user: @user)
 
-        assert_response 201
-        assert_match("File size must be less than or equal to", Upload.last.error)
+          assert_response 201
+          assert_match("File size too large", Upload.last.error)
+        end
 
-        Danbooru.config.unstub(:max_file_size)
+        should "fail for a source upload with a Content-Length header" do
+          create_upload!("https://nghttp2.org/httpbin/bytes/2000", user: @user)
+
+          assert_response 201
+          assert_match("File size too large", Upload.last.error)
+        end
+
+        should "fail for a source upload without a Content-Length header" do
+          create_upload!("https://nghttp2.org/httpbin/stream-bytes/2000", user: @user)
+
+          assert_response 201
+          assert_match("File size too large", Upload.last.error)
+        end
       end
 
       context "for a corrupted image" do