Skip to content

Commit

Permalink
Fix: 絵文字リアクションに厳しいレートリミットが適用される問題 (#752)
Browse files Browse the repository at this point in the history
  • Loading branch information
kmycode authored Jun 2, 2024
1 parent 14820ee commit 51155d6
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 15 deletions.
4 changes: 3 additions & 1 deletion config/initializers/rack_attack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,10 @@ def paging_request?
req.session[:attempt_user_id] || req.params.dig('user', 'email').presence if req.post? && req.path_matches?('/auth/sign_in')
end

API_CREATE_EMOJI_REACTION_REGEX = %r{\A/api/v1/statuses/\d+/emoji_reactions}

throttle('throttle_password_change/account', limit: 10, period: 10.minutes) do |req|
req.warden_user_id if req.put? || (req.patch? && req.path_matches?('/auth'))
req.warden_user_id if (req.put? && !req.path.match?(API_CREATE_EMOJI_REACTION_REGEX)) || (req.patch? && req.path_matches?('/auth'))
end

self.throttled_responder = lambda do |request|
Expand Down
79 changes: 65 additions & 14 deletions spec/config/initializers/rack/attack_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def app
Rails.application
end

shared_examples 'throttled endpoint' do
shared_context 'with throttled endpoint base' do
before do
# Rack::Attack periods are not rolling, so avoid flaky tests by setting the time in a way
# to avoid crossing period boundaries.
Expand All @@ -19,6 +19,30 @@ def app
travel_to Time.zone.at(counter_prefix * period.seconds)
end

def below_limit
limit - 1
end

def above_limit
limit * 2
end

def throttle_count
described_class.cache.read("#{counter_prefix}:#{throttle}:#{discriminator}") || 0
end

def counter_prefix
(Time.now.to_i / period.seconds).to_i
end

def increment_counter
described_class.cache.count("#{throttle}:#{discriminator}", period)
end
end

shared_examples 'throttled endpoint' do
include_examples 'with throttled endpoint base'

context 'when the number of requests is lower than the limit' do
before do
below_limit.times { increment_counter }
Expand Down Expand Up @@ -46,25 +70,32 @@ def app
expect(response).to_not have_http_status(429)
end
end
end

def below_limit
limit - 1
end
shared_examples 'does not throttle endpoint' do
include_examples 'with throttled endpoint base'

def above_limit
limit * 2
end
context 'when the number of requests is lower than the limit' do
before do
below_limit.times { increment_counter }
end

def throttle_count
described_class.cache.read("#{counter_prefix}:#{throttle}:#{discriminator}") || 0
end
it 'does not change the request status' do
expect { request.call }.to change { throttle_count }.by(0)

def counter_prefix
(Time.now.to_i / period.seconds).to_i
expect(response).to_not have_http_status(429)
end
end

def increment_counter
described_class.cache.count("#{throttle}:#{discriminator}", period)
context 'when the number of requests is higher than the limit' do
before do
above_limit.times { increment_counter }
end

it 'returns http too many requests after limit and returns to normal status after period' do
expect { request.call }.to change { throttle_count }.by(0)
expect(response).to_not have_http_status(429)
end
end
end

Expand Down Expand Up @@ -176,4 +207,24 @@ def increment_counter

it_behaves_like 'throttled endpoint'
end

describe 'throttle excessive emoji reaction requests by account' do
let(:user) { Fabricate(:user, email: '[email protected]') }
let(:throttle) { 'throttle_password_change/account' }
let(:limit) { 10 }
let(:period) { 10.minutes }
let(:request) { -> { put path, headers: { 'REMOTE_ADDR' => remote_ip } } }
let(:status) { Fabricate(:status) }
let(:emoji) { Fabricate(:custom_emoji) }
let(:path) { "/api/v1/statuses/#{status.id}/emoji_reactions/#{emoji.shortcode}" }
let(:discriminator) { user.id }

before do
sign_in user, scope: :user

get '/'
end

it_behaves_like 'does not throttle endpoint'
end
end

0 comments on commit 51155d6

Please sign in to comment.