Fix persistent sessions raising StateError on cross-origin redirects

HTTP.persistent now returns an HTTP::Session that pools one HTTP::Client
per origin instead of a single Client pinned to one host. When following
redirects across domains, the session transparently looks up (or
creates) a pooled client for the redirect target's origin.

Key changes:
* Session maintains a @Clients hash (origin => Client) for connection
  pooling when persistent? is true
* Session#close shuts down all pooled clients
* Session#perform_redirects selects the appropriate pooled client for
  each redirect hop via #redirect_client
* Chainable#persistent returns Session (was Client), using branch
  instead of make_client
* Cookie management is preserved across all cross-origin hops
* Non-persistent sessions are unaffected (fresh client per request)

Closes #557.

Author: sferik

CHANGELOG.md

@@ -38,9 +38,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   `read_nonblock` and `:wait_readable` from `write_nonblock` on SSL sockets
   during TLS renegotiation. Previously these symbols were returned as data
   instead of being waited on. ([#358])
+- Persistent sessions now follow cross-origin redirects instead of raising
+  `StateError`. `HTTP.persistent` returns an `HTTP::Session` that pools one
+  `HTTP::Client` per origin, so redirects to a different domain transparently
+  open (and reuse) a separate persistent connection. Cookie management is
+  preserved across all hops. ([#557])
 
 ### Changed
 
+- **BREAKING** `HTTP.persistent` now returns an `HTTP::Session` instead of an
+  `HTTP::Client`. The session pools persistent clients per origin and exposes
+  the same chainable API (`get`, `post`, `headers`, `follow`, etc.) plus a
+  `close` method that shuts down all pooled connections. Code that called
+  `HTTP::Client`-only methods on the return value will need updating. ([#557])
 - **BREAKING** Convert options hash parameters to explicit keyword arguments
   across the public API. Methods like `HTTP.get(url, body: "data")` continue to
   work, but passing an explicit hash (e.g., `HTTP.get(url, {body: "data"})`) is
@@ -223,6 +233,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#536]: https://github.com/httprb/http/issues/536
 [#537]: https://github.com/httprb/http/issues/537
 [#544]: https://github.com/httprb/http/issues/544
+[#557]: https://github.com/httprb/http/issues/557
 [#560]: https://github.com/httprb/http/pull/560
 [#565]: https://github.com/httprb/http/issues/565
 [#566]: https://github.com/httprb/http/issues/566

README.md

@@ -165,15 +165,25 @@ threads.each(&:join)
 Chainable configuration methods (`.headers`, `.timeout`, `.auth`, etc.) return
 an `HTTP::Session`, which creates a fresh `HTTP::Client` for every request.
 
-Persistent connections (`HTTP.persistent`) return an `HTTP::Client`, which is
-**not** thread-safe. For thread-safe persistent connections, use the
+Persistent connections (`HTTP.persistent`) return an `HTTP::Session` that pools
+one `HTTP::Client` per origin. The session itself is **not** thread-safe. For
+thread-safe persistent connections, use the
 [connection_pool](https://rubygems.org/gems/connection_pool) gem:
 
 ```ruby
 pool = ConnectionPool.new(size: 5) { HTTP.persistent("https://example.com") }
 pool.with { |http| http.get("/path") }
 ```
 
+Cross-origin redirects are handled transparently — the session opens a separate
+persistent connection for each origin encountered during a redirect chain:
+
+```ruby
+HTTP.persistent("https://example.com").follow do |http|
+  http.get("/moved-to-other-domain")  # follows redirect across origins
+end
+```
+
 ## Supported Ruby Versions
 
 This library aims to support and is [tested against][build-link]

lib/http/chainable.rb

@@ -90,6 +90,10 @@ def base_uri(uri)
 
     # Open a persistent connection to a host
     #
+    # Returns an {HTTP::Session} that pools persistent {HTTP::Client}
+    # instances by origin. This allows connection reuse within the same
+    # origin and transparent cross-origin redirect handling.
+    #
     # When no host is given, the origin is derived from the configured base URI.
     #
     # @example
@@ -104,10 +108,10 @@ def base_uri(uri)
     #   @option [Integer] timeout Keep alive timeout
     #   @raise  [ArgumentError] if host is nil and no base URI is set
     #   @raise  [Request::Error] if Host is invalid
-    #   @return [HTTP::Client] Persistent client
+    #   @return [HTTP::Session] Persistent session
     # @overload persistent(host = nil, timeout: 5, &block)
-    #   Executes given block with persistent client and automatically closes
-    #   connection at the end of execution.
+    #   Executes given block with persistent session and automatically closes
+    #   all connections at the end of execution.
     #
     #   @example
     #
@@ -127,21 +131,21 @@ def base_uri(uri)
     #       end
     #
     #
-    #   @yieldparam [HTTP::Client] client Persistent client
+    #   @yieldparam [HTTP::Session] session Persistent session
     #   @return [Object] result of last expression in the block
-    # @return [HTTP::Client, Object]
+    # @return [HTTP::Session, Object]
     # @api public
     def persistent(host = nil, timeout: 5)
       host ||= default_options.base_uri&.origin
       raise ArgumentError, "host is required for persistent connections" unless host
 
       options = default_options.merge(keep_alive_timeout: timeout).with_persistent(host)
-      p_client = make_client(options)
-      return p_client unless block_given?
+      session = branch(options)
+      return session unless block_given?
 
-      yield p_client
+      yield session
     ensure
-      p_client&.close
+      session&.close if block_given?
     end
 
     # Make a request through an HTTP proxy

lib/http/session.rb

@@ -15,13 +15,23 @@ module HTTP
   # They hold an immutable {Options} object and create a new {Client}
   # for each request, making them safe to share across threads.
   #
+  # When configured for persistent connections (via {Chainable#persistent}),
+  # the session maintains a pool of {Client} instances keyed by origin,
+  # enabling connection reuse within the same origin and transparent
+  # cross-origin redirect handling.
+  #
   # @example Reuse a configured session across threads
   #   session = HTTP.headers("Accept" => "application/json").timeout(10)
   #   threads = 5.times.map do
   #     Thread.new { session.get("https://example.com") }
   #   end
   #   threads.each(&:join)
   #
+  # @example Persistent session with cross-origin redirects
+  #   HTTP.persistent("https://example.com").follow do |http|
+  #     http.get("/redirect-to-other-domain")  # follows cross-origin redirect
+  #   end
+  #
   # @see Chainable
   # @see Client
   class Session
@@ -51,11 +61,32 @@ class Session
     # @api public
     def initialize(default_options = nil, **)
       @default_options = HTTP::Options.new(default_options, **)
+      @clients = {}
+    end
+
+    # Close all persistent connections held by this session
+    #
+    # When the session is persistent, this closes every pooled {Client}
+    # and clears the pool. Safe to call on non-persistent sessions (no-op).
+    #
+    # @example
+    #   session = HTTP.persistent("https://example.com")
+    #   session.get("/")
+    #   session.close
+    #
+    # @return [void]
+    # @api public
+    def close
+      @clients.each_value(&:close)
+      @clients.clear
     end
 
-    # Make an HTTP request by creating a new {Client}
+    # Make an HTTP request
+    #
+    # For non-persistent sessions a fresh {Client} is created for each
+    # request, ensuring thread safety. For persistent sessions the pooled
+    # {Client} for the request's origin is reused.
     #
-    # A fresh {Client} is created for each request, ensuring thread safety.
     # Manages cookies across redirect hops when following redirects.
     #
     # @example Without a block
@@ -85,21 +116,23 @@ def request(verb, uri,
           timeout_class: timeout_class, timeout_options: timeout_options,
           keep_alive_timeout: keep_alive_timeout, base_uri: base_uri, persistent: persistent }.compact
       )
-      client = make_client(default_options)
+      client = persistent? ? nil : make_client(default_options)
       res    = perform_request(client, verb, uri, merged)
 
       return res unless block
 
       yield res
     ensure
-      client&.close if block
+      if block
+        persistent? ? close : client&.close
+      end
     end
 
     private
 
     # Execute a request with cookie management
     #
-    # @param client [HTTP::Client] the client to perform the request
+    # @param client [HTTP::Client, nil] the client (nil when persistent; looked up from pool)
     # @param verb [Symbol] the HTTP method
     # @param uri [#to_s] the URI to request
     # @param merged [HTTP::Options] the merged options
@@ -109,6 +142,7 @@ def perform_request(client, verb, uri, merged)
       cookie_jar = CookieJar.new
       builder = Request::Builder.new(merged)
       req = builder.build(verb, uri)
+      client ||= client_for_origin(req.uri.origin)
       load_cookies(cookie_jar, req)
       res = client.perform(req, merged)
       store_cookies(cookie_jar, res)
@@ -120,8 +154,13 @@ def perform_request(client, verb, uri, merged)
 
     # Follow redirects with cookie management
     #
+    # For persistent sessions, each redirect hop may target a different
+    # origin. The session looks up (or creates) a pooled {Client} for
+    # the redirect target's origin, allowing cross-origin redirects
+    # without raising {StateError}.
+    #
     # @param jar [HTTP::CookieJar] the cookie jar
-    # @param client [HTTP::Client] the client to perform requests
+    # @param client [HTTP::Client] the client for the initial request
     # @param req [HTTP::Request] the original request
     # @param res [HTTP::Response] the initial redirect response
     # @param opts [HTTP::Options] the merged options
@@ -134,12 +173,47 @@ def perform_redirects(jar, client, req, res, opts)
         wrapped = builder.wrap(redirect_req)
         apply_cookies(jar, wrapped)
         apply_cookies(jar, redirect_req)
-        response = client.perform(wrapped, opts)
+        response = redirect_client(client, wrapped).perform(wrapped, opts)
         store_cookies(jar, response)
         response
       end
     end
 
+    # Return the appropriate client for a redirect hop
+    #
+    # @param client [HTTP::Client] the client for the original request
+    # @param request [HTTP::Request] the redirect request
+    # @return [HTTP::Client] the client for the redirect target
+    # @api private
+    def redirect_client(client, request)
+      persistent? ? client_for_origin(request.uri.origin) : client
+    end
+
+    # Return a pooled persistent {Client} for the given origin
+    #
+    # Creates a new {Client} if one does not already exist for this origin.
+    # For the session's primary persistent origin, the default options are
+    # used directly. For other origins (e.g. redirect targets), the
+    # persistent origin is overridden and base_uri is cleared.
+    #
+    # @param origin [String] the URI origin (scheme + host + port)
+    # @return [HTTP::Client] a persistent client for the origin
+    # @api private
+    def client_for_origin(origin)
+      @clients[origin] ||= make_client(options_for_origin(origin))
+    end
+
+    # Build {Options} for a persistent client targeting the given origin
+    #
+    # @param origin [String] the URI origin
+    # @return [HTTP::Options] options configured for this origin
+    # @api private
+    def options_for_origin(origin)
+      return default_options if origin == default_options.persistent
+
+      default_options.merge(persistent: origin, base_uri: nil)
+    end
+
     # Load cookies from the request's Cookie header into the jar
     #
     # @param jar [HTTP::CookieJar] the cookie jar

sig/http.rbs

@@ -97,8 +97,8 @@ module HTTP
     ) { (Response) -> T } -> T
     def timeout: (Numeric | Hash[Symbol, Numeric] | :null options) -> Session
     def base_uri: (String | URI uri) -> Session
-    def persistent: (?String? host, ?timeout: Integer) -> Client
-                   | (?String? host, ?timeout: Integer) { (Client) -> void } -> void
+    def persistent: (?String? host, ?timeout: Integer) -> Session
+                   | [T] (?String? host, ?timeout: Integer) { (Session) -> T } -> T
     def via: (*(String | Integer | Hash[String, String]) proxy) -> Session
     alias through via
     def follow: (?strict: bool, ?max_hops: Integer, ?on_redirect: (^(Response, Request) -> void)?) -> Session
@@ -180,8 +180,10 @@ module HTTP
     include Chainable
 
     @default_options: Options
+    @clients: Hash[String, Client]
 
     def initialize: (?Options? default_options, **untyped) -> void
+    def close: () -> void
     def request: (
       verb verb,
       String | URI uri,
@@ -236,8 +238,11 @@ module HTTP
 
     private
 
-    def perform_request: (Client client, verb verb, untyped uri, Options merged) -> Response
+    def perform_request: (Client? client, verb verb, untyped uri, Options merged) -> Response
     def perform_redirects: (CookieJar jar, Client client, Request req, Response res, Options opts) -> Response
+    def redirect_client: (Client client, Request request) -> Client
+    def client_for_origin: (String origin) -> Client
+    def options_for_origin: (String origin) -> Options
     def load_cookies: (CookieJar jar, Request request) -> void
     def store_cookies: (CookieJar jar, Response response) -> void
     def apply_cookies: (CookieJar jar, Request request) -> void