mirror of
https://github.com/discourse/discourse.git
synced 2025-06-06 23:07:28 +08:00
DEV: Retry-after header values should be strings (#12475)
Fixes `Rack::Lint::LintError: a header value must be a String, but the value of 'Retry-After' is a Integer`. (see: 14a236b4f0/lib/rack/lint.rb (L676)
)
I found it when I got flooded by those warning a while back in a test-related accident 😉 (ember CLI tests were hitting a local rails server at a fast rate)
This commit is contained in:
@ -166,7 +166,7 @@ class ApplicationController < ActionController::Base
|
|||||||
type: :rate_limit,
|
type: :rate_limit,
|
||||||
status: 429,
|
status: 429,
|
||||||
extras: { wait_seconds: retry_time_in_seconds },
|
extras: { wait_seconds: retry_time_in_seconds },
|
||||||
headers: { 'Retry-After': retry_time_in_seconds }
|
headers: { 'Retry-After': retry_time_in_seconds.to_s }
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -98,7 +98,7 @@ MessageBus.on_middleware_error do |env, e|
|
|||||||
if Discourse::InvalidAccess === e
|
if Discourse::InvalidAccess === e
|
||||||
[403, {}, ["Invalid Access"]]
|
[403, {}, ["Invalid Access"]]
|
||||||
elsif RateLimiter::LimitExceeded === e
|
elsif RateLimiter::LimitExceeded === e
|
||||||
[429, { 'Retry-After' => e.available_in }, [e.description]]
|
[429, { 'Retry-After' => e.available_in.to_s }, [e.description]]
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -166,7 +166,7 @@ class Middleware::RequestTracker
|
|||||||
if available_in = rate_limit(request)
|
if available_in = rate_limit(request)
|
||||||
return [
|
return [
|
||||||
429,
|
429,
|
||||||
{ "Retry-After" => available_in },
|
{ "Retry-After" => available_in.to_s },
|
||||||
["Slow down, too many requests from this IP address"]
|
["Slow down, too many requests from this IP address"]
|
||||||
]
|
]
|
||||||
end
|
end
|
||||||
|
@ -16,6 +16,7 @@ describe Middleware::RequestTracker do
|
|||||||
end
|
end
|
||||||
|
|
||||||
before do
|
before do
|
||||||
|
ApplicationRequest.clear_cache!
|
||||||
ApplicationRequest.enable
|
ApplicationRequest.enable
|
||||||
end
|
end
|
||||||
|
|
||||||
@ -69,7 +70,6 @@ describe Middleware::RequestTracker do
|
|||||||
end
|
end
|
||||||
|
|
||||||
it "can log requests correctly" do
|
it "can log requests correctly" do
|
||||||
|
|
||||||
data = Middleware::RequestTracker.get_data(env(
|
data = Middleware::RequestTracker.get_data(env(
|
||||||
"HTTP_USER_AGENT" => "AdsBot-Google (+http://www.google.com/adsbot.html)"
|
"HTTP_USER_AGENT" => "AdsBot-Google (+http://www.google.com/adsbot.html)"
|
||||||
), ["200", { "Content-Type" => 'text/html' }], 0.1)
|
), ["200", { "Content-Type" => 'text/html' }], 0.1)
|
||||||
@ -253,7 +253,7 @@ describe Middleware::RequestTracker do
|
|||||||
|
|
||||||
expect(Rails.logger.warnings).to eq(1)
|
expect(Rails.logger.warnings).to eq(1)
|
||||||
expect(status).to eq(429)
|
expect(status).to eq(429)
|
||||||
expect(headers["Retry-After"]).to eq(10)
|
expect(headers["Retry-After"]).to eq("10")
|
||||||
end
|
end
|
||||||
|
|
||||||
it "does warn if rate limiter is enabled" do
|
it "does warn if rate limiter is enabled" do
|
||||||
@ -282,13 +282,13 @@ describe Middleware::RequestTracker do
|
|||||||
expect(status).to eq(200)
|
expect(status).to eq(200)
|
||||||
status, headers = middleware.call(env1)
|
status, headers = middleware.call(env1)
|
||||||
expect(status).to eq(429)
|
expect(status).to eq(429)
|
||||||
expect(headers["Retry-After"]).to eq(10)
|
expect(headers["Retry-After"]).to eq("10")
|
||||||
|
|
||||||
env2 = env("REMOTE_ADDR" => "1.1.1.1")
|
env2 = env("REMOTE_ADDR" => "1.1.1.1")
|
||||||
|
|
||||||
status, headers = middleware.call(env2)
|
status, headers = middleware.call(env2)
|
||||||
expect(status).to eq(429)
|
expect(status).to eq(429)
|
||||||
expect(headers["Retry-After"]).to eq(10)
|
expect(headers["Retry-After"]).to eq("10")
|
||||||
end
|
end
|
||||||
|
|
||||||
it "does block if rate limiter is enabled" do
|
it "does block if rate limiter is enabled" do
|
||||||
@ -303,7 +303,7 @@ describe Middleware::RequestTracker do
|
|||||||
|
|
||||||
status, headers = middleware.call(env1)
|
status, headers = middleware.call(env1)
|
||||||
expect(status).to eq(429)
|
expect(status).to eq(429)
|
||||||
expect(headers["Retry-After"]).to eq(10)
|
expect(headers["Retry-After"]).to eq("10")
|
||||||
|
|
||||||
status, _ = middleware.call(env2)
|
status, _ = middleware.call(env2)
|
||||||
expect(status).to eq(200)
|
expect(status).to eq(200)
|
||||||
|
@ -24,7 +24,7 @@ describe 'rate limiter integration' do
|
|||||||
}
|
}
|
||||||
|
|
||||||
expect(response.status).to eq(429)
|
expect(response.status).to eq(429)
|
||||||
expect(response.headers['Retry-After']).to be > 29
|
expect(response.headers['Retry-After'].to_i).to be > 29
|
||||||
end
|
end
|
||||||
|
|
||||||
it "will not rate limit when all is good" do
|
it "will not rate limit when all is good" do
|
||||||
@ -76,7 +76,7 @@ describe 'rate limiter integration' do
|
|||||||
|
|
||||||
data = response.parsed_body
|
data = response.parsed_body
|
||||||
|
|
||||||
expect(response.headers['Retry-After']).to eq(60)
|
expect(response.headers["Retry-After"]).to eq("60")
|
||||||
expect(data["extras"]["wait_seconds"]).to eq(60)
|
expect(data["extras"]["wait_seconds"]).to eq(60)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
Reference in New Issue
Block a user