Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DEV: Change external upload rate limit maximums to settings (#20577) 

Way back when this was introduced way back in b96c10a903
I didn't have any frame of reference for what these max rate
limit numbers should be, so 10 seemed like a reasonable limit
until a real world case where this did not make sense came
along.

The time has come.

Moving these into site settings, which are hidden since in most
cases there is no need to change these.
This commit is contained in:
Martin Brennan 2023-03-08 15:27:17 +10:00 committed by GitHub
parent b62d44b40a
commit 6feb436303
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 58 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
config/site_settings.yml
View File

@ -1985,6 +1985,15 @@ rate_limits:
max_batch_presign_multipart_per_minute:
default: 20
hidden: true
max_presigned_put_per_minute:
default: 10
hidden: true
max_create_multipart_per_minute:
default: 10
hidden: true
max_complete_multipart_per_minute:
default: 10
hidden: true
developer:
force_hostname:

24
lib/external_upload_helpers.rb
View File

@ -8,10 +8,6 @@ module ExternalUploadHelpers
class ExternalUploadValidationError < StandardError
end
PRESIGNED_PUT_RATE_LIMIT_PER_MINUTE = 10
CREATE_MULTIPART_RATE_LIMIT_PER_MINUTE = 10
COMPLETE_MULTIPART_RATE_LIMIT_PER_MINUTE = 10
included do
before_action :external_store_check,
only: %i[
@ -38,7 +34,7 @@ module ExternalUploadHelpers
RateLimiter.new(
current_user,
"generate-presigned-put-upload-stub",
ExternalUploadHelpers::PRESIGNED_PUT_RATE_LIMIT_PER_MINUTE,
SiteSetting.max_presigned_put_per_minute,
1.minute,
).performed!
@ -81,7 +77,7 @@ module ExternalUploadHelpers
RateLimiter.new(
current_user,
"create-multipart-upload",
ExternalUploadHelpers::CREATE_MULTIPART_RATE_LIMIT_PER_MINUTE,
SiteSetting.max_create_multipart_per_minute,
1.minute,
).performed!
@ -127,9 +123,6 @@ module ExternalUploadHelpers
# 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",
@ -225,7 +218,7 @@ module ExternalUploadHelpers
RateLimiter.new(
current_user,
"complete-multipart-upload",
ExternalUploadHelpers::COMPLETE_MULTIPART_RATE_LIMIT_PER_MINUTE,
SiteSetting.max_complete_multipart_per_minute,
1.minute,
).performed!
@ -256,12 +249,11 @@ module ExternalUploadHelpers
.sort_by { |part| part[:part_number] }
begin
complete_response =
store.complete_multipart(
upload_id: external_upload_stub.external_upload_identifier,
key: external_upload_stub.key,
parts: parts,
)
store.complete_multipart(
upload_id: external_upload_stub.external_upload_identifier,
key: external_upload_stub.key,
parts: parts,
)
rescue Aws::S3::Errors::ServiceError => err
return(
render_json_error(

83
spec/requests/uploads_controller_spec.rb
View File

@ -779,21 +779,21 @@ RSpec.describe UploadsController do
it "rate limits" do
RateLimiter.enable
RateLimiter.clear_all!
SiteSetting.max_presigned_put_per_minute = 1
post "/uploads/generate-presigned-put.json",
params: {
file_name: "test.png",
type: "card_background",
file_size: 1024,
}
post "/uploads/generate-presigned-put.json",
params: {
file_name: "test.png",
type: "card_background",
file_size: 1024,
}
stub_const(ExternalUploadHelpers, "PRESIGNED_PUT_RATE_LIMIT_PER_MINUTE", 1) do
post "/uploads/generate-presigned-put.json",
params: {
file_name: "test.png",
type: "card_background",
file_size: 1024,
}
post "/uploads/generate-presigned-put.json",
params: {
file_name: "test.png",
type: "card_background",
file_size: 1024,
}
end
expect(response.status).to eq(429)
end
end
@ -937,25 +937,24 @@ RSpec.describe UploadsController do
it "rate limits" do
RateLimiter.enable
RateLimiter.clear_all!
SiteSetting.max_create_multipart_per_minute = 1
stub_create_multipart_request
stub_const(ExternalUploadHelpers, "CREATE_MULTIPART_RATE_LIMIT_PER_MINUTE", 1) do
post "/uploads/create-multipart.json",
params: {
file_name: "test.png",
upload_type: "composer",
file_size: 1024,
}
expect(response.status).to eq(200)
post "/uploads/create-multipart.json",
params: {
file_name: "test.png",
upload_type: "composer",
file_size: 1024,
}
expect(response.status).to eq(200)
post "/uploads/create-multipart.json",
params: {
file_name: "test.png",
upload_type: "composer",
file_size: 1024,
}
expect(response.status).to eq(429)
end
post "/uploads/create-multipart.json",
params: {
file_name: "test.png",
upload_type: "composer",
file_size: 1024,
}
expect(response.status).to eq(429)
end
end
@ -1334,19 +1333,19 @@ RSpec.describe UploadsController do
it "rate limits" do
RateLimiter.enable
RateLimiter.clear_all!
SiteSetting.max_complete_multipart_per_minute = 1
post "/uploads/complete-multipart.json",
params: {
unique_identifier: "blah",
parts: [{ part_number: 1, etag: "test1" }, { part_number: 2, etag: "test2" }],
}
post "/uploads/complete-multipart.json",
params: {
unique_identifier: "blah",
parts: [{ part_number: 1, etag: "test1" }, { part_number: 2, etag: "test2" }],
}
stub_const(ExternalUploadHelpers, "COMPLETE_MULTIPART_RATE_LIMIT_PER_MINUTE", 1) do
post "/uploads/complete-multipart.json",
params: {
unique_identifier: "blah",
parts: [{ part_number: 1, etag: "test1" }, { part_number: 2, etag: "test2" }],
}
post "/uploads/complete-multipart.json",
params: {
unique_identifier: "blah",
parts: [{ part_number: 1, etag: "test1" }, { part_number: 2, etag: "test2" }],
}
end
expect(response.status).to eq(429)
end
end