From 6ff888bd2c01e5d461c729e2ae814af196309453 Mon Sep 17 00:00:00 2001
From: Jarek Radosz <jradosz@gmail.com>
Date: Tue, 23 Mar 2021 20:32:36 +0100
Subject: [PATCH] DEV: Retry-after header values should be strings (#12475)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fixes `Rack::Lint::LintError: a header value must be a String, but the value of 'Retry-After' is a Integer`. (see: https://github.com/rack/rack/blob/14a236b4f0899e46bc41c4f80dcff29159a59312/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)
---
app/controllers/application_controller.rb | 2 +-
config/initializers/004-message_bus.rb | 2 +-
lib/middleware/request_tracker.rb | 2 +-
spec/components/middleware/request_tracker_spec.rb | 10 +++++-----
spec/integration/rate_limiting_spec.rb | 4 ++--
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index e2c4be1ad66..7fc969b31a3 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -166,7 +166,7 @@ class ApplicationController < ActionController::Base
type: :rate_limit,
status: 429,
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
diff --git a/config/initializers/004-message_bus.rb b/config/initializers/004-message_bus.rb
index 924fe6dc711..a566d4658f5 100644
--- a/config/initializers/004-message_bus.rb
+++ b/config/initializers/004-message_bus.rb
@@ -98,7 +98,7 @@ MessageBus.on_middleware_error do |env, e|
if Discourse::InvalidAccess === e
[403, {}, ["Invalid Access"]]
elsif RateLimiter::LimitExceeded === e
- [429, { 'Retry-After' => e.available_in }, [e.description]]
+ [429, { 'Retry-After' => e.available_in.to_s }, [e.description]]
end
end
diff --git a/lib/middleware/request_tracker.rb b/lib/middleware/request_tracker.rb
index 45231f63b91..d542ebd98a1 100644
--- a/lib/middleware/request_tracker.rb
+++ b/lib/middleware/request_tracker.rb
@@ -166,7 +166,7 @@ class Middleware::RequestTracker
if available_in = rate_limit(request)
return [
429,
- { "Retry-After" => available_in },
+ { "Retry-After" => available_in.to_s },
["Slow down, too many requests from this IP address"]
]
end
diff --git a/spec/components/middleware/request_tracker_spec.rb b/spec/components/middleware/request_tracker_spec.rb
index f321f75ec54..0396f3049be 100644
--- a/spec/components/middleware/request_tracker_spec.rb
+++ b/spec/components/middleware/request_tracker_spec.rb
@@ -16,6 +16,7 @@ describe Middleware::RequestTracker do
end
before do
+ ApplicationRequest.clear_cache!
ApplicationRequest.enable
end
@@ -69,7 +70,6 @@ describe Middleware::RequestTracker do
end
it "can log requests correctly" do
-
data = Middleware::RequestTracker.get_data(env(
"HTTP_USER_AGENT" => "AdsBot-Google (+http://www.google.com/adsbot.html)"
), ["200", { "Content-Type" => 'text/html' }], 0.1)
@@ -253,7 +253,7 @@ describe Middleware::RequestTracker do
expect(Rails.logger.warnings).to eq(1)
expect(status).to eq(429)
- expect(headers["Retry-After"]).to eq(10)
+ expect(headers["Retry-After"]).to eq("10")
end
it "does warn if rate limiter is enabled" do
@@ -282,13 +282,13 @@ describe Middleware::RequestTracker do
expect(status).to eq(200)
status, headers = middleware.call(env1)
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")
status, headers = middleware.call(env2)
expect(status).to eq(429)
- expect(headers["Retry-After"]).to eq(10)
+ expect(headers["Retry-After"]).to eq("10")
end
it "does block if rate limiter is enabled" do
@@ -303,7 +303,7 @@ describe Middleware::RequestTracker do
status, headers = middleware.call(env1)
expect(status).to eq(429)
- expect(headers["Retry-After"]).to eq(10)
+ expect(headers["Retry-After"]).to eq("10")
status, _ = middleware.call(env2)
expect(status).to eq(200)
diff --git a/spec/integration/rate_limiting_spec.rb b/spec/integration/rate_limiting_spec.rb
index a623d7aa228..353569ef61d 100644
--- a/spec/integration/rate_limiting_spec.rb
+++ b/spec/integration/rate_limiting_spec.rb
@@ -24,7 +24,7 @@ describe 'rate limiter integration' do
}
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
it "will not rate limit when all is good" do
@@ -76,7 +76,7 @@ describe 'rate limiter integration' do
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)
end
end