diff --git a/config/site_settings.yml b/config/site_settings.yml index bffff294ebc..45f16b69b21 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1886,6 +1886,9 @@ rate_limits: max_allowed_secondary_emails: default: 10 hidden: true + max_batch_presign_multipart_per_minute: + default: 20 + hidden: true developer: force_hostname: diff --git a/lib/external_upload_helpers.rb b/lib/external_upload_helpers.rb index ff187e5de38..5b0e43f5ab3 100644 --- a/lib/external_upload_helpers.rb +++ b/lib/external_upload_helpers.rb @@ -10,7 +10,6 @@ module ExternalUploadHelpers PRESIGNED_PUT_RATE_LIMIT_PER_MINUTE = 10 CREATE_MULTIPART_RATE_LIMIT_PER_MINUTE = 10 COMPLETE_MULTIPART_RATE_LIMIT_PER_MINUTE = 10 - BATCH_PRESIGN_RATE_LIMIT_PER_MINUTE = 10 included do before_action :external_store_check, only: [ @@ -112,8 +111,17 @@ module ExternalUploadHelpers part_numbers = params.require(:part_numbers) unique_identifier = params.require(:unique_identifier) + ## + # NOTE: This is configurable by hidden site setting because this really is heavily + # dependent on upload speed. We request 5-10 URLs at a time with this endpoint; for + # a 1.5GB upload with 5mb parts this could mean 60 requests to the server to get all + # the part URLs. If the user's upload speed is super fast they may request all 60 + # batches in a minute, if it is slow they may request 5 batches in a minute. + # + # The other external upload endpoints are not hit as often, so they can stay as constant + # values for now. RateLimiter.new( - current_user, "batch-presign", ExternalUploadHelpers::BATCH_PRESIGN_RATE_LIMIT_PER_MINUTE, 1.minute + current_user, "batch-presign", SiteSetting.max_batch_presign_multipart_per_minute, 1.minute ).performed! part_numbers = part_numbers.map do |part_number| diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb index 46b0e198752..d54cd871449 100644 --- a/spec/requests/uploads_controller_spec.rb +++ b/spec/requests/uploads_controller_spec.rb @@ -1056,23 +1056,22 @@ describe UploadsController do it "rate limits" do RateLimiter.enable RateLimiter.clear_all! + SiteSetting.max_batch_presign_multipart_per_minute = 1 - stub_const(ExternalUploadHelpers, "BATCH_PRESIGN_RATE_LIMIT_PER_MINUTE", 1) do - stub_list_multipart_request - post "/uploads/batch-presign-multipart-parts.json", params: { - unique_identifier: external_upload_stub.unique_identifier, - part_numbers: [1, 2, 3] - } + stub_list_multipart_request + post "/uploads/batch-presign-multipart-parts.json", params: { + unique_identifier: external_upload_stub.unique_identifier, + part_numbers: [1, 2, 3] + } - expect(response.status).to eq(200) + expect(response.status).to eq(200) - post "/uploads/batch-presign-multipart-parts.json", params: { - unique_identifier: external_upload_stub.unique_identifier, - part_numbers: [1, 2, 3] - } + post "/uploads/batch-presign-multipart-parts.json", params: { + unique_identifier: external_upload_stub.unique_identifier, + part_numbers: [1, 2, 3] + } - expect(response.status).to eq(429) - end + expect(response.status).to eq(429) end end