diff --git a/app/controllers/image_uploads_controller.rb b/app/controllers/image_uploads_controller.rb index 1d6661c77145c..842d3bca161f2 100644 --- a/app/controllers/image_uploads_controller.rb +++ b/app/controllers/image_uploads_controller.rb @@ -7,7 +7,15 @@ def create uploader = ArticleImageUploader.new begin + raise RateLimitChecker::UploadRateLimitReached if RateLimitChecker.new(current_user).limit_by_situation("image_upload") + uploader.store!(params[:image]) + RateLimitChecker.new(current_user).track_image_uploads + rescue RateLimitChecker::UploadRateLimitReached + respond_to do |format| + format.json { render json: { error: "Upload limit reached!" } } + end + return rescue CarrierWave::IntegrityError => e # client error respond_to do |format| format.json { render json: { error: e.message }, status: :unprocessable_entity } @@ -20,6 +28,10 @@ def create return end + cloudinary_link(uploader) + end + + def cloudinary_link(uploader) link = if params[:wrap_cloudinary] ApplicationController.helpers.cloud_cover_url(uploader.url) else diff --git a/app/labor/rate_limit_checker.rb b/app/labor/rate_limit_checker.rb index a57fbd8c183d5..256f43b415ec4 100644 --- a/app/labor/rate_limit_checker.rb +++ b/app/labor/rate_limit_checker.rb @@ -4,12 +4,16 @@ def initialize(user = nil) @user = user end + class UploadRateLimitReached < StandardError; end + def limit_by_situation(situation) result = case situation when "comment_creation" user.comments.where("created_at > ?", 30.seconds.ago).size > 9 when "published_article_creation" user.articles.published.where("created_at > ?", 30.seconds.ago).size > 9 + when "image_upload" + Rails.cache.read("#{user.id}_image_upload").to_i > 9 else false end @@ -17,6 +21,12 @@ def limit_by_situation(situation) result end + def track_image_uploads + count = Rails.cache.read("#{@user.id}_image_upload").to_i + count += 1 + Rails.cache.write("#{@user.id}_image_upload", count, expires_in: 30.seconds) + end + def limit_by_email_recipient_address(address) # This is related to the recipient, not the "user" initiator, like in situation. EmailMessage.where(to: address). diff --git a/spec/requests/image_uploads_spec.rb b/spec/requests/image_uploads_spec.rb index 7c943de7f9124..86ddd7f943c79 100644 --- a/spec/requests/image_uploads_spec.rb +++ b/spec/requests/image_uploads_spec.rb @@ -10,6 +10,8 @@ "image/jpeg", ) end + let(:memory_store) { ActiveSupport::Cache.lookup_store(:memory_store) } + let(:cache) { Rails.cache } let(:bad_image) do Rack::Test::UploadedFile.new( Rails.root.join("spec", "support", "fixtures", "images", "bad-image.jpg"), @@ -51,5 +53,29 @@ expect(result["error"]).not_to be_nil end end + + context "when uploading rate limiting works" do + before do + sign_in user + allow(Rails).to receive(:cache).and_return(memory_store) + Rails.cache.clear + end + + it "counts number of uploads in cache" do + post "/image_uploads", headers: headers, params: { image: image } + expect(cache.read("#{user.id}_image_upload")).to eq(1) + end + + it "raises error with too many uploads" do + upload = proc do + Rack::Test::UploadedFile.new( + Rails.root.join("spec", "support", "fixtures", "images", "images1.jpeg"), "image/jpeg" + ) + end + expect do + 11.times { post "/image_uploads", headers: headers, params: { image: upload.call } } + end.to raise_error(RuntimeError) + end + end end end