From 1a210070688d961d60875094d178470bdd3552dd Mon Sep 17 00:00:00 2001 From: Christoph Haas Date: Sun, 21 Feb 2021 11:38:53 +0100 Subject: [PATCH] Properly detect duplicate uploads --- app/assets/javascripts/application.js | 5 ++- app/controllers/packages_controller.rb | 53 +++++++++++++++----------- 2 files changed, 34 insertions(+), 24 deletions(-) diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index e06d12f..7f54e3c 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -105,7 +105,8 @@ function upload_file_with_ajax(file) error: function(xhr, txt, err) { console.log("AJAX file upload returned error: "+txt+" / "+err); - display_error('Did you copy a proper image into your clipboard?'); + response = JSON.parse(xhr.responseText); + display_error(response); }, success: function(res) { @@ -119,5 +120,5 @@ function upload_file_with_ajax(file) function display_error(msg) { $('#messages').html( - '
Sorry, pasting did not work.' + msg + '
') + '
Sorry, pasting did not work. ' + msg + '
') } diff --git a/app/controllers/packages_controller.rb b/app/controllers/packages_controller.rb index 63cb4a6..e6bfe39 100644 --- a/app/controllers/packages_controller.rb +++ b/app/controllers/packages_controller.rb @@ -70,21 +70,32 @@ class PackagesController < ApplicationController new_screenshot.version = @package.version # ActiveStorage does not yet create a file checksum automatically. # Let's do that. It helps detect duplicate uploads later. - new_screenshot.image_fingerprint = Digest::MD5.hexdigest(img.path) - - # Can the upload get approved automatically? - if user_signed_in? - new_screenshot.user = current_user - new_screenshot.approve! if auto_approve? + new_screenshot.image_fingerprint = Digest::MD5.hexdigest(File.read img.path) + + # Check that this screenshot is not a duplicate for this package + if @package.screenshots.where(image_fingerprint: new_screenshot.image_fingerprint).any? + Log.log "Duplicate image with fingerprint #{new_screenshot.image_fingerprint} found. Rejecting." + all_errors << "Your file #{img.original_filename} is a duplicate." + else + # Can the upload get approved automatically? + if user_signed_in? + new_screenshot.user = current_user + new_screenshot.approve! if auto_approve? + end + + new_screenshot.simage_derivatives! + Log.log "Derivatives created" + + new_screenshot.save! + Log.log "Screenshot #{new_screenshot.id} uploaded successfully. " + \ + "ip=#{new_screenshot.uploaderip}. "+ \ + "user-hash=#{new_screenshot.uploaderhash}. "+ \ + "user-name=#{current_user} "+ \ + "image-fingerprint=#{new_screenshot.image_fingerprint} "+ \ + "image-path=#{img.path}" + + @valid_images.push new_screenshot end - - new_screenshot.simage_derivatives! - Log.log "Derivatives created" - - new_screenshot.save! - Log.log "Screenshot #{new_screenshot.id} uploaded successfully. ip=#{new_screenshot.uploaderip}. user-hash=#{new_screenshot.uploaderhash}. user-name=#{current_user}" - - @valid_images.push new_screenshot else errors = new_screenshot.errors[:simage] Log.log "Screenshot #{img.original_filename} invalid (#{errors})." @@ -98,11 +109,11 @@ class PackagesController < ApplicationController flash[:error] = all_errors end - # Redirect back to upload form if all uploads were invalid - unless @valid_images.any? - Log.log "No valid images uploaded. Back to upload form." - redirect_to(upload_path, error: all_errors) and return - end + # # Redirect back to upload form if all uploads were invalid + # unless @valid_images.any? + # Log.log "No valid images uploaded. Back to upload form." + # redirect_to(upload_path, error: all_errors) and return + # end # Show a list of invalid uploads by default. Or redirect to the review page # if all uploads were okay. @@ -117,12 +128,10 @@ class PackagesController < ApplicationController # is set in routes.rb to signal that this method was called by AJAX. if params[:returns] == :json # TODO: send all_errors back as JSON and make Javascript display it in #messages - render :json => true + render :json => {errors: all_errors.join(' ')} else redirect_to package_path end - - # render :details end # Legacy action to upload an image along with metadata.