diff --git a/lib/http/redirector.rb b/lib/http/redirector.rb index 5d2de095..ea890e96 100644 --- a/lib/http/redirector.rb +++ b/lib/http/redirector.rb @@ -49,6 +49,8 @@ def perform(request, response) @request = request @response = response @visited = [] + collect_cookies_from_request + collect_cookies_from_response while REDIRECT_CODES.include? @response.status.code @visited << "#{@request.verb} #{@request.uri}" @@ -59,8 +61,12 @@ def perform(request, response) @response.flush # XXX(ixti): using `Array#inject` to return `nil` if no Location header. - @request = redirect_to(@response.headers.get(Headers::LOCATION).inject(:+)) + @request = redirect_to(@response.headers.get(Headers::LOCATION).inject(:+)) + unless cookie_jar.empty? + @request.headers.set(Headers::COOKIE, cookie_jar.cookies.map { |c| "#{c.name}=#{c.value}" }.join("; ")) + end @response = yield @request + collect_cookies_from_response end @response @@ -68,6 +74,48 @@ def perform(request, response) private + # All known cookies. On the original request, this is only the original cookies, but after that, + # Set-Cookie headers can add, set or delete cookies. + def cookie_jar + # it seems that @response.cookies instance is reused between responses, so we have to "clone" + @cookie_jar ||= HTTP::CookieJar.new + end + + def collect_cookies_from_request + request_cookie_header = @request.headers["Cookie"] + cookies = + if request_cookie_header + HTTP::Cookie.cookie_value_to_hash(request_cookie_header) + else + {} + end + + cookies.each do |key, value| + cookie_jar.add(HTTP::Cookie.new(key, value, :path => @request.uri.path, :domain => @request.host)) + end + end + + # Carry cookies from one response to the next. Carrying cookies to the next response ends up + # carrying them to the next request as well. + # + # Note that this isn't part of the IETF standard, but all major browsers support setting cookies + # on redirect: https://blog.dubbelboer.com/2012/11/25/302-cookie.html + 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 # @return [Boolean] def too_many_hops? diff --git a/spec/lib/http/redirector_spec.rb b/spec/lib/http/redirector_spec.rb index 110f4ec1..8a3abd2d 100644 --- a/spec/lib/http/redirector_spec.rb +++ b/spec/lib/http/redirector_spec.rb @@ -11,8 +11,12 @@ def simple_response(status, body = "", headers = {}) ) end - def redirect_response(status, location) - simple_response status, "", "Location" => location + def redirect_response(status, location, set_cookie = {}) + res = simple_response status, "", "Location" => location + set_cookie.each do |name, value| + res.headers.add("Set-Cookie", "#{name}=#{value}; path=/; httponly; secure; SameSite=none; Secure") + end + res end describe "#strict" do @@ -89,6 +93,61 @@ def redirect_response(status, location) expect(res.to_s).to eq "http://example.com/123" end + it "returns cookies in response" do + req = HTTP::Request.new :verb => :head, :uri => "http://example.com" + hops = [ + redirect_response(301, "http://example.com/1", {"foo" => "42"}), + redirect_response(301, "http://example.com/2", {"bar" => "53", "deleted" => "foo"}), + redirect_response(301, "http://example.com/3", {"baz" => "64", "deleted" => ""}), + redirect_response(301, "http://example.com/4", {"baz" => "65"}), + simple_response(200, "bar") + ] + + request_cookies = [ + {"foo" => "42"}, + {"foo" => "42", "bar" => "53", "deleted" => "foo"}, + {"foo" => "42", "bar" => "53", "baz" => "64"}, + {"foo" => "42", "bar" => "53", "baz" => "65"} + ] + + res = redirector.perform(req, hops.shift) do |request| + req_cookie = HTTP::Cookie.cookie_value_to_hash(request.headers["Cookie"] || "") + expect(req_cookie).to eq request_cookies.shift + hops.shift + end + expect(res.to_s).to eq "bar" + cookies = res.cookies.cookies.map { |c| [c.name, c.value] }.to_h + expect(cookies["foo"]).to eq "42" + expect(cookies["bar"]).to eq "53" + expect(cookies["baz"]).to eq "65" + expect(cookies["deleted"]).to eq nil + end + + it "returns original cookies in response" do + req = HTTP::Request.new :verb => :head, :uri => "http://example.com" + req.headers.set("Cookie", "foo=42; deleted=baz") + hops = [ + redirect_response(301, "http://example.com/1", {"bar" => "64", "deleted" => ""}), + simple_response(200, "bar") + ] + + request_cookies = [ + {"foo" => "42", "bar" => "64"}, + {"foo" => "42", "bar" => "64"} + ] + + res = redirector.perform(req, hops.shift) do |request| + req_cookie = HTTP::Cookie.cookie_value_to_hash(request.headers["Cookie"] || "") + expect(req_cookie).to eq request_cookies.shift + hops.shift + end + expect(res.to_s).to eq "bar" + cookies = res.cookies.cookies.map { |c| [c.name, c.value] }.to_h + expect(cookies["foo"]).to eq "42" + expect(cookies["bar"]).to eq "64" + expect(cookies["deleted"]).to eq nil + end + context "following 300 redirect" do context "with strict mode" do let(:options) { {:strict => true} } @@ -400,7 +459,7 @@ def redirect_response(status, location) describe "changing verbs during redirects" do let(:options) { {:strict => false} } let(:post_body) { HTTP::Request::Body.new("i might be way longer in real life") } - let(:cookie) { "dont eat my cookies" } + let(:cookie) { "dont=eat my cookies" } def a_dangerous_request(verb) HTTP::Request.new(