Fix #3983: Uploads: NoMethodError - undefined method `>=' for nil:NilClass

* Rename Upload#download_for_upload to #get_file_for_upload.

* Fix #get_file_for_upload to raise error if no file or source url was given.

* Fix javascript upload validation to disallow submitting form if file is
  not present and the source is not an url.

app/javascript/src/javascripts/uploads.js

@@ -35,7 +35,7 @@ Upload.initialize_submit = function() {
 
 Upload.validate_upload = function (e) {
   var error_messages = [];
-  if (($("#upload_file").val() === "") && ($("#upload_source").val() === "") && $("#upload_md5_confirmation").val() === "") {
+  if (($("#upload_file").val() === "") && !/^https?:\/\//i.test($("#upload_source").val()) && $("#upload_md5_confirmation").val() === "") {
     error_messages.push("Must choose file or specify source");
   }
   if (!$("#upload_rating_s").prop("checked") && !$("#upload_rating_q").prop("checked") && !$("#upload_rating_e").prop("checked") &&

app/logical/upload_service.rb

@@ -48,13 +48,8 @@ class UploadService
 
       @upload.update(status: "processing")
 
-      if @upload.file.nil? && @upload.source_url.present?
-        @upload.file = Utils.download_for_upload(@upload)
-      end
-
-      if @upload.file.present?
-        Utils.process_file(upload, @upload.file)
-      end
+      @upload.file = Utils.get_file_for_upload(@upload, file: @upload.file)
+      Utils.process_file(upload, @upload.file)
 
       @upload.save!
       @post = create_post_from_upload(@upload)

app/logical/upload_service/preprocessor.rb

@@ -91,12 +91,7 @@ class UploadService
       begin
         upload.update(status: "preprocessing")
 
-        if params[:file].present?
-          file = params[:file]
-        elsif upload.source_url.present?
-          file = Utils.download_for_upload(upload)
-        end
-
+        file = Utils.get_file_for_upload(upload, file: params[:file])
         Utils.process_file(upload, file, original_post_id: original_post_id)
 
         upload.rating = params[:rating]

app/logical/upload_service/utils.rb

@@ -206,7 +206,10 @@ class UploadService
       tags.join(" ")
     end
 
-    def download_for_upload(upload)
+    def get_file_for_upload(upload, file: nil)
+      return file if file.present?
+      raise RuntimeError, "No file or source URL provided" if upload.source_url.blank?
+
       attempts = 0
 
       begin

app/models/upload.rb

@@ -115,11 +115,11 @@ class Upload < ApplicationRecord
     end
 
     def is_duplicate?
-      status =~ /duplicate: \d+/
+      status.match?(/duplicate: \d+/)
     end
 
     def is_errored?
-      status =~ /error:/
+      status.match?(/error:/)
     end
 
     def sanitized_status

test/models/upload_service_test.rb

@@ -25,7 +25,7 @@ class UploadServiceTest < ActiveSupport::TestCase
   context "::Utils" do
     subject { UploadService::Utils }
 
-    context "#download_for_upload" do
+    context "#get_file_for_upload" do
       context "for a non-source site" do
         setup do
           @source = "https://upload.wikimedia.org/wikipedia/commons/c/c5/Moraine_Lake_17092005.jpg"          
@@ -34,7 +34,7 @@ class UploadServiceTest < ActiveSupport::TestCase
         end
 
         should "work on a jpeg" do
-          file = subject.download_for_upload(@upload)
+          file = subject.get_file_for_upload(@upload)
 
           assert_operator(File.size(file.path), :>, 0)
 
@@ -59,7 +59,7 @@ class UploadServiceTest < ActiveSupport::TestCase
         should "retry three times" do
           DanbooruImageResizer.expects(:validate_shell).times(4).returns(false)
           assert_raise(UploadService::Utils::CorruptFileError) do
-            subject.download_for_upload(@mock_upload)
+            subject.get_file_for_upload(@mock_upload)
           end
         end
       end
@@ -73,7 +73,7 @@ class UploadServiceTest < ActiveSupport::TestCase
 
         should "work on an ugoira url" do
           begin
-            file = subject.download_for_upload(@upload)
+            file = subject.get_file_for_upload(@upload)
 
             assert_operator(File.size(file.path), :>, 0)
 
@@ -95,7 +95,7 @@ class UploadServiceTest < ActiveSupport::TestCase
 
         should "work on an ugoira url" do
           begin
-            file = subject.download_for_upload(@upload)
+            file = subject.get_file_for_upload(@upload)
 
             assert_not_nil(@upload.context["ugoira"])
             assert_operator(File.size(file.path), :>, 0)
@@ -1108,6 +1108,27 @@ 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
   end
 
   context "#create_post_from_upload" do