From f6eabd61b5b2b9b50f1d62291d7525d1ba51fb49 Mon Sep 17 00:00:00 2001 From: "vadim.kar" Date: Mon, 27 Apr 2026 15:21:00 +0300 Subject: [PATCH 1/9] adapt logic to both versions of http --- lib/ezclient.rb | 3 + lib/ezclient/persistent_client.rb | 10 +++- lib/ezclient/request.rb | 62 ++++++++++++++------- spec/ezclient_spec.rb | 91 +++++++++++++++++++++++++++++-- spec/spec_helper.rb | 47 ++++++++++++++++ 5 files changed, 185 insertions(+), 28 deletions(-) diff --git a/lib/ezclient.rb b/lib/ezclient.rb index 0b6f9c5..67918aa 100644 --- a/lib/ezclient.rb +++ b/lib/ezclient.rb @@ -12,6 +12,9 @@ require_relative "ezclient/check_options" module EzClient + # NOTE: Whether httprb v6+ is being used (has breaking API changes vs v4/v5) + HTTP_GEM_V6 = Gem::Version.new(HTTP::VERSION) >= Gem::Version.new("6.0.0") + def self.new(*args) Client.new(*args) end diff --git a/lib/ezclient/persistent_client.rb b/lib/ezclient/persistent_client.rb index 9f9fd1d..f7837e7 100644 --- a/lib/ezclient/persistent_client.rb +++ b/lib/ezclient/persistent_client.rb @@ -26,6 +26,14 @@ def timed_out? attr_accessor :origin, :keep_alive_timeout, :last_request_at def http_client - @http_client ||= HTTP.persistent(origin, timeout: keep_alive_timeout) + @http_client ||= + if EzClient::HTTP_GEM_V6 + # NOTE: In v6, HTTP.persistent returns HTTP::Session (no #perform(req, opts)). + # NOTE: Instead, create an HTTP::Client directly with persistent connection options. + HTTP::Client.new(persistent: origin, keep_alive_timeout: keep_alive_timeout) + else + # NOTE: In v4/v5, HTTP.persistent returns HTTP::Client directly. + HTTP.persistent(origin, timeout: keep_alive_timeout) + end end end diff --git a/lib/ezclient/request.rb b/lib/ezclient/request.rb index a298d91..79710f9 100644 --- a/lib/ezclient/request.rb +++ b/lib/ezclient/request.rb @@ -74,27 +74,43 @@ def http_options attr_accessor :client def http_request - @http_request ||= begin - opts = {} - - opts[verb == "GET" ? :params : :form] = options[:params] - opts[:json] = options[:json] if options[:json] - opts[:body] = options[:body] if options[:body] - opts[:params] = options[:query] if options[:query] - opts[:form] = options[:form] if options[:form] - opts[:form] = prepare_form_params(opts[:form]) if opts[:form] - opts[:headers] = prepare_headers(options[:headers]) - - http_client.build_request(verb, url, opts) - end + @http_request ||= + if EzClient::HTTP_GEM_V6 + # NOTE: build_request was removed from HTTP::Client in v6; use Request::Builder instead + merged = http_client.default_options.merge(build_request_opts) + HTTP::Request::Builder.new(merged).build(verb, url) + else + http_client.build_request(verb, url, build_request_opts) + end + end + + def build_request_opts + opts = {} + opts[verb == "GET" ? :params : :form] = options[:params] if options[:params] + opts[:json] = options[:json] if options[:json] + opts[:body] = options[:body] if options[:body] + opts[:params] = options[:query] if options[:query] + opts[:form] = options[:form] if options[:form] + opts[:form] = prepare_form_params(opts[:form]) if opts[:form] + opts[:headers] = prepare_headers(options[:headers]) + opts end def http_client - # Only used to build proper HTTP::Request and HTTP::Options instances + # NOTE: Only used to build proper HTTP::Request and HTTP::Options instances @http_client ||= begin http_client = client.dup http_client = set_timeout(http_client) - http_client = http_client.basic_auth(basic_auth) if basic_auth + if basic_auth + # NOTE: In v6, basic_auth takes keyword args (user:, pass:); + # NOTE: in v4/v5 it takes a positional hash + http_client = + if EzClient::HTTP_GEM_V6 + http_client.basic_auth(**basic_auth) + else + http_client.basic_auth(basic_auth) + end + end http_client = http_client.cookies(options[:cookies]) if options[:cookies] http_client end @@ -103,13 +119,17 @@ def http_client def perform_request perform_started_at = EzClient.get_time with_retry do - # Use original client so that connection can be reused + # NOTE: Use original client so that connection can be reused res = client.perform(http_request, http_options) return res unless follow - HTTP::Redirector.new(follow).perform(http_request, res) do |request| - client.perform(request, http_options) - end + # NOTE: In v6, Redirector.new takes keyword args; in v4/v5 it takes a positional hash + redirector = if EzClient::HTTP_GEM_V6 + HTTP::Redirector.new(**follow) + else + HTTP::Redirector.new(follow) + end + redirector.perform(http_request, res) { |req| client.perform(req, http_options) } end ensure self.elapsed_seconds = EzClient.get_time - perform_started_at @@ -132,8 +152,8 @@ def with_retry(&block) end def retry_on_connection_error - # This may result in 2 requests reaching the server so I hope HTTP fixes it - # https://github.com/httprb/http/issues/459 + # NOTE: This may result in 2 requests reaching the server + # NOTE: https://github.com/httprb/http/issues/459 yield rescue HTTP::ConnectionError => error on_retry.call(self, error, options[:metadata]) diff --git a/spec/ezclient_spec.rb b/spec/ezclient_spec.rb index 3fc9ee9..0ef3a46 100644 --- a/spec/ezclient_spec.rb +++ b/spec/ezclient_spec.rb @@ -396,12 +396,14 @@ def self.sign!(*); end context "object inspectation" do specify "#inspect" do - expect(response.inspect.gsub(/0x\w+/, "0x0000")).to eq(<<~TXT.gsub(/\s+/, " ").strip) - #, - @http_request=#, - @body=""> - TXT + inspected = response.inspect.gsub(/0x\w+/, "0x0000") + # NOTE: HTTP::Response#inspect format changed between httprb v5 and v6: + # NOTE: v5: "#" (shows headers as {}) + # NOTE: v6: "#" (shows mime_type, nil = empty) + expect(inspected).to include("#") + expect(inspected).to include('@body="">') end specify "#to_s" do @@ -548,4 +550,81 @@ def self.sign!(*); end expect(request.headers).to include("Authorization" => "Basic dXNlcjpwYXNzd29yZA==") end end + + # NOTE: The following contexts exercise httprb v6-specific code paths by stubbing + # NOTE: EzClient::HTTP_GEM_V6 = true, ensuring coverage even when running under httprb v5. + context "when HTTP_GEM_V6 is true (v6 code paths)" do + before do + stub_const("EzClient::HTTP_GEM_V6", true) + + # NOTE: HTTP::Request::Builder doesn't exist in httprb v5; stub it to delegate + # NOTE: to the v5 build_request API so requests remain WebMock-compatible. + stub_const("HTTP::Request::Builder", Class.new do + def initialize(opts) + @opts = opts + end + + def build(verb, url) + HTTP::Client.new.build_request(verb, url, @opts) + end + end) + end + + context "when making a basic request" do + before { request_stub.to_return(body: "v6 response") } + + it "performs request using HTTP::Request::Builder" do + response = request.perform + expect(response.body).to eq("v6 response") + end + end + + context "when basic_auth request option is provided" do + let(:request_options) { { basic_auth: { user: "user", pass: "password" } } } + + it "sets Authorization header using keyword args (v6 path)" do + expect(request.headers).to include("Authorization" => "Basic dXNlcjpwYXNzd29yZA==") + end + end + + context "when follow redirect" do + before do + request_stub.to_return(status: 302, headers: { "Location" => "http://redirect.me" }) + end + + let(:verb) { :get } + let(:request_options) { { follow: true } } + + before do + stub_request(:get, /redirect\.me/) + .with { |req| webmock_requests << req } + .to_return(body: "redirected") + end + + it "follows redirect using HTTP::Redirector with keyword args" do + request.perform + expect(webmock_requests.size).to eq(2) + end + end + end +end + +RSpec.describe EzClient::PersistentClient do + # NOTE: Exercises the httprb v6-specific code path in http_client by stubbing HTTP_GEM_V6. + context "when HTTP_GEM_V6 is true" do + before { stub_const("EzClient::HTTP_GEM_V6", true) } + + it "creates HTTP::Client with persistent connection options" do + mock_client = double("HTTP::Client") + allow(HTTP::Client).to receive(:new) + .with(persistent: "http://example.com", keep_alive_timeout: 5) + .and_return(mock_client) + + client = EzClient::PersistentClient.new("http://example.com", 5) + client.send(:http_client) + + expect(HTTP::Client).to have_received(:new) + .with(persistent: "http://example.com", keep_alive_timeout: 5) + end + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c7ba115..50d8905 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -19,6 +19,53 @@ require "webmock/rspec" require "ezclient" +# NOTE: WebMock (up to at least 3.24.0) has two incompatibilities with httprb v6: +# +# NOTE: 1. HTTP::Response.new changed from accepting a positional Hash to keyword arguments. +# NOTE: WebMock calls `new({status: ..., version: ..., ...})` which raises ArgumentError in v6. +# +# NOTE: 2. HTTP::Response::Body#read_contents (and #readpartial) expect the underlying stream's +# NOTE: #readpartial to raise EOFError at end-of-stream (per the v6 IO#readpartial contract), +# NOTE: but WebMock's Streamer returns nil, causing TypeError: no implicit conversion of nil +# NOTE: into String. +if EzClient::HTTP_GEM_V6 + module HTTP + class Response + class << self + def from_webmock(request, webmock_response, _request_signature = nil) + status = Status.new(webmock_response.status.first) + headers = webmock_response.headers || {} + body = build_http_rb_response_body_from_webmock_response(webmock_response) + + new( + status: status, + version: "1.1", + headers: headers, + body: body, + request: request, + ) + end + end + end + + class Response + class Streamer + # NOTE: httprb v6 requires readpartial to raise EOFError at end-of-stream + # NOTE: (matching the IO#readpartial contract) instead of returning nil. + # NOTE: StringIO#read(nil) returns "" at EOF (not nil), so we must check eof? first. + def readpartial(size = nil, outbuf = nil) + raise EOFError, "end of stream reached" if @io.eof? + + chunk = size ? @io.read(size, outbuf) : @io.read + raise EOFError, "end of stream reached" if chunk.nil? + + chunk.force_encoding(@encoding) + end + end + end + end +end + RSpec.configure do |config| config.order = :random Kernel.srand config.seed From 937fcfe374ffcae0aefe0ca1b113a7a53dd8dc33 Mon Sep 17 00:00:00 2001 From: "vadim.kar" Date: Tue, 28 Apr 2026 10:50:35 +0300 Subject: [PATCH 2/9] changed comments --- lib/ezclient.rb | 1 - lib/ezclient/persistent_client.rb | 4 +--- lib/ezclient/request.rb | 15 ++++++--------- spec/ezclient_spec.rb | 16 ++++++++-------- spec/spec_helper.rb | 18 ++++++++---------- 5 files changed, 23 insertions(+), 31 deletions(-) diff --git a/lib/ezclient.rb b/lib/ezclient.rb index 67918aa..984855a 100644 --- a/lib/ezclient.rb +++ b/lib/ezclient.rb @@ -12,7 +12,6 @@ require_relative "ezclient/check_options" module EzClient - # NOTE: Whether httprb v6+ is being used (has breaking API changes vs v4/v5) HTTP_GEM_V6 = Gem::Version.new(HTTP::VERSION) >= Gem::Version.new("6.0.0") def self.new(*args) diff --git a/lib/ezclient/persistent_client.rb b/lib/ezclient/persistent_client.rb index f7837e7..5fd5065 100644 --- a/lib/ezclient/persistent_client.rb +++ b/lib/ezclient/persistent_client.rb @@ -28,11 +28,9 @@ def timed_out? def http_client @http_client ||= if EzClient::HTTP_GEM_V6 - # NOTE: In v6, HTTP.persistent returns HTTP::Session (no #perform(req, opts)). - # NOTE: Instead, create an HTTP::Client directly with persistent connection options. + # In v6, HTTP.persistent returns HTTP::Session; use HTTP::Client directly instead HTTP::Client.new(persistent: origin, keep_alive_timeout: keep_alive_timeout) else - # NOTE: In v4/v5, HTTP.persistent returns HTTP::Client directly. HTTP.persistent(origin, timeout: keep_alive_timeout) end end diff --git a/lib/ezclient/request.rb b/lib/ezclient/request.rb index 79710f9..0f55eaa 100644 --- a/lib/ezclient/request.rb +++ b/lib/ezclient/request.rb @@ -76,7 +76,7 @@ def http_options def http_request @http_request ||= if EzClient::HTTP_GEM_V6 - # NOTE: build_request was removed from HTTP::Client in v6; use Request::Builder instead + # build_request was removed from HTTP::Client in v6; use Request::Builder instead merged = http_client.default_options.merge(build_request_opts) HTTP::Request::Builder.new(merged).build(verb, url) else @@ -97,13 +97,11 @@ def build_request_opts end def http_client - # NOTE: Only used to build proper HTTP::Request and HTTP::Options instances @http_client ||= begin http_client = client.dup http_client = set_timeout(http_client) if basic_auth - # NOTE: In v6, basic_auth takes keyword args (user:, pass:); - # NOTE: in v4/v5 it takes a positional hash + # In v6, basic_auth takes keyword args (user:, pass:); in v4/v5 it takes a positional hash http_client = if EzClient::HTTP_GEM_V6 http_client.basic_auth(**basic_auth) @@ -119,11 +117,11 @@ def http_client def perform_request perform_started_at = EzClient.get_time with_retry do - # NOTE: Use original client so that connection can be reused + # Use original client so that connection can be reused res = client.perform(http_request, http_options) return res unless follow - # NOTE: In v6, Redirector.new takes keyword args; in v4/v5 it takes a positional hash + # In v6, Redirector.new takes keyword args; in v4/v5 it takes a positional hash redirector = if EzClient::HTTP_GEM_V6 HTTP::Redirector.new(**follow) else @@ -152,8 +150,8 @@ def with_retry(&block) end def retry_on_connection_error - # NOTE: This may result in 2 requests reaching the server - # NOTE: https://github.com/httprb/http/issues/459 + # This may result in 2 requests reaching the server so I hope HTTP fixes it + # https://github.com/httprb/http/issues/459 yield rescue HTTP::ConnectionError => error on_retry.call(self, error, options[:metadata]) @@ -207,7 +205,6 @@ def prepare_headers(headers) def prepare_form_params(original_params) params = {} - # NOTE: use Hash#transform_values after Ruby 2.3 support is dropped original_params.each do |key, value| params[key] = if value.is_a?(File) diff --git a/spec/ezclient_spec.rb b/spec/ezclient_spec.rb index 0ef3a46..585e85c 100644 --- a/spec/ezclient_spec.rb +++ b/spec/ezclient_spec.rb @@ -397,9 +397,9 @@ def self.sign!(*); end context "object inspectation" do specify "#inspect" do inspected = response.inspect.gsub(/0x\w+/, "0x0000") - # NOTE: HTTP::Response#inspect format changed between httprb v5 and v6: - # NOTE: v5: "#" (shows headers as {}) - # NOTE: v6: "#" (shows mime_type, nil = empty) + # HTTP::Response#inspect format changed between httprb v5 and v6: + # v5: "#" (shows headers as {}) + # v6: "#" (shows mime_type, nil = empty) expect(inspected).to include("#") @@ -551,14 +551,14 @@ def self.sign!(*); end end end - # NOTE: The following contexts exercise httprb v6-specific code paths by stubbing - # NOTE: EzClient::HTTP_GEM_V6 = true, ensuring coverage even when running under httprb v5. + # The following contexts exercise httprb v6-specific code paths by stubbing + # EzClient::HTTP_GEM_V6 = true, ensuring coverage even when running under httprb v5. context "when HTTP_GEM_V6 is true (v6 code paths)" do before do stub_const("EzClient::HTTP_GEM_V6", true) - # NOTE: HTTP::Request::Builder doesn't exist in httprb v5; stub it to delegate - # NOTE: to the v5 build_request API so requests remain WebMock-compatible. + # HTTP::Request::Builder doesn't exist in httprb v5; stub it to delegate + # to the v5 build_request API so requests remain WebMock-compatible. stub_const("HTTP::Request::Builder", Class.new do def initialize(opts) @opts = opts @@ -610,7 +610,7 @@ def build(verb, url) end RSpec.describe EzClient::PersistentClient do - # NOTE: Exercises the httprb v6-specific code path in http_client by stubbing HTTP_GEM_V6. + # Exercises the httprb v6-specific code path in http_client by stubbing HTTP_GEM_V6. context "when HTTP_GEM_V6 is true" do before { stub_const("EzClient::HTTP_GEM_V6", true) } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 50d8905..863bdb2 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -19,15 +19,15 @@ require "webmock/rspec" require "ezclient" -# NOTE: WebMock (up to at least 3.24.0) has two incompatibilities with httprb v6: +# WebMock (up to at least 3.24.0) has two incompatibilities with httprb v6: # -# NOTE: 1. HTTP::Response.new changed from accepting a positional Hash to keyword arguments. -# NOTE: WebMock calls `new({status: ..., version: ..., ...})` which raises ArgumentError in v6. +# 1. HTTP::Response.new changed from accepting a positional Hash to keyword arguments. +# WebMock calls `new({status: ..., version: ..., ...})` which raises ArgumentError in v6. # -# NOTE: 2. HTTP::Response::Body#read_contents (and #readpartial) expect the underlying stream's -# NOTE: #readpartial to raise EOFError at end-of-stream (per the v6 IO#readpartial contract), -# NOTE: but WebMock's Streamer returns nil, causing TypeError: no implicit conversion of nil -# NOTE: into String. +# 2. HTTP::Response::Body#read_contents (and #readpartial) expect the underlying stream's +# #readpartial to raise EOFError at end-of-stream (per the v6 IO#readpartial contract), +# but WebMock's Streamer returns nil, causing TypeError: no implicit conversion of nil +# into String. if EzClient::HTTP_GEM_V6 module HTTP class Response @@ -50,9 +50,7 @@ def from_webmock(request, webmock_response, _request_signature = nil) class Response class Streamer - # NOTE: httprb v6 requires readpartial to raise EOFError at end-of-stream - # NOTE: (matching the IO#readpartial contract) instead of returning nil. - # NOTE: StringIO#read(nil) returns "" at EOF (not nil), so we must check eof? first. + # httprb v6 requires readpartial to raise EOFError at end-of-stream instead of returning nil def readpartial(size = nil, outbuf = nil) raise EOFError, "end of stream reached" if @io.eof? From d4accb3ae185ea768a14731ac51593e77def5c13 Mon Sep 17 00:00:00 2001 From: "vadim.kar" Date: Tue, 19 May 2026 15:42:32 +0300 Subject: [PATCH 3/9] fix build request --- lib/ezclient.rb | 2 +- lib/ezclient/persistent_client.rb | 6 ++--- lib/ezclient/request.rb | 18 +++++++-------- spec/ezclient_spec.rb | 38 +++++++++++++++---------------- spec/spec_helper.rb | 2 +- 5 files changed, 33 insertions(+), 33 deletions(-) diff --git a/lib/ezclient.rb b/lib/ezclient.rb index 984855a..07c3f9c 100644 --- a/lib/ezclient.rb +++ b/lib/ezclient.rb @@ -12,7 +12,7 @@ require_relative "ezclient/check_options" module EzClient - HTTP_GEM_V6 = Gem::Version.new(HTTP::VERSION) >= Gem::Version.new("6.0.0") + HTTP_CLIENT_SUPPORTS_BUILD_REQUEST = HTTP::Client.method_defined?(:build_request) def self.new(*args) Client.new(*args) diff --git a/lib/ezclient/persistent_client.rb b/lib/ezclient/persistent_client.rb index 5fd5065..dfc7df1 100644 --- a/lib/ezclient/persistent_client.rb +++ b/lib/ezclient/persistent_client.rb @@ -27,11 +27,11 @@ def timed_out? def http_client @http_client ||= - if EzClient::HTTP_GEM_V6 + if EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST + HTTP.persistent(origin, timeout: keep_alive_timeout) + else # In v6, HTTP.persistent returns HTTP::Session; use HTTP::Client directly instead HTTP::Client.new(persistent: origin, keep_alive_timeout: keep_alive_timeout) - else - HTTP.persistent(origin, timeout: keep_alive_timeout) end end end diff --git a/lib/ezclient/request.rb b/lib/ezclient/request.rb index 0f55eaa..010c2c5 100644 --- a/lib/ezclient/request.rb +++ b/lib/ezclient/request.rb @@ -75,12 +75,12 @@ def http_options def http_request @http_request ||= - if EzClient::HTTP_GEM_V6 + if EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST + http_client.build_request(verb, url, build_request_opts) + else # build_request was removed from HTTP::Client in v6; use Request::Builder instead merged = http_client.default_options.merge(build_request_opts) HTTP::Request::Builder.new(merged).build(verb, url) - else - http_client.build_request(verb, url, build_request_opts) end end @@ -103,10 +103,10 @@ def http_client if basic_auth # In v6, basic_auth takes keyword args (user:, pass:); in v4/v5 it takes a positional hash http_client = - if EzClient::HTTP_GEM_V6 - http_client.basic_auth(**basic_auth) - else + if EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST http_client.basic_auth(basic_auth) + else + http_client.basic_auth(**basic_auth) end end http_client = http_client.cookies(options[:cookies]) if options[:cookies] @@ -122,10 +122,10 @@ def perform_request return res unless follow # In v6, Redirector.new takes keyword args; in v4/v5 it takes a positional hash - redirector = if EzClient::HTTP_GEM_V6 - HTTP::Redirector.new(**follow) - else + redirector = if EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST HTTP::Redirector.new(follow) + else + HTTP::Redirector.new(**follow) end redirector.perform(http_request, res) { |req| client.perform(req, http_options) } end diff --git a/spec/ezclient_spec.rb b/spec/ezclient_spec.rb index 585e85c..d69b050 100644 --- a/spec/ezclient_spec.rb +++ b/spec/ezclient_spec.rb @@ -131,9 +131,8 @@ def self.sign!(*); end request.perform expect(webmock_requests.last.body).to eq('{"a":1}') - expect(webmock_requests.last.headers).to include( - "Content-Type" => "application/json; charset=utf-8", - ) + expect(webmock_requests.last.headers["Content-Type"].downcase) + .to eq("application/json; charset=utf-8") end end @@ -552,22 +551,22 @@ def self.sign!(*); end end # The following contexts exercise httprb v6-specific code paths by stubbing - # EzClient::HTTP_GEM_V6 = true, ensuring coverage even when running under httprb v5. - context "when HTTP_GEM_V6 is true (v6 code paths)" do + # HTTP::Client#build_request as unsupported, ensuring coverage even when running under httprb v5. + context "when HTTP::Client#build_request is unsupported" do before do - stub_const("EzClient::HTTP_GEM_V6", true) + stub_const("EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST", false) - # HTTP::Request::Builder doesn't exist in httprb v5; stub it to delegate - # to the v5 build_request API so requests remain WebMock-compatible. - stub_const("HTTP::Request::Builder", Class.new do - def initialize(opts) - @opts = opts - end + unless defined?(HTTP::Request::Builder) + stub_const("HTTP::Request::Builder", Class.new do + def initialize(opts) + @opts = opts + end - def build(verb, url) - HTTP::Client.new.build_request(verb, url, @opts) - end - end) + def build(verb, url) + HTTP::Client.new.build_request(verb, url, @opts) + end + end) + end end context "when making a basic request" do @@ -610,9 +609,10 @@ def build(verb, url) end RSpec.describe EzClient::PersistentClient do - # Exercises the httprb v6-specific code path in http_client by stubbing HTTP_GEM_V6. - context "when HTTP_GEM_V6 is true" do - before { stub_const("EzClient::HTTP_GEM_V6", true) } + # Exercises the httprb v6-specific code path in http_client by stubbing + # build_request as unsupported. + context "when HTTP::Client#build_request is unsupported" do + before { stub_const("EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST", false) } it "creates HTTP::Client with persistent connection options" do mock_client = double("HTTP::Client") diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 863bdb2..e513847 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -28,7 +28,7 @@ # #readpartial to raise EOFError at end-of-stream (per the v6 IO#readpartial contract), # but WebMock's Streamer returns nil, causing TypeError: no implicit conversion of nil # into String. -if EzClient::HTTP_GEM_V6 +unless EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST module HTTP class Response class << self From 1092318664a5694dd127ebe5a0080567c08df16f Mon Sep 17 00:00:00 2001 From: "vadim.kar" Date: Tue, 19 May 2026 16:20:56 +0300 Subject: [PATCH 4/9] fix cookies + refactor --- lib/ezclient/request.rb | 79 +++++++++++++++++++++++++++++++++++++---- spec/ezclient_spec.rb | 21 +++++++++++ 2 files changed, 93 insertions(+), 7 deletions(-) diff --git a/lib/ezclient/request.rb b/lib/ezclient/request.rb index 010c2c5..7ca0ffa 100644 --- a/lib/ezclient/request.rb +++ b/lib/ezclient/request.rb @@ -121,18 +121,83 @@ def perform_request res = client.perform(http_request, http_options) return res unless follow - # In v6, Redirector.new takes keyword args; in v4/v5 it takes a positional hash - redirector = if EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST - HTTP::Redirector.new(follow) - else - HTTP::Redirector.new(**follow) - end - redirector.perform(http_request, res) { |req| client.perform(req, http_options) } + perform_redirects(res) end ensure self.elapsed_seconds = EzClient.get_time - perform_started_at end + def perform_redirects(response) + if EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST + redirector(follow).perform(http_request, response) { |req| client.perform(req, http_options) } + else + perform_redirects_with_cookies(response) + end + end + + def perform_redirects_with_cookies(response) + cookie_jar = HTTP::CookieJar.new + store_request_cookies(cookie_jar, http_request) + store_response_cookies(cookie_jar, response) + + applied_redirects = {}.compare_by_identity + options = follow + options = options.merge( + on_redirect: redirect_callback(cookie_jar, options[:on_redirect], applied_redirects), + ) + + redirector(options).perform(http_request, response) do |req| + apply_cookies(cookie_jar, req) unless applied_redirects.delete(req) + client.perform(req, http_options).tap do |res| + store_response_cookies(cookie_jar, res) + end + end + end + + def redirector(options) + # In v6, Redirector.new takes keyword args; in v4/v5 it takes a positional hash + if EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST + HTTP::Redirector.new(options) + else + HTTP::Redirector.new(**options) + end + end + + def redirect_callback(cookie_jar, callback, applied_redirects) + proc do |response, request| + apply_cookies(cookie_jar, request) + applied_redirects[request] = true + callback&.call(response, request) + end + end + + def store_request_cookies(cookie_jar, request) + header = request.headers[HTTP::Headers::COOKIE].to_s + + HTTP::Cookie.cookie_value_to_hash(header).each do |name, value| + cookie_jar.add(HTTP::Cookie.new(name, value, path: request.uri.path, domain: request.host)) + end + end + + def store_response_cookies(cookie_jar, response) + response.cookies.each do |cookie| + if cookie.value == "" + cookie_jar.delete(cookie) + else + cookie_jar.add(cookie) + end + end + end + + def apply_cookies(cookie_jar, request) + if cookie_jar.empty? + request.headers.delete(HTTP::Headers::COOKIE) + else + cookies = cookie_jar.map { |cookie| "#{cookie.name}=#{cookie.value}" }.join("; ") + request.headers.set(HTTP::Headers::COOKIE, cookies) + end + end + def with_retry(&block) retries = 0 diff --git a/spec/ezclient_spec.rb b/spec/ezclient_spec.rb index d69b050..9af9fc6 100644 --- a/spec/ezclient_spec.rb +++ b/spec/ezclient_spec.rb @@ -605,6 +605,27 @@ def build(verb, url) expect(webmock_requests.size).to eq(2) end end + + context "when followed redirect sets cookies" do + before do + request_stub.to_return( + status: 302, + headers: { "Location" => "http://example.com/redirected", "Set-Cookie" => "sid=1" }, + ) + + stub_request(:get, "http://example.com/redirected") + .with { |req| webmock_requests << req } + .to_return(body: "redirected") + end + + let(:verb) { :get } + let(:request_options) { { follow: true } } + + it "sends response cookies to the next request" do + request.perform + expect(webmock_requests.last.headers).to include("Cookie" => "sid=1") + end + end end end From c0f16724674b69a9c42e3fdf6b69e20928904493 Mon Sep 17 00:00:00 2001 From: "vadim.kar" Date: Wed, 20 May 2026 13:38:51 +0300 Subject: [PATCH 5/9] fix api auth --- lib/ezclient/request.rb | 16 +++++++++++++++- spec/ezclient_spec.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/lib/ezclient/request.rb b/lib/ezclient/request.rb index 7ca0ffa..bc0fe38 100644 --- a/lib/ezclient/request.rb +++ b/lib/ezclient/request.rb @@ -43,7 +43,8 @@ def perform! def api_auth!(*args) raise "ApiAuth gem is not loaded" unless defined?(ApiAuth) - ApiAuth.sign!(http_request, *args) + + ApiAuth.sign!(api_auth_request, *args) self end @@ -73,6 +74,19 @@ def http_options attr_accessor :client + def api_auth_request + http_request.tap { |request| define_api_auth_header_accessors(request) } + end + + def define_api_auth_header_accessors(request) + # api-auth 2.x expects HTTP::Request to expose header accessors that were removed in httprb 6. + request.define_singleton_method(:[]) { |key| headers[key] } unless request.respond_to?(:[]) + + return if request.respond_to?(:[]=) + + request.define_singleton_method(:[]=) { |key, value| headers[key] = value } + end + def http_request @http_request ||= if EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST diff --git a/spec/ezclient_spec.rb b/spec/ezclient_spec.rb index 9af9fc6..f184e36 100644 --- a/spec/ezclient_spec.rb +++ b/spec/ezclient_spec.rb @@ -369,6 +369,33 @@ def self.sign!(*); end expect(request.headers).to include("Authorization" => "some-hash-here") end + + context "when HTTP::Request does not expose header accessors" do + let(:client_options) { {} } + let(:http_request) { Struct.new(:headers).new(HTTP::Headers.new) } + + before do + def http_request.respond_to?(name, *args) + return false if %i[[] []=].include?(name) + + super + end + + allow(request).to receive(:http_request).and_return(http_request) + end + + it "adds api-auth-compatible header accessors" do + expect(ApiAuth).to receive(:sign!) do |signed_request, access_id, access_key| + expect(access_id).to eq("id") + expect(access_key).to eq("secret") + signed_request["Authorization"] = "some-hash-here" + end + + request.api_auth!("id", "secret") + + expect(http_request.headers.to_h).to include("Authorization" => "some-hash-here") + end + end end context "when unknown client option is passed" do From ced82ca5df3037093e360ea18468d0ea1dafeda2 Mon Sep 17 00:00:00 2001 From: "vadim.kar" Date: Wed, 20 May 2026 16:55:39 +0300 Subject: [PATCH 6/9] another fix --- lib/ezclient.rb | 3 +- lib/ezclient/httprb_compatibility.rb | 102 +++++++++++++ lib/ezclient/persistent_client.rb | 10 +- lib/ezclient/request.rb | 33 ++--- spec/ezclient_spec.rb | 209 ++++++++++++++++++++++++++- spec/spec_helper.rb | 4 +- 6 files changed, 318 insertions(+), 43 deletions(-) create mode 100644 lib/ezclient/httprb_compatibility.rb diff --git a/lib/ezclient.rb b/lib/ezclient.rb index 07c3f9c..fb57bf5 100644 --- a/lib/ezclient.rb +++ b/lib/ezclient.rb @@ -3,6 +3,7 @@ require "http" require_relative "ezclient/version" +require_relative "ezclient/httprb_compatibility" require_relative "ezclient/client" require_relative "ezclient/persistent_client" require_relative "ezclient/persistent_client_registry" @@ -12,7 +13,7 @@ require_relative "ezclient/check_options" module EzClient - HTTP_CLIENT_SUPPORTS_BUILD_REQUEST = HTTP::Client.method_defined?(:build_request) + HTTP_CLIENT_SUPPORTS_BUILD_REQUEST = HttprbCompatibility.client_supports_build_request? def self.new(*args) Client.new(*args) diff --git a/lib/ezclient/httprb_compatibility.rb b/lib/ezclient/httprb_compatibility.rb new file mode 100644 index 0000000..6759bee --- /dev/null +++ b/lib/ezclient/httprb_compatibility.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +module EzClient::HttprbCompatibility + KEYWORD_PARAMETER_TYPES = %i[key keyreq].freeze + + module_function + + def install! + install_legacy_hash_initializer!(HTTP::Response) + install_legacy_hash_initializer!(HTTP::Request) + install_legacy_hash_initializer!(HTTP::Redirector) + install_legacy_header_accessors!(HTTP::Response) + install_legacy_header_accessors!(HTTP::Request) + end + + def client_supports_build_request? + HTTP::Client.method_defined?(:build_request) + end + + def response_body_requires_eof_error? + Gem::Version.new(HTTP::VERSION) >= Gem::Version.new("6") + end + + def build_request(client, verb, url, opts) + if client_supports_build_request? + client.build_request(verb, url, opts) + else + HTTP::Request::Builder.new(client.default_options.merge(opts)).build(verb, url) + end + end + + def basic_auth(client, opts) + if keyword_initializer?(client.method(:basic_auth)) + client.basic_auth(**opts) + else + client.basic_auth(opts) + end + end + + def redirector(opts) + if keyword_initializer?(HTTP::Redirector.instance_method(:initialize)) + HTTP::Redirector.new(**opts) + else + HTTP::Redirector.new(opts) + end + end + + def persistent_client(origin, keep_alive_timeout) + if client_supports_build_request? + HTTP.persistent(origin, timeout: keep_alive_timeout) + else + HTTP::Client.new(persistent: origin, keep_alive_timeout: keep_alive_timeout) + end + end + + def response(**attrs) + if keyword_initializer?(HTTP::Response.instance_method(:initialize)) + HTTP::Response.new(**attrs) + else + HTTP::Response.new(attrs) + end + end + + def keyword_initializer?(method) + method.parameters.any? { |type, _name| KEYWORD_PARAMETER_TYPES.include?(type) } + end + + def install_legacy_hash_initializer!(klass) + return unless keyword_initializer?(klass.instance_method(:initialize)) + return if klass < LegacyHashInitializer + + klass.prepend(LegacyHashInitializer) + end + + def install_legacy_header_accessors!(klass) + return if klass.method_defined?(:[]) && klass.method_defined?(:[]=) + + klass.include(LegacyHeaderAccessors) + end + + module LegacyHashInitializer + def initialize(*args, **kwargs) + if kwargs.empty? && args.size == 1 && args.first.respond_to?(:to_hash) + super(**args.first.to_hash) + else + super + end + end + end + + module LegacyHeaderAccessors + def [](key) + headers[key] + end + + def []=(key, value) + headers[key] = value + end + end +end + +EzClient::HttprbCompatibility.install! diff --git a/lib/ezclient/persistent_client.rb b/lib/ezclient/persistent_client.rb index dfc7df1..4ddfb49 100644 --- a/lib/ezclient/persistent_client.rb +++ b/lib/ezclient/persistent_client.rb @@ -3,7 +3,7 @@ class EzClient::PersistentClient extend Forwardable - def_delegators :http_client, :build_request, :default_options, :timeout + def_delegators :http_client, :basic_auth, :build_request, :cookies, :default_options, :timeout def initialize(origin, keep_alive_timeout) self.origin = origin @@ -26,12 +26,6 @@ def timed_out? attr_accessor :origin, :keep_alive_timeout, :last_request_at def http_client - @http_client ||= - if EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST - HTTP.persistent(origin, timeout: keep_alive_timeout) - else - # In v6, HTTP.persistent returns HTTP::Session; use HTTP::Client directly instead - HTTP::Client.new(persistent: origin, keep_alive_timeout: keep_alive_timeout) - end + @http_client ||= EzClient::HttprbCompatibility.persistent_client(origin, keep_alive_timeout) end end diff --git a/lib/ezclient/request.rb b/lib/ezclient/request.rb index bc0fe38..b29920e 100644 --- a/lib/ezclient/request.rb +++ b/lib/ezclient/request.rb @@ -88,14 +88,12 @@ def define_api_auth_header_accessors(request) end def http_request - @http_request ||= - if EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST - http_client.build_request(verb, url, build_request_opts) - else - # build_request was removed from HTTP::Client in v6; use Request::Builder instead - merged = http_client.default_options.merge(build_request_opts) - HTTP::Request::Builder.new(merged).build(verb, url) - end + @http_request ||= EzClient::HttprbCompatibility.build_request( + http_client, + verb, + url, + build_request_opts, + ) end def build_request_opts @@ -114,15 +112,7 @@ def http_client @http_client ||= begin http_client = client.dup http_client = set_timeout(http_client) - if basic_auth - # In v6, basic_auth takes keyword args (user:, pass:); in v4/v5 it takes a positional hash - http_client = - if EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST - http_client.basic_auth(basic_auth) - else - http_client.basic_auth(**basic_auth) - end - end + http_client = EzClient::HttprbCompatibility.basic_auth(http_client, basic_auth) if basic_auth http_client = http_client.cookies(options[:cookies]) if options[:cookies] http_client end @@ -142,7 +132,7 @@ def perform_request end def perform_redirects(response) - if EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST + if EzClient::HttprbCompatibility.client_supports_build_request? redirector(follow).perform(http_request, response) { |req| client.perform(req, http_options) } else perform_redirects_with_cookies(response) @@ -169,12 +159,7 @@ def perform_redirects_with_cookies(response) end def redirector(options) - # In v6, Redirector.new takes keyword args; in v4/v5 it takes a positional hash - if EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST - HTTP::Redirector.new(options) - else - HTTP::Redirector.new(**options) - end + EzClient::HttprbCompatibility.redirector(options) end def redirect_callback(cookie_jar, callback, applied_redirects) diff --git a/spec/ezclient_spec.rb b/spec/ezclient_spec.rb index f184e36..17ae622 100644 --- a/spec/ezclient_spec.rb +++ b/spec/ezclient_spec.rb @@ -239,6 +239,19 @@ def self.sign!(*); end response = request.perform! expect(response.body).to eq("some body") end + + context "when basic_auth and cookies are provided" do + let(:request_options) { { basic_auth: %w[user password], cookies: { a: 1 } } } + + it "uses them while building a persistent request" do + request.perform + + expect(webmock_requests.last.headers).to include( + "Authorization" => "Basic dXNlcjpwYXNzd29yZA==", + "Cookie" => "a=1", + ) + end + end end end @@ -375,12 +388,9 @@ def self.sign!(*); end let(:http_request) { Struct.new(:headers).new(HTTP::Headers.new) } before do - def http_request.respond_to?(name, *args) - return false if %i[[] []=].include?(name) - - super - end - + allow(http_request).to receive(:respond_to?).and_call_original + allow(http_request).to receive(:respond_to?).with(:[]).and_return(false) + allow(http_request).to receive(:respond_to?).with(:[]=).and_return(false) allow(request).to receive(:http_request).and_return(http_request) end @@ -581,7 +591,9 @@ def http_request.respond_to?(name, *args) # HTTP::Client#build_request as unsupported, ensuring coverage even when running under httprb v5. context "when HTTP::Client#build_request is unsupported" do before do - stub_const("EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST", false) + allow(EzClient::HttprbCompatibility) + .to receive(:client_supports_build_request?) + .and_return(false) unless defined?(HTTP::Request::Builder) stub_const("HTTP::Request::Builder", Class.new do @@ -653,6 +665,183 @@ def build(verb, url) expect(webmock_requests.last.headers).to include("Cookie" => "sid=1") end end + + context "when redirected request has cookies" do + before do + request_stub.to_return( + status: 302, + headers: { "Location" => "http://example.com/redirected" }, + ) + + stub_request(:get, "http://example.com/redirected") + .with { |req| webmock_requests << req } + .to_return(body: "redirected") + end + + let(:verb) { :get } + let(:request_options) { { cookies: { sid: 1 }, follow: true } } + + it "sends original request cookies to the next request" do + request.perform + expect(webmock_requests.last.headers).to include("Cookie" => "sid=1") + end + end + + context "when redirect response expires cookies" do + before do + request_stub.to_return( + status: 302, + headers: { + "Location" => "http://example.com/redirected", + "Set-Cookie" => "sid=; Path=/", + }, + ) + + stub_request(:get, "http://example.com/redirected") + .with { |req| webmock_requests << req } + .to_return(body: "redirected") + end + + let(:verb) { :get } + let(:request_options) { { cookies: { sid: 1 }, follow: true } } + + it "removes expired cookies from the next request" do + request.perform + expect(webmock_requests.last.headers).not_to include("Cookie") + end + end + end +end + +RSpec.describe EzClient::HttprbCompatibility do + context "when basic auth expects keyword arguments" do + let(:client_class) do + Class.new do + attr_reader :credentials + + def basic_auth(user:, pass:) + @credentials = { user: user, pass: pass } + self + end + end + end + + let(:client) { client_class.new } + + it "passes credentials as keyword arguments" do + expect(described_class.basic_auth(client, { user: "user", pass: "password" })).to eq(client) + expect(client.credentials).to eq(user: "user", pass: "password") + end + end + + context "when redirector expects keyword arguments" do + let(:redirector_class) do + Class.new do + attr_reader :options + + def initialize(max_hops:) + @options = { max_hops: max_hops } + end + end + end + + before do + stub_const("HTTP::Redirector", redirector_class) + end + + it "passes options as keyword arguments" do + expect(described_class.redirector(max_hops: 3).options).to eq(max_hops: 3) + end + end + + context "when response expects keyword arguments" do + let(:response_class) do + Class.new do + attr_reader :attributes + + def initialize(status:, headers:) + @attributes = { status: status, headers: headers } + end + end + end + + before do + stub_const("HTTP::Response", response_class) + end + + it "passes attributes as keyword arguments" do + expect(described_class.response(status: 200, headers: {}).attributes) + .to eq(status: 200, headers: {}) + end + end + + context "when response expects positional hash" do + let(:response_class) do + Class.new do + attr_reader :attributes + + def initialize(attributes) + @attributes = attributes + end + end + end + + before do + stub_const("HTTP::Response", response_class) + end + + it "passes attributes as a positional hash" do + expect(described_class.response(status: 200, headers: {}).attributes) + .to eq(status: 200, headers: {}) + end + end + + context "when legacy hash initializer is installed" do + let(:response_class) do + Class.new do + attr_reader :attributes + + def initialize(status:, headers:) + @attributes = { status: status, headers: headers } + end + end + end + + before do + described_class.install_legacy_hash_initializer!(response_class) + described_class.install_legacy_hash_initializer!(response_class) + end + + it "allows keyword-only initializers to accept a legacy positional hash" do + expect(response_class.new({ status: 200, headers: {} }).attributes) + .to eq(status: 200, headers: {}) + expect(response_class.new(status: 201, headers: { "X-Test" => "1" }).attributes) + .to eq(status: 201, headers: { "X-Test" => "1" }) + end + end + + context "when legacy header accessors are installed" do + let(:request_class) do + Class.new do + attr_reader :headers + + def initialize + @headers = {} + end + end + end + + let(:request) { request_class.new } + + before do + described_class.install_legacy_header_accessors!(request_class) + end + + it "adds hash-like header accessors" do + request["Authorization"] = "token" + + expect(request["Authorization"]).to eq("token") + end end end @@ -660,7 +849,11 @@ def build(verb, url) # Exercises the httprb v6-specific code path in http_client by stubbing # build_request as unsupported. context "when HTTP::Client#build_request is unsupported" do - before { stub_const("EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST", false) } + before do + allow(EzClient::HttprbCompatibility) + .to receive(:client_supports_build_request?) + .and_return(false) + end it "creates HTTP::Client with persistent connection options" do mock_client = double("HTTP::Client") diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e513847..ab6b607 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -28,7 +28,7 @@ # #readpartial to raise EOFError at end-of-stream (per the v6 IO#readpartial contract), # but WebMock's Streamer returns nil, causing TypeError: no implicit conversion of nil # into String. -unless EzClient::HTTP_CLIENT_SUPPORTS_BUILD_REQUEST +if EzClient::HttprbCompatibility.response_body_requires_eof_error? module HTTP class Response class << self @@ -37,7 +37,7 @@ def from_webmock(request, webmock_response, _request_signature = nil) headers = webmock_response.headers || {} body = build_http_rb_response_body_from_webmock_response(webmock_response) - new( + EzClient::HttprbCompatibility.response( status: status, version: "1.1", headers: headers, From e96c66ecbc37eb41bd748fff7d5275cf3c77cdc9 Mon Sep 17 00:00:00 2001 From: "vadim.kar" Date: Thu, 28 May 2026 11:51:25 +0300 Subject: [PATCH 7/9] Add HTTP 6 CI matrix --- .github/workflows/ci.yml | 26 +++++ gemfiles/http_6.gemfile | 7 ++ gemfiles/http_6.gemfile.lock | 187 +++++++++++++++++++++++++++++++++++ 3 files changed, 220 insertions(+) create mode 100644 gemfiles/http_6.gemfile create mode 100644 gemfiles/http_6.gemfile.lock diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6e9e661..b173508 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,3 +29,29 @@ jobs: - uses: coverallsapp/github-action@v2 with: github-token: ${{ secrets.GITHUB_TOKEN }} + + test-http-6: + runs-on: ubuntu-latest + + # We want to run on external PRs, but not on our own internal PRs as they'll be run on push event + if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name != 'umbrellio/ezclient' + + strategy: + fail-fast: false + matrix: + ruby: ["3.2", "3.3"] + + name: http 6 / Ruby ${{ matrix.ruby }} + + env: + BUNDLE_GEMFILE: gemfiles/http_6.gemfile + + steps: + - uses: actions/checkout@v4 + + - uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby }} + bundler-cache: true + + - run: bundle exec rspec diff --git a/gemfiles/http_6.gemfile b/gemfiles/http_6.gemfile new file mode 100644 index 0000000..b9d97cf --- /dev/null +++ b/gemfiles/http_6.gemfile @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +source "https://rubygems.org" + +eval_gemfile "../Gemfile" + +gem "http", "~> 6.0" diff --git a/gemfiles/http_6.gemfile.lock b/gemfiles/http_6.gemfile.lock new file mode 100644 index 0000000..be4f5e9 --- /dev/null +++ b/gemfiles/http_6.gemfile.lock @@ -0,0 +1,187 @@ +PATH + remote: .. + specs: + ezclient (1.7.2) + http (>= 4) + +GEM + remote: https://rubygems.org/ + specs: + activesupport (8.1.3) + base64 + bigdecimal + concurrent-ruby (~> 1.0, >= 1.3.1) + connection_pool (>= 2.2.5) + drb + i18n (>= 1.6, < 2) + json + logger (>= 1.4.2) + minitest (>= 5.1) + securerandom (>= 0.3) + tzinfo (~> 2.0, >= 2.0.5) + uri (>= 0.13.1) + addressable (2.9.0) + public_suffix (>= 2.0.2, < 8.0) + ast (2.4.3) + base64 (0.3.0) + bigdecimal (4.1.2) + coderay (1.1.3) + concurrent-ruby (1.3.6) + connection_pool (3.0.2) + coveralls (0.7.2) + multi_json (~> 1.3) + rest-client (= 1.6.7) + simplecov (>= 0.7) + term-ansicolor (= 1.2.2) + thor (= 0.18.1) + crack (1.0.1) + bigdecimal + rexml + diff-lcs (1.6.2) + docile (1.4.1) + domain_name (0.6.20240107) + drb (2.2.3) + hashdiff (1.2.1) + http (6.0.3) + http-cookie (~> 1.0) + llhttp (~> 0.6.1) + http-cookie (1.1.6) + domain_name (~> 0.5) + i18n (1.14.8) + concurrent-ruby (~> 1.0) + io-console (0.8.2) + json (2.19.4) + language_server-protocol (3.17.0.5) + lint_roller (1.1.0) + llhttp (0.6.1) + logger (1.7.0) + method_source (1.1.0) + mime-types (3.7.0) + logger + mime-types-data (~> 3.2025, >= 3.2025.0507) + mime-types-data (3.2026.0414) + minitest (6.0.5) + drb (~> 2.0) + prism (~> 1.5) + multi_json (1.20.1) + parallel (1.28.0) + parser (3.3.11.1) + ast (~> 2.4.1) + racc + prism (1.9.0) + pry (0.16.0) + coderay (~> 1.1) + method_source (~> 1.0) + reline (>= 0.6.0) + public_suffix (7.0.5) + racc (1.8.1) + rack (3.2.6) + rainbow (3.1.1) + rake (13.4.2) + regexp_parser (2.12.0) + reline (0.6.3) + io-console (~> 0.5) + rest-client (1.6.7) + mime-types (>= 1.16) + rexml (3.4.4) + rspec (3.13.2) + rspec-core (~> 3.13.0) + rspec-expectations (~> 3.13.0) + rspec-mocks (~> 3.13.0) + rspec-core (3.13.6) + rspec-support (~> 3.13.0) + rspec-expectations (3.13.5) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.13.0) + rspec-mocks (3.13.8) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.13.0) + rspec-support (3.13.7) + rubocop (1.84.2) + json (~> 2.3) + language_server-protocol (~> 3.17.0.2) + lint_roller (~> 1.1.0) + parallel (~> 1.10) + parser (>= 3.3.0.2) + rainbow (>= 2.2.2, < 4.0) + regexp_parser (>= 2.9.3, < 3.0) + rubocop-ast (>= 1.49.0, < 2.0) + ruby-progressbar (~> 1.7) + unicode-display_width (>= 2.4.0, < 4.0) + rubocop-ast (1.49.1) + parser (>= 3.3.7.2) + prism (~> 1.7) + rubocop-config-umbrellio (1.84.118) + rubocop (~> 1.84.0) + rubocop-factory_bot (~> 2.28.0) + rubocop-performance (~> 1.26.0) + rubocop-rails (~> 2.34.0) + rubocop-rake (~> 0.7.0) + rubocop-rspec (~> 3.9.0) + rubocop-sequel (~> 0.4.0) + rubocop-factory_bot (2.28.0) + lint_roller (~> 1.1) + rubocop (~> 1.72, >= 1.72.1) + rubocop-performance (1.26.1) + lint_roller (~> 1.1) + rubocop (>= 1.75.0, < 2.0) + rubocop-ast (>= 1.47.1, < 2.0) + rubocop-rails (2.34.3) + activesupport (>= 4.2.0) + lint_roller (~> 1.1) + rack (>= 1.1) + rubocop (>= 1.75.0, < 2.0) + rubocop-ast (>= 1.44.0, < 2.0) + rubocop-rake (0.7.1) + lint_roller (~> 1.1) + rubocop (>= 1.72.1) + rubocop-rspec (3.9.0) + lint_roller (~> 1.1) + rubocop (~> 1.81) + rubocop-sequel (0.4.1) + lint_roller (~> 1.1) + rubocop (>= 1.72.1, < 2) + ruby-progressbar (1.13.0) + securerandom (0.4.1) + simplecov (0.22.0) + docile (~> 1.1) + simplecov-html (~> 0.11) + simplecov_json_formatter (~> 0.1) + simplecov-html (0.13.2) + simplecov-lcov (0.9.0) + simplecov_json_formatter (0.1.4) + term-ansicolor (1.2.2) + tins (~> 0.8) + thor (0.18.1) + tins (0.13.2) + tzinfo (2.0.6) + concurrent-ruby (~> 1.0) + unicode-display_width (3.2.0) + unicode-emoji (~> 4.1) + unicode-emoji (4.2.0) + uri (1.1.1) + webmock (3.26.2) + addressable (>= 2.8.0) + crack (>= 0.3.2) + hashdiff (>= 0.4.0, < 2.0.0) + +PLATFORMS + arm64-darwin-24 + ruby + +DEPENDENCIES + bundler + coveralls + ezclient! + http (~> 6.0) + pry + rake + rspec + rubocop-config-umbrellio + rubocop-rake + simplecov + simplecov-lcov + webmock + +BUNDLED WITH + 2.7.2 From a72db762d5df92263923ce0c1a22f521fd15c3a8 Mon Sep 17 00:00:00 2001 From: "vadim.kar" Date: Thu, 28 May 2026 12:11:31 +0300 Subject: [PATCH 8/9] code review --- lib/ezclient.rb | 2 - lib/ezclient/httprb_compatibility.rb | 45 +----------- lib/ezclient/request.rb | 68 +++++++++--------- spec/ezclient_spec.rb | 102 ++++++++++++++------------- spec/spec_helper.rb | 6 +- 5 files changed, 92 insertions(+), 131 deletions(-) diff --git a/lib/ezclient.rb b/lib/ezclient.rb index fb57bf5..4cc6ada 100644 --- a/lib/ezclient.rb +++ b/lib/ezclient.rb @@ -13,8 +13,6 @@ require_relative "ezclient/check_options" module EzClient - HTTP_CLIENT_SUPPORTS_BUILD_REQUEST = HttprbCompatibility.client_supports_build_request? - def self.new(*args) Client.new(*args) end diff --git a/lib/ezclient/httprb_compatibility.rb b/lib/ezclient/httprb_compatibility.rb index 6759bee..86ea72f 100644 --- a/lib/ezclient/httprb_compatibility.rb +++ b/lib/ezclient/httprb_compatibility.rb @@ -5,19 +5,11 @@ module EzClient::HttprbCompatibility module_function - def install! - install_legacy_hash_initializer!(HTTP::Response) - install_legacy_hash_initializer!(HTTP::Request) - install_legacy_hash_initializer!(HTTP::Redirector) - install_legacy_header_accessors!(HTTP::Response) - install_legacy_header_accessors!(HTTP::Request) - end - def client_supports_build_request? HTTP::Client.method_defined?(:build_request) end - def response_body_requires_eof_error? + def httprb_v6_or_later? Gem::Version.new(HTTP::VERSION) >= Gem::Version.new("6") end @@ -64,39 +56,4 @@ def response(**attrs) def keyword_initializer?(method) method.parameters.any? { |type, _name| KEYWORD_PARAMETER_TYPES.include?(type) } end - - def install_legacy_hash_initializer!(klass) - return unless keyword_initializer?(klass.instance_method(:initialize)) - return if klass < LegacyHashInitializer - - klass.prepend(LegacyHashInitializer) - end - - def install_legacy_header_accessors!(klass) - return if klass.method_defined?(:[]) && klass.method_defined?(:[]=) - - klass.include(LegacyHeaderAccessors) - end - - module LegacyHashInitializer - def initialize(*args, **kwargs) - if kwargs.empty? && args.size == 1 && args.first.respond_to?(:to_hash) - super(**args.first.to_hash) - else - super - end - end - end - - module LegacyHeaderAccessors - def [](key) - headers[key] - end - - def []=(key, value) - headers[key] = value - end - end end - -EzClient::HttprbCompatibility.install! diff --git a/lib/ezclient/request.rb b/lib/ezclient/request.rb index b29920e..90e9d54 100644 --- a/lib/ezclient/request.rb +++ b/lib/ezclient/request.rb @@ -141,19 +141,20 @@ def perform_redirects(response) def perform_redirects_with_cookies(response) cookie_jar = HTTP::CookieJar.new - store_request_cookies(cookie_jar, http_request) - store_response_cookies(cookie_jar, response) + expired_cookie_names = [] + store_response_cookies(cookie_jar, response, expired_cookie_names) - applied_redirects = {}.compare_by_identity - options = follow - options = options.merge( - on_redirect: redirect_callback(cookie_jar, options[:on_redirect], applied_redirects), - ) + redirect_opts = follow.dup + on_redirect = redirect_opts.delete(:on_redirect) + redirect_response = response + + redirector(redirect_opts).perform(http_request, response) do |req| + apply_cookies(cookie_jar, req, expired_cookie_names) + on_redirect&.call(redirect_response, req) - redirector(options).perform(http_request, response) do |req| - apply_cookies(cookie_jar, req) unless applied_redirects.delete(req) client.perform(req, http_options).tap do |res| - store_response_cookies(cookie_jar, res) + store_response_cookies(cookie_jar, res, expired_cookie_names) + redirect_response = res end end end @@ -162,41 +163,40 @@ def redirector(options) EzClient::HttprbCompatibility.redirector(options) end - def redirect_callback(cookie_jar, callback, applied_redirects) - proc do |response, request| - apply_cookies(cookie_jar, request) - applied_redirects[request] = true - callback&.call(response, request) - end - end - - def store_request_cookies(cookie_jar, request) - header = request.headers[HTTP::Headers::COOKIE].to_s - - HTTP::Cookie.cookie_value_to_hash(header).each do |name, value| - cookie_jar.add(HTTP::Cookie.new(name, value, path: request.uri.path, domain: request.host)) - end - end - - def store_response_cookies(cookie_jar, response) - response.cookies.each do |cookie| - if cookie.value == "" - cookie_jar.delete(cookie) - else + def store_response_cookies(cookie_jar, response, expired_cookie_names) + response.headers.get(HTTP::Headers::SET_COOKIE).each do |set_cookie| + HTTP::Cookie.parse(set_cookie, response.request.uri).each do |cookie| + expired_cookie_names << cookie.name if cookie.expired? cookie_jar.add(cookie) end end end - def apply_cookies(cookie_jar, request) - if cookie_jar.empty? + def apply_cookies(cookie_jar, request, expired_cookie_names) + cookies = cookie_header_for(cookie_jar, request, expired_cookie_names) + + if cookies.empty? request.headers.delete(HTTP::Headers::COOKIE) else - cookies = cookie_jar.map { |cookie| "#{cookie.name}=#{cookie.value}" }.join("; ") request.headers.set(HTTP::Headers::COOKIE, cookies) end end + def cookie_header_for(cookie_jar, request, expired_cookie_names) + response_cookies = cookie_jar.cookies(request.uri) + excluded_names = expired_cookie_names | response_cookies.map(&:name) + + cookie_values = request_cookie_values(request, excluded_names) + cookie_values.concat(response_cookies.map(&:cookie_value)).join("; ") + end + + def request_cookie_values(request, excluded_names) + HTTP::Cookie.cookie_value_to_hash(request.headers[HTTP::Headers::COOKIE].to_s) + .except(*excluded_names) + .map { |name, value| HTTP::Cookie.new(name, value, domain: "example.com", path: "/") } + .map(&:cookie_value) + end + def with_retry(&block) retries = 0 diff --git a/spec/ezclient_spec.rb b/spec/ezclient_spec.rb index 17ae622..60e4b5b 100644 --- a/spec/ezclient_spec.rb +++ b/spec/ezclient_spec.rb @@ -666,6 +666,34 @@ def build(verb, url) end end + context "when follow redirect has on_redirect callback" do + let(:verb) { :get } + let(:calls) { [] } + let(:request_options) { { follow: { on_redirect: on_redirect } } } + + let(:on_redirect) do + proc do |response, redirect_request| + calls << [response.code, redirect_request.headers[HTTP::Headers::COOKIE].to_s] + end + end + + before do + request_stub.to_return( + status: 302, + headers: { "Location" => "http://example.com/redirected", "Set-Cookie" => "sid=1" }, + ) + + stub_request(:get, "http://example.com/redirected") + .with { |req| webmock_requests << req } + .to_return(body: "redirected") + end + + it "calls it with the response and redirected request after applying cookies" do + request.perform + expect(calls).to eq([[302, "sid=1"]]) + end + end + context "when redirected request has cookies" do before do request_stub.to_return( @@ -687,7 +715,7 @@ def build(verb, url) end end - context "when redirect response expires cookies" do + context "when redirect response sets an empty cookie" do before do request_stub.to_return( status: 302, @@ -702,6 +730,30 @@ def build(verb, url) .to_return(body: "redirected") end + let(:verb) { :get } + let(:request_options) { { follow: true } } + + it "preserves it as a legitimate cookie value" do + request.perform + expect(webmock_requests.last.headers).to include("Cookie" => "sid=") + end + end + + context "when redirect response expires cookies" do + before do + request_stub.to_return( + status: 302, + headers: { + "Location" => "http://example.com/redirected", + "Set-Cookie" => "sid=; Max-Age=0; Path=/", + }, + ) + + stub_request(:get, "http://example.com/redirected") + .with { |req| webmock_requests << req } + .to_return(body: "redirected") + end + let(:verb) { :get } let(:request_options) { { cookies: { sid: 1 }, follow: true } } @@ -795,54 +847,6 @@ def initialize(attributes) .to eq(status: 200, headers: {}) end end - - context "when legacy hash initializer is installed" do - let(:response_class) do - Class.new do - attr_reader :attributes - - def initialize(status:, headers:) - @attributes = { status: status, headers: headers } - end - end - end - - before do - described_class.install_legacy_hash_initializer!(response_class) - described_class.install_legacy_hash_initializer!(response_class) - end - - it "allows keyword-only initializers to accept a legacy positional hash" do - expect(response_class.new({ status: 200, headers: {} }).attributes) - .to eq(status: 200, headers: {}) - expect(response_class.new(status: 201, headers: { "X-Test" => "1" }).attributes) - .to eq(status: 201, headers: { "X-Test" => "1" }) - end - end - - context "when legacy header accessors are installed" do - let(:request_class) do - Class.new do - attr_reader :headers - - def initialize - @headers = {} - end - end - end - - let(:request) { request_class.new } - - before do - described_class.install_legacy_header_accessors!(request_class) - end - - it "adds hash-like header accessors" do - request["Authorization"] = "token" - - expect(request["Authorization"]).to eq("token") - end - end end RSpec.describe EzClient::PersistentClient do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ab6b607..4b85fb5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -19,7 +19,9 @@ require "webmock/rspec" require "ezclient" -# WebMock (up to at least 3.24.0) has two incompatibilities with httprb v6: +# WebMock (up to at least 3.24.0) has two incompatibilities with httprb v6. +# Remove after upgrading to a WebMock version that includes: +# https://github.com/bblimke/webmock/pull/1123 # # 1. HTTP::Response.new changed from accepting a positional Hash to keyword arguments. # WebMock calls `new({status: ..., version: ..., ...})` which raises ArgumentError in v6. @@ -28,7 +30,7 @@ # #readpartial to raise EOFError at end-of-stream (per the v6 IO#readpartial contract), # but WebMock's Streamer returns nil, causing TypeError: no implicit conversion of nil # into String. -if EzClient::HttprbCompatibility.response_body_requires_eof_error? +if EzClient::HttprbCompatibility.httprb_v6_or_later? module HTTP class Response class << self From 756f6c58a1a515141604d3740d56fc0b77db28d6 Mon Sep 17 00:00:00 2001 From: "vadim.kar" Date: Fri, 29 May 2026 12:15:57 +0300 Subject: [PATCH 9/9] code review --- lib/ezclient/request.rb | 88 ++++++++++++++++++++++------------------- spec/ezclient_spec.rb | 32 +++++++++++++++ 2 files changed, 80 insertions(+), 40 deletions(-) diff --git a/lib/ezclient/request.rb b/lib/ezclient/request.rb index 90e9d54..16f463a 100644 --- a/lib/ezclient/request.rb +++ b/lib/ezclient/request.rb @@ -10,6 +10,51 @@ class EzClient::Request query ].freeze + class RedirectCookieState + def initialize(response) + self.cookie_jar = HTTP::CookieJar.new + self.expired_cookie_names = [] + store(response) + end + + def store(response) + response.headers.get(HTTP::Headers::SET_COOKIE).each do |set_cookie| + HTTP::Cookie.parse(set_cookie, response.request.uri).each do |cookie| + expired_cookie_names << cookie.name if cookie.expired? + cookie_jar.add(cookie) + end + end + end + + def apply_to(request) + cookies = cookie_header_for(request) + + if cookies.empty? + request.headers.delete(HTTP::Headers::COOKIE) + else + request.headers.set(HTTP::Headers::COOKIE, cookies) + end + end + + private + + attr_accessor :cookie_jar, :expired_cookie_names + + def cookie_header_for(request) + response_cookies = cookie_jar.cookies(request.uri) + excluded_names = expired_cookie_names | response_cookies.map(&:name) + + cookie_values = request_cookie_values(request, excluded_names) + cookie_values.concat(response_cookies.map(&:cookie_value)).join("; ") + end + + def request_cookie_values(request, excluded_names) + HTTP::Cookie.cookie_value_to_hash(request.headers[HTTP::Headers::COOKIE].to_s) + .except(*excluded_names) + .map { |name, value| "#{name}=#{HTTP::Cookie::Scanner.quote(value)}" } + end + end + attr_accessor :verb, :url, :options, :elapsed_seconds def initialize(verb, url, options) @@ -140,20 +185,17 @@ def perform_redirects(response) end def perform_redirects_with_cookies(response) - cookie_jar = HTTP::CookieJar.new - expired_cookie_names = [] - store_response_cookies(cookie_jar, response, expired_cookie_names) - + cookie_state = RedirectCookieState.new(response) redirect_opts = follow.dup on_redirect = redirect_opts.delete(:on_redirect) redirect_response = response redirector(redirect_opts).perform(http_request, response) do |req| - apply_cookies(cookie_jar, req, expired_cookie_names) + cookie_state.apply_to(req) on_redirect&.call(redirect_response, req) client.perform(req, http_options).tap do |res| - store_response_cookies(cookie_jar, res, expired_cookie_names) + cookie_state.store(res) redirect_response = res end end @@ -163,40 +205,6 @@ def redirector(options) EzClient::HttprbCompatibility.redirector(options) end - def store_response_cookies(cookie_jar, response, expired_cookie_names) - response.headers.get(HTTP::Headers::SET_COOKIE).each do |set_cookie| - HTTP::Cookie.parse(set_cookie, response.request.uri).each do |cookie| - expired_cookie_names << cookie.name if cookie.expired? - cookie_jar.add(cookie) - end - end - end - - def apply_cookies(cookie_jar, request, expired_cookie_names) - cookies = cookie_header_for(cookie_jar, request, expired_cookie_names) - - if cookies.empty? - request.headers.delete(HTTP::Headers::COOKIE) - else - request.headers.set(HTTP::Headers::COOKIE, cookies) - end - end - - def cookie_header_for(cookie_jar, request, expired_cookie_names) - response_cookies = cookie_jar.cookies(request.uri) - excluded_names = expired_cookie_names | response_cookies.map(&:name) - - cookie_values = request_cookie_values(request, excluded_names) - cookie_values.concat(response_cookies.map(&:cookie_value)).join("; ") - end - - def request_cookie_values(request, excluded_names) - HTTP::Cookie.cookie_value_to_hash(request.headers[HTTP::Headers::COOKIE].to_s) - .except(*excluded_names) - .map { |name, value| HTTP::Cookie.new(name, value, domain: "example.com", path: "/") } - .map(&:cookie_value) - end - def with_retry(&block) retries = 0 diff --git a/spec/ezclient_spec.rb b/spec/ezclient_spec.rb index 60e4b5b..17f3c20 100644 --- a/spec/ezclient_spec.rb +++ b/spec/ezclient_spec.rb @@ -765,6 +765,38 @@ def build(verb, url) end end +RSpec.describe EzClient::Request::RedirectCookieState do + let(:cookies) { { sid: "a;b" } } + + let(:ezclient_request) do + EzClient.new.request(:get, "http://example.com", cookies: cookies) + end + + let(:http_request) { ezclient_request.send(:http_request) } + let(:redirect_request) { http_request.redirect("http://example.com/redirected") } + let(:response) { Struct.new(:headers, :request).new(HTTP::Headers.coerce({}), http_request) } + + it "preserves request cookie values that require quoting" do + described_class.new(response).apply_to(redirect_request) + + expect(redirect_request.headers[HTTP::Headers::COOKIE].to_s).to eq('sid="a;b"') + end + + context "when request has a full Cookie header string" do + let(:cookies) { {} } + + before do + http_request.headers[HTTP::Headers::COOKIE] = 'sid="a;b"; path=/' + end + + it "round-trips every parsed cookie pair to the redirect request" do + described_class.new(response).apply_to(redirect_request) + + expect(redirect_request.headers[HTTP::Headers::COOKIE].to_s).to eq('sid="a;b"; path=/') + end + end +end + RSpec.describe EzClient::HttprbCompatibility do context "when basic auth expects keyword arguments" do let(:client_class) do