Skip to content

Commit 73ecd0c

Browse files
seanpdoylebyroot
authored andcommitted
RateLimiting: raise ActionController::TooManyRequests error
Prior to this commit, the default behavior for exceeding a rate limit involved responding with a status of [429 Too many requests][]. The only indication that a rate limit was exceeded was the `429` code. This commit changes the default behavior from a `head :too_many_requests` call to instead raise a new `ActionController::TooManyRequests` error. This change adheres more closely to precedent established by other status codes like `ActionController::BadRequest` to a `:bad_request` (`400 Bad Request`) status, or `ActiveRecord::RecordNotFound` to a `:not_found` (`404 Not Found`) status. Application-side controllers can be configured to `rescue_from` that exception, or they can rely on a new `ActionController::TooManyRequests`-to-`429 Too many requests` status mapping entry in the `ActionDispatch/Middleware/ExceptionWrapper` error-to-status mapping. [429 Too many requests]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/429
1 parent a721e89 commit 73ecd0c

File tree

7 files changed

+53
-22
lines changed

7 files changed

+53
-22
lines changed

actionpack/CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
* Raise `ActionController::TooManyRequests` error from `ActionController::RateLimiting`
2+
3+
Requests that exceed the rate limit raise an `ActionController::TooManyRequests` error.
4+
By default, Action Dispatch rescues the error and responds with a `429 Too Many Requests` status.
5+
6+
*Sean Doyle*
7+
18
* Add .md/.markdown as Markdown extensions and add a default `markdown:` renderer:
29

