From 96197bdc7bd45c6553c72d970691a0b36364ee6a Mon Sep 17 00:00:00 2001 From: Mattias Roback Date: Thu, 2 Jan 2025 16:19:07 +0100 Subject: [PATCH 1/2] Fix bug causing the bodies of all requests to be concatenated When I added the middleware that checks the response body size in #32, I introduced a bug where the response bodies for all requests in a redirect "chain" gets included in the body returned in the `Twingly::HTTP::Response` object. What I did here was to move the middleware after the FollowRedirects middleware, to ensure we run the middleware on each request, instead of handling all response body chunks at once. The test I added here (along with some other existing tests), would fail with the following error before this fix: 1) Twingly::HTTP::Client#post when given a host that redirects when following redirects only returns the body of the final request Failure/Error: expect(response.fetch(:body)).to eq("finished!") expected: "finished!" got: "redirect...finished!" (compared using ==) Shared Example Group: "common HTTP behaviour for" called from ./spec/lib/twingly/http_spec.rb:554 # ./spec/lib/twingly/http_spec.rb:159:in `block (5 levels) in ' --- lib/twingly/http.rb | 4 ++-- spec/lib/twingly/http_spec.rb | 15 ++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/twingly/http.rb b/lib/twingly/http.rb index 1bc6dc0..2d0ea89 100644 --- a/lib/twingly/http.rb +++ b/lib/twingly/http.rb @@ -200,8 +200,6 @@ def create_http_client # rubocop:disable Metrics/MethodLength exceptions: @retryable_exceptions, methods: [], # empty [] forces Faraday to run retry_if retry_if: retry_if - faraday.request :response_body_size_limit, - max_size_bytes: @max_response_body_size_bytes faraday.response :logfmt_logger, @logger.dup, headers: true, bodies: true, @@ -210,6 +208,8 @@ def create_http_client # rubocop:disable Metrics/MethodLength faraday.use FaradayMiddleware::FollowRedirects, limit: @follow_redirects_limit end + faraday.request :response_body_size_limit, + max_size_bytes: @max_response_body_size_bytes faraday.adapter Faraday.default_adapter faraday.headers[:user_agent] = user_agent end diff --git a/spec/lib/twingly/http_spec.rb b/spec/lib/twingly/http_spec.rb index 7031288..9937192 100644 --- a/spec/lib/twingly/http_spec.rb +++ b/spec/lib/twingly/http_spec.rb @@ -133,10 +133,11 @@ class CustomError < StandardError; end stub_request(:any, redir_url) .to_return(status: 302, - headers: { "Location" => "#{base_redir_url}#{n + 1}" }) + headers: { "Location" => "#{base_redir_url}#{n + 1}" }, + body: "redirect...") end - stub_request(:any, "#{base_redir_url}#{times}").to_return(status: 200) + stub_request(:any, "#{base_redir_url}#{times}").to_return(status: 200, body: "finished!") end end @@ -150,9 +151,13 @@ class CustomError < StandardError; end it do is_expected.to match(headers: {}, status: 200, - body: "", + body: "finished!", final_url: "http://redirect.1") end + + it "only returns the body of the final request" do + expect(response.fetch(:body)).to eq("finished!") + end end context "when not following redirects" do @@ -165,7 +170,7 @@ class CustomError < StandardError; end it do is_expected.to match(headers: { "location" => "http://redirect.1" }, status: 302, - body: "", + body: "redirect...", final_url: url) end end @@ -182,7 +187,7 @@ class CustomError < StandardError; end it do is_expected.to match(headers: {}, status: 200, - body: "", + body: "finished!", final_url: "http://redirect.5") end end From 144bbb8ef3c2ceb9519891c326a20c4305165d84 Mon Sep 17 00:00:00 2001 From: Mattias Roback Date: Thu, 2 Jan 2025 16:46:57 +0100 Subject: [PATCH 2/2] Register the "response body size limit" as a respomse middleware Before this it was registered as a request middleware. I didn't think about that when I added it in #32. This doesn't seem to make any difference, I just thought it makes sense to change this anyway, as this middleware works with the response. --- lib/faraday/response_body_size_limit/middleware.rb | 2 +- lib/twingly/http.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/faraday/response_body_size_limit/middleware.rb b/lib/faraday/response_body_size_limit/middleware.rb index b4599aa..dee28db 100644 --- a/lib/faraday/response_body_size_limit/middleware.rb +++ b/lib/faraday/response_body_size_limit/middleware.rb @@ -33,6 +33,6 @@ def call(env) # rubocop:disable Metrics/MethodLength end end -Faraday::Request.register_middleware( +Faraday::Response.register_middleware( response_body_size_limit: Faraday::ResponseBodySizeLimit::Middleware ) diff --git a/lib/twingly/http.rb b/lib/twingly/http.rb index 2d0ea89..ec1ec29 100644 --- a/lib/twingly/http.rb +++ b/lib/twingly/http.rb @@ -208,8 +208,8 @@ def create_http_client # rubocop:disable Metrics/MethodLength faraday.use FaradayMiddleware::FollowRedirects, limit: @follow_redirects_limit end - faraday.request :response_body_size_limit, - max_size_bytes: @max_response_body_size_bytes + faraday.response :response_body_size_limit, + max_size_bytes: @max_response_body_size_bytes faraday.adapter Faraday.default_adapter faraday.headers[:user_agent] = user_agent end