Refactor cookie API

sferik · sferik · commit 0dd249f3e691 · 2026-03-09T10:46:43.000-07:00
Remove cookie management from Options. Chainable#cookies now sets the Cookie header directly with no implicit merging (last call wins). Response#cookies returns Array<HTTP::Cookie> instead of CookieJar. Closes #536.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -54,6 +54,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   rejects unknown keys, non-numeric values, string keys, and empty hashes (#754)
 - Bumped min llhttp dependency version
 - **BREAKING** Handle responses in the reverse order from the requests (#776)
+- **BREAKING** `Response#cookies` now returns `Array<HTTP::Cookie>` instead of
+  `HTTP::CookieJar`. The `cookies` option has been removed from `Options`;
+  `Chainable#cookies` now sets the `Cookie` header directly with no implicit
+  merging — the last `.cookies()` call wins (#536)
 
 ### Removed
 
diff --git a/lib/http/chainable.rb b/lib/http/chainable.rb
@@ -164,11 +164,20 @@ def headers(headers)
     # @example
     #   HTTP.cookies(session: "abc123").get("http://example.com")
     #
-    # @param [Hash] cookies cookies to set
+    # @param [Hash, Array<HTTP::Cookie>] cookies cookies to set
     # @return [HTTP::Session]
     # @api public
     def cookies(cookies)
-      branch default_options.with_cookies(cookies)
+      value = cookies.map do |entry|
+        case entry
+        when HTTP::Cookie then entry.cookie_value
+        else
+          name, val = entry
+          HTTP::Cookie.new(name.to_s, val.to_s).cookie_value
+        end
+      end.join("; ")
+
+      headers(Headers::COOKIE => value)
     end
 
     # Force a specific encoding for response body
diff --git a/lib/http/client/request_builder.rb b/lib/http/client/request_builder.rb
@@ -39,7 +39,7 @@ def merge_query_params!(uri, opts)
         uri.query_values = uri.query_values(Array).to_a.concat(opts.params.to_a)
       end
 
-      # Creates request headers with cookies (if any) merged in
+      # Creates request headers
       #
       # @return [HTTP::Headers] the constructed headers
       # @api private
@@ -49,13 +49,6 @@ def make_request_headers(opts)
         # Tell the server to keep the conn open
         headers[Headers::CONNECTION] = default_options.persistent? ? Connection::KEEP_ALIVE : Connection::CLOSE
 
-        cookies = opts.cookies.values
-
-        unless cookies.empty?
-          cookies = opts.headers.get(Headers::COOKIE).concat(cookies).join("; ")
-          headers[Headers::COOKIE] = cookies
-        end
-
         headers
       end
 
diff --git a/lib/http/options.rb b/lib/http/options.rb
@@ -213,7 +213,7 @@ def []=(option, val)
     # @return [Hash]
     def default_options_hash
       { response: :auto, encoding: nil, nodelay: false, keep_alive_timeout: 5,
-        proxy: Hash[], ssl: Hash[], headers: Hash[], cookies: Hash[], features: Hash[],
+        proxy: Hash[], ssl: Hash[], headers: Hash[], features: Hash[],
         timeout_class: self.class.default_timeout_class, timeout_options: Hash[],
         socket_class: self.class.default_socket_class,
         ssl_socket_class: self.class.default_ssl_socket_class }
diff --git a/lib/http/options/definitions.rb b/lib/http/options/definitions.rb
@@ -7,13 +7,6 @@ class Options
       headers.merge(new_headers) # steep:ignore
     end
 
-    def_option :cookies do |new_cookies|
-      new_cookies.each_with_object cookies.dup do |(k, v), jar| # steep:ignore
-        cookie = k.is_a?(Cookie) ? k : Cookie.new(k.to_s, v.to_s)
-        jar[cookie.name] = cookie.cookie_value
-      end
-    end
-
     def_option :encoding do |encoding|
       self.encoding = Encoding.find(encoding) # steep:ignore
     end
diff --git a/lib/http/redirector.rb b/lib/http/redirector.rb
@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require "http/cookie_jar"
 require "http/headers"
 
 module HTTP
@@ -125,7 +126,6 @@ def redirect_uri
     # @api private
     # @return [HTTP::CookieJar]
     def cookie_jar
-      # it seems that @response.cookies instance is reused between responses, so we have to "clone"
       @cookie_jar ||= CookieJar.new
     end
 
@@ -147,19 +147,13 @@ def collect_cookies_from_request
     # @api private
     # @return [void]
     def collect_cookies_from_response
-      # Overwrite previous cookies
       @response.cookies.each do |cookie|
         if cookie.value == ""
           cookie_jar.delete(cookie)
         else
           cookie_jar.add(cookie)
         end
       end
-
-      # I wish we could just do @response.cookes = cookie_jar
-      cookie_jar.each do |cookie|
-        @response.cookies.add(cookie)
-      end
     end
 
     # Check if we reached max amount of redirect hops
diff --git a/lib/http/response.rb b/lib/http/response.rb
@@ -8,7 +8,7 @@
 require "http/mime_type"
 require "http/response/status"
 require "http/response/inflater"
-require "http/cookie_jar"
+require "http/cookie"
 require "time"
 
 module HTTP
@@ -255,14 +255,12 @@ def content_type
     # Cookies from Set-Cookie headers
     #
     # @example
-    #   response.cookies # => #<HTTP::CookieJar ...>
+    #   response.cookies # => [#<HTTP::Cookie ...>, ...]
     #
-    # @return [HTTP::CookieJar]
+    # @return [Array<HTTP::Cookie>]
     # @api public
     def cookies
-      @cookies ||= headers.get(Headers::SET_COOKIE).each_with_object CookieJar.new do |v, jar|
-        jar.parse(v, uri)
-      end
+      @cookies ||= headers.get(Headers::SET_COOKIE).flat_map { |v| HTTP::Cookie.parse(v, uri) }
     end
 
     # Check if the response uses chunked transfer encoding
diff --git a/sig/deps.rbs b/sig/deps.rbs
@@ -195,6 +195,7 @@ module HTTP
     attr_reader value: String
     attr_reader cookie_value: String
 
+    def self.parse: (String, untyped) -> Array[HTTP::Cookie]
     def self.cookie_value_to_hash: (String) -> Hash[String, String]
     def initialize: (String, String, **untyped) -> void
   end
diff --git a/sig/http.rbs b/sig/http.rbs
@@ -446,14 +446,12 @@ module HTTP
     attr_accessor keep_alive_timeout: Integer
     attr_accessor timeout_class: Class
     attr_accessor timeout_options: Hash[Symbol, untyped]
-    attr_accessor cookies: Hash[untyped, untyped]
     attr_accessor encoding: Encoding?
     attr_reader follow: Hash[untyped, untyped]?
     attr_reader retriable: Hash[untyped, untyped]?
     attr_reader persistent: String?
 
     def with_headers: (untyped value) -> Options
-    def with_cookies: (untyped value) -> Options
     def with_encoding: (untyped value) -> Options
     def with_features: (untyped value) -> Options
     def with_follow: (untyped value) -> Options
@@ -673,7 +671,7 @@ module HTTP
     extend Forwardable
 
     @content_type: ContentType
-    @cookies: untyped
+    @cookies: Array[HTTP::Cookie]?
 
     attr_reader status: Response::Status
     attr_reader version: String
@@ -698,7 +696,7 @@ module HTTP
     def content_type: () -> ContentType
     def mime_type: () -> String?
     def charset: () -> String?
-    def cookies: () -> untyped
+    def cookies: () -> Array[HTTP::Cookie]
     def chunked?: () -> bool
     def parse: (?untyped? type) -> untyped
     def inspect: () -> String
diff --git a/test/http/options/merge_test.rb b/test/http/options/merge_test.rb
@@ -67,7 +67,6 @@
         nodelay:            false,
         ssl_socket_class:   HTTP::Options.default_ssl_socket_class,
         ssl_context:        nil,
-        cookies:            {},
         encoding:           nil,
         features:           {}
       },
diff --git a/test/http/redirector_test.rb b/test/http/redirector_test.rb
@@ -114,7 +114,7 @@ def cookie_jar_from(redirector)
       assert_equal "http://example.com/123", res.to_s
     end
 
-    it "returns cookies in response" do
+    it "forwards accumulated cookies through redirect chain" do
       req  = HTTP::Request.new verb: :head, uri: "http://example.com"
       hops = [
         redirect_response(301, "http://example.com/1", { "foo" => "42" }),
@@ -139,13 +139,9 @@ def cookie_jar_from(redirector)
       end
 
       assert_equal "bar", res.to_s
-      assert_equal(
-        { "foo" => "42", "bar" => "53", "baz" => "65" },
-        res.cookies.cookies.to_h { |c| [c.name, c.value] }
-      )
     end
 
-    it "returns original cookies in response" do
+    it "forwards original request cookies through redirect chain" do
       req = HTTP::Request.new verb: :head, uri: "http://example.com"
       req.headers.set("Cookie", "foo=42; deleted=baz")
       hops = [
@@ -158,19 +154,12 @@ def cookie_jar_from(redirector)
         { "foo" => "42", "bar" => "64" }
       ]
 
-      res = redirector.perform(req, hops.shift) do |request|
+      redirector.perform(req, hops.shift) do |request|
         req_cookie = HTTP::Cookie.cookie_value_to_hash(request.headers["Cookie"] || "")
 
         assert_equal request_cookies.shift, req_cookie
         hops.shift
       end
-
-      assert_equal "bar", res.to_s
-      cookies = res.cookies.cookies.to_h { |c| [c.name, c.value] }
-
-      assert_equal "42", cookies["foo"]
-      assert_equal "64", cookies["bar"]
-      assert_nil cookies["deleted"]
     end
 
     it "collects request cookies with correct path" do
@@ -609,18 +598,17 @@ def empty_body
       end
     end
 
-    it "deletes cookies with empty value from final response" do
+    it "deletes cookies with empty value during redirect" do
       req = HTTP::Request.new verb: :get, uri: "http://example.com"
       hops = [
         redirect_response(301, "http://example.com/1", { "mycookie" => "present" }),
         redirect_response(301, "http://example.com/2", { "mycookie" => "" }),
         simple_response(200, "done")
       ]
 
-      res = redirector.perform(req, hops.shift) { hops.shift }
-      cookie_names = res.cookies.cookies.map(&:name)
+      redirector.perform(req, hops.shift) { hops.shift }
+      cookie_names = cookie_jar_from(redirector).map(&:name)
 
-      # The cookie with empty value should have been deleted from the jar
       refute_includes cookie_names, "mycookie"
     end
 
diff --git a/test/http/response_test.rb b/test/http/response_test.rb
@@ -255,25 +255,26 @@
   end
 
   describe "#cookies" do
-    let(:jar) { response.cookies }
+    let(:cookie_list) { response.cookies }
 
     let(:cookies) { ["a=1", "b=2; domain=example.com", "c=3; domain=bad.org"] }
     let(:headers) { { "Set-Cookie" => cookies } }
 
-    it "returns an HTTP::CookieJar" do
-      assert_kind_of HTTP::CookieJar, jar
+    it "returns an Array of HTTP::Cookie" do
+      assert_kind_of Array, cookie_list
+      cookie_list.each { |c| assert_kind_of HTTP::Cookie, c }
     end
 
     it "contains cookies without domain restriction" do
-      assert_equal(1, jar.count { |c| "a" == c.name })
+      assert_equal(1, cookie_list.count { |c| "a" == c.name })
     end
 
     it "contains cookies limited to domain of request uri" do
-      assert_equal(1, jar.count { |c| "b" == c.name })
+      assert_equal(1, cookie_list.count { |c| "b" == c.name })
     end
 
     it "does not contain cookies limited to non-requested uri" do
-      assert_equal(0, jar.count { |c| "c" == c.name })
+      assert_equal(0, cookie_list.count { |c| "c" == c.name })
     end
   end
 
diff --git a/test/http/session_test.rb b/test/http/session_test.rb
@@ -122,7 +122,7 @@
 
       assert_equal "text/html", chained.default_options.headers[:accept]
       assert_equal HTTP::Timeout::Global, chained.default_options.timeout_class
-      assert_equal "session_id=abc", chained.default_options.cookies["session_id"]
+      assert_equal "session_id=abc", chained.default_options.headers["Cookie"]
     end
   end
 
diff --git a/test/http_test.rb b/test/http_test.rb
@@ -571,23 +571,16 @@
       assert_equal "abc: def", HTTP.cookies(abc: :def).get(endpoint).to_s
     end
 
-    it "properly works with cookie jars from response" do
+    it "properly works with cookies from response" do
       res = HTTP.get(endpoint).flush
 
       assert_equal "foo: bar", HTTP.cookies(res.cookies).get(endpoint).to_s
     end
 
-    it "properly merges cookies" do
-      res     = HTTP.get(endpoint).flush
-      client  = HTTP.cookies(foo: 123, bar: 321).cookies(res.cookies)
+    it "replaces previously set cookies" do
+      client = HTTP.cookies(foo: 123, bar: 321).cookies(baz: :moo)
 
-      assert_equal "foo: bar\nbar: 321", client.get(endpoint).to_s
-    end
-
-    it "properly merges Cookie headers and cookies" do
-      client = HTTP.headers("Cookie" => "foo=bar").cookies(baz: :moo)
-
-      assert_equal "foo: bar\nbaz: moo", client.get(endpoint).to_s
+      assert_equal "baz: moo", client.get(endpoint).to_s
     end
   end
 