310
```ruby

actionpack/lib/action_controller/metal/exceptions.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,4 +103,9 @@ def initialize(message, controller, action_name)
103103
super(message)
104104
end
105105
end
106+
107+
# Raised when a Rate Limit is exceeded by too many requests within a period of
108+
# time.
109+
class TooManyRequests < ActionControllerError
110+
end
106111
end

actionpack/lib/action_controller/metal/rate_limiting.rb

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ module ClassMethods
2222
# share rate limits across multiple controllers, you can provide your own scope,
2323
# by passing value in the `scope:` parameter.
2424
#
25-
# Requests that exceed the rate limit are refused with a `429 Too Many Requests`
26-
# response. You can specialize this by passing a callable in the `with:`
25+
# Requests that exceed the rate limit will raise an `ActionController::TooManyRequests`
26+
# error. By default, Action Dispatch will rescue from the error and refuse the request
27+
# with a `429 Too Many Requests` response. You can specialize this by passing a callable in the `with:`
2728
# parameter. It's evaluated within the context of the controller processing the
2829
# request.
2930
#
@@ -57,7 +58,7 @@ module ClassMethods
5758
# rate_limit to: 3, within: 2.seconds, name: "short-term"
5859
# rate_limit to: 10, within: 5.minutes, name: "long-term"
5960
# end
60-
def rate_limit(to:, within:, by: -> { request.remote_ip }, with: -> { head :too_many_requests }, store: cache_store, name: nil, scope: nil, **options)
61+
def rate_limit(to:, within:, by: -> { request.remote_ip }, with: -> { raise TooManyRequests }, store: cache_store, name: nil, scope: nil, **options)
6162
before_action -> { rate_limiting(to: to, within: within, by: by, with: with, store: store, name: name, scope: scope || controller_path) }, **options
6263
end
6364
end

actionpack/lib/action_dispatch/middleware/exception_wrapper.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ class ExceptionWrapper
2323
"ActionDispatch::Http::Parameters::ParseError" => :bad_request,
2424
"ActionController::BadRequest" => :bad_request,
2525
"ActionController::ParameterMissing" => :bad_request,
26+
"ActionController::TooManyRequests" => :too_many_requests,
2627
"Rack::QueryParser::ParameterTypeError" => :bad_request,
2728
"Rack::QueryParser::InvalidParameterError" => :bad_request
2829
)

actionpack/test/controller/api/rate_limiting_test.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@ class ApiRateLimitingTest < ActionController::TestCase
2323
get :limited_to_two
2424
assert_response :ok
2525

26-
get :limited_to_two
27-
assert_response :too_many_requests
26+
assert_raises ActionController::TooManyRequests do
27+
get :limited_to_two
28+
end
2829
end
2930

3031
test "limit resets after time" do

actionpack/test/controller/rate_limiting_test.rb

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,9 @@ class RateLimitingTest < ActionController::TestCase
6666
get :limited
6767
assert_response :ok
6868

69-
get :limited
70-
assert_response :too_many_requests
69+
assert_raises ActionController::TooManyRequests do
70+
get :limited
71+
end
7172
end
7273

7374
test "notification on limit action" do
@@ -80,25 +81,27 @@ class RateLimitingTest < ActionController::TestCase
8081
within: 2.seconds,
8182
name: nil,
8283
by: request.remote_ip) do
83-
get :limited
84+
assert_raises ActionController::TooManyRequests do
85+
get :limited
86+
end
8487
end
8588
end
8689

8790
test "multiple rate limits" do
91+
freeze_time
8892
get :limited
8993
get :limited
9094
assert_response :ok
9195

92-
travel_to 3.seconds.from_now do
93-
get :limited
94-
get :limited
95-
assert_response :ok
96-
end
96+
travel 3.seconds
97+
get :limited
98+
get :limited
99+
assert_response :ok
97100

98-
travel_to 3.seconds.from_now do
99-
get :limited
101+
travel 3.seconds
102+
get :limited
103+
assert_raises ActionController::TooManyRequests do
100104
get :limited
101-
assert_response :too_many_requests
102105
end
103106
end
104107

@@ -140,13 +143,15 @@ class RateLimitingTest < ActionController::TestCase
140143

141144
@controller = RateLimitedSharedTwoController.new
142145

143-
get :limited_shared_two
144-
assert_response :too_many_requests
146+
assert_raises ActionController::TooManyRequests do
147+
get :limited_shared_two
148+
end
145149

146150
@controller = RateLimitedSharedOneController.new
147151

148-
get :limited_shared_one
149-
assert_response :too_many_requests
152+
assert_raises ActionController::TooManyRequests do
153+
get :limited_shared_one
154+
end
150155
ensure
151156
RateLimitedBaseController.cache_store.clear
152157
end
@@ -166,8 +171,9 @@ class RateLimitingTest < ActionController::TestCase
166171

167172
@controller = RateLimitedSharedThreeController.new
168173

169-
get :limited_shared_three
170-
assert_response :too_many_requests
174+
assert_raises ActionController::TooManyRequests do
175+
get :limited_shared_three
176+
end
171177
ensure
172178
RateLimitedSharedController.cache_store.clear
173179
end

actionpack/test/dispatch/show_exceptions_test.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ def call(env)
2727
rescue
2828
raise ActionView::Template::Error.new("template")
2929
end
30+
when "/rate_limited"
31+
raise ActionController::TooManyRequests.new
3032
else
3133
raise "puke!"
3234
end
@@ -41,6 +43,10 @@ def setup
4143
assert_raise RuntimeError do
4244
get "/", env: { "action_dispatch.show_exceptions" => :none }
4345
end
46+
47+
assert_raise ActionController::TooManyRequests do
48+
get "/rate_limited", headers: { "action_dispatch.show_exceptions" => :none }
49+
end
4450
end
4551

4652
test "rescue with error page" do
@@ -67,6 +73,10 @@ def setup
6773
get "/invalid_mimetype", headers: { "Accept" => "text/html,*", "action_dispatch.show_exceptions" => :all }
6874
assert_response 406
6975
assert_equal "", body
76+
77+
get "/rate_limited", headers: { "action_dispatch.show_exceptions" => :all }
78+
assert_response 429
79+
assert_equal "", body
7080
end
7181

7282
test "rescue with no body for HEAD requests" do

0 commit comments

Comments
 (0)