From 17f153eae61225957ed67816ab13f6aeac5b7d91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Tue, 16 Mar 2021 15:25:48 +0100 Subject: [PATCH] Remove implicit `path=/` from `HTTP::Cookie` (#10491) --- spec/std/http/client/response_spec.cr | 2 +- spec/std/http/cookie_spec.cr | 63 ++++++++++++++------------- spec/std/http/server/response_spec.cr | 4 +- src/http/cookie.cr | 6 +-- 4 files changed, 39 insertions(+), 36 deletions(-) diff --git a/spec/std/http/client/response_spec.cr b/spec/std/http/client/response_spec.cr index 29b839f21c2e..311259f6aa9e 100644 --- a/spec/std/http/client/response_spec.cr +++ b/spec/std/http/client/response_spec.cr @@ -223,7 +223,7 @@ class HTTP::Client::Response io.clear response.to_io(io) - io.to_s.should eq("HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\nContent-Length: 5\r\nSet-Cookie: foo=baz; path=/\r\nSet-Cookie: quux=baz; path=/\r\n\r\nhello") + io.to_s.should eq("HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\nContent-Length: 5\r\nSet-Cookie: foo=baz\r\nSet-Cookie: quux=baz\r\n\r\nhello") end it "sets content length from body" do diff --git a/spec/std/http/cookie_spec.cr b/spec/std/http/cookie_spec.cr index 9ac457ed6ecd..46fb5508c540 100644 --- a/spec/std/http/cookie_spec.cr +++ b/spec/std/http/cookie_spec.cr @@ -98,21 +98,24 @@ module HTTP end describe "#to_set_cookie_header" do - it { HTTP::Cookie.new("x", "v$1").to_set_cookie_header.should eq "x=v$1; path=/" } + it { HTTP::Cookie.new("x", "v$1").to_set_cookie_header.should eq "x=v$1" } - it { HTTP::Cookie.new("x", "seven", domain: "127.0.0.1").to_set_cookie_header.should eq "x=seven; domain=127.0.0.1; path=/" } + it { HTTP::Cookie.new("x", "seven", domain: "127.0.0.1").to_set_cookie_header.should eq "x=seven; domain=127.0.0.1" } - it { HTTP::Cookie.new("x", "expiring", expires: Time.unix(1257894000)).to_set_cookie_header.should eq "x=expiring; path=/; expires=Tue, 10 Nov 2009 23:00:00 GMT" } - it { HTTP::Cookie.new("x", "expiring-1601", expires: Time.utc(1601, 1, 1, 1, 1, 1, nanosecond: 1)).to_set_cookie_header.should eq "x=expiring-1601; path=/; expires=Mon, 01 Jan 1601 01:01:01 GMT" } + it { HTTP::Cookie.new("x", "y", path: "/").to_set_cookie_header.should eq "x=y; path=/" } + it { HTTP::Cookie.new("x", "y", path: "/example").to_set_cookie_header.should eq "x=y; path=/example" } + + it { HTTP::Cookie.new("x", "expiring", expires: Time.unix(1257894000)).to_set_cookie_header.should eq "x=expiring; expires=Tue, 10 Nov 2009 23:00:00 GMT" } + it { HTTP::Cookie.new("x", "expiring-1601", expires: Time.utc(1601, 1, 1, 1, 1, 1, nanosecond: 1)).to_set_cookie_header.should eq "x=expiring-1601; expires=Mon, 01 Jan 1601 01:01:01 GMT" } it "samesite" do - HTTP::Cookie.new("x", "samesite-default", samesite: nil).to_set_cookie_header.should eq "x=samesite-default; path=/" - HTTP::Cookie.new("x", "samesite-lax", samesite: :lax).to_set_cookie_header.should eq "x=samesite-lax; path=/; SameSite=Lax" - HTTP::Cookie.new("x", "samesite-strict", samesite: :strict).to_set_cookie_header.should eq "x=samesite-strict; path=/; SameSite=Strict" - HTTP::Cookie.new("x", "samesite-none", samesite: :none).to_set_cookie_header.should eq "x=samesite-none; path=/; SameSite=None" + HTTP::Cookie.new("x", "samesite-default", samesite: nil).to_set_cookie_header.should eq "x=samesite-default" + HTTP::Cookie.new("x", "samesite-lax", samesite: :lax).to_set_cookie_header.should eq "x=samesite-lax; SameSite=Lax" + HTTP::Cookie.new("x", "samesite-strict", samesite: :strict).to_set_cookie_header.should eq "x=samesite-strict; SameSite=Strict" + HTTP::Cookie.new("x", "samesite-none", samesite: :none).to_set_cookie_header.should eq "x=samesite-none; SameSite=None" end - it { HTTP::Cookie.new("empty-value", "").to_set_cookie_header.should eq "empty-value=; path=/" } + it { HTTP::Cookie.new("empty-value", "").to_set_cookie_header.should eq "empty-value=" } end end @@ -122,7 +125,7 @@ module HTTP cookie = parse_first_cookie("key=value") cookie.name.should eq("key") cookie.value.should eq("value") - cookie.to_set_cookie_header.should eq("key=value; path=/") + cookie.to_set_cookie_header.should eq("key=value") end it "parse_set_cookie with space" do @@ -135,28 +138,28 @@ module HTTP cookie = parse_first_cookie("key=") cookie.name.should eq("key") cookie.value.should eq("") - cookie.to_set_cookie_header.should eq("key=; path=/") + cookie.to_set_cookie_header.should eq("key=") end it "parses key=key=value" do cookie = parse_first_cookie("key=key=value") cookie.name.should eq("key") cookie.value.should eq("key=value") - cookie.to_set_cookie_header.should eq("key=key=value; path=/") + cookie.to_set_cookie_header.should eq("key=key=value") end it "parses key=key%3Dvalue" do cookie = parse_first_cookie("key=key%3Dvalue") cookie.name.should eq("key") cookie.value.should eq("key%3Dvalue") - cookie.to_set_cookie_header.should eq("key=key%3Dvalue; path=/") + cookie.to_set_cookie_header.should eq("key=key%3Dvalue") end it "parses special character in name" do cookie = parse_first_cookie("key%3Dvalue=value") cookie.name.should eq("key%3Dvalue") cookie.value.should eq("value") - cookie.to_set_cookie_header.should eq("key%3Dvalue=value; path=/") + cookie.to_set_cookie_header.should eq("key%3Dvalue=value") end it "parses multiple cookies" do @@ -184,7 +187,7 @@ module HTTP cookie.name.should eq("key") cookie.value.should eq("value") cookie.secure.should be_true - cookie.to_set_cookie_header.should eq("key=value; path=/; Secure") + cookie.to_set_cookie_header.should eq("key=value; Secure") end it "parses HttpOnly" do @@ -192,7 +195,7 @@ module HTTP cookie.name.should eq("key") cookie.value.should eq("value") cookie.http_only.should be_true - cookie.to_set_cookie_header.should eq("key=value; path=/; HttpOnly") + cookie.to_set_cookie_header.should eq("key=value; HttpOnly") end describe "SameSite" do @@ -202,7 +205,7 @@ module HTTP cookie.name.should eq "key" cookie.value.should eq "value" cookie.samesite.should eq HTTP::Cookie::SameSite::Lax - cookie.to_set_cookie_header.should eq "key=value; path=/; SameSite=Lax" + cookie.to_set_cookie_header.should eq "key=value; SameSite=Lax" end end @@ -212,7 +215,7 @@ module HTTP cookie.name.should eq "key" cookie.value.should eq "value" cookie.samesite.should eq HTTP::Cookie::SameSite::Strict - cookie.to_set_cookie_header.should eq "key=value; path=/; SameSite=Strict" + cookie.to_set_cookie_header.should eq "key=value; SameSite=Strict" end end @@ -222,7 +225,7 @@ module HTTP cookie.name.should eq "key" cookie.value.should eq "value" cookie.samesite.should be_nil - cookie.to_set_cookie_header.should eq "key=value; path=/" + cookie.to_set_cookie_header.should eq "key=value" end end end @@ -232,7 +235,7 @@ module HTTP cookie.name.should eq("key") cookie.value.should eq("value") cookie.domain.should eq("www.example.com") - cookie.to_set_cookie_header.should eq("key=value; domain=www.example.com; path=/") + cookie.to_set_cookie_header.should eq("key=value; domain=www.example.com") end it "parses expires iis" do @@ -291,7 +294,7 @@ module HTTP end it "parse domain as IP" do - parse_set_cookie("a=1; domain=127.0.0.1; path=/; HttpOnly").domain.should eq "127.0.0.1" + parse_set_cookie("a=1; domain=127.0.0.1; HttpOnly").domain.should eq "127.0.0.1" end it "parse max-age as seconds from current time" do @@ -450,20 +453,20 @@ module HTTP describe "adding response headers" do it "overwrites all pre-existing Set-Cookie headers" do headers = Headers.new - headers.add("Set-Cookie", "a=b; path=/") - headers.add("Set-Cookie", "c=d; path=/") + headers.add("Set-Cookie", "a=b") + headers.add("Set-Cookie", "c=d") cookies = Cookies.new cookies << Cookie.new("x", "y") headers.get("Set-Cookie").size.should eq 2 - headers.get("Set-Cookie").includes?("a=b; path=/").should be_true - headers.get("Set-Cookie").includes?("c=d; path=/").should be_true + headers.get("Set-Cookie").includes?("a=b").should be_true + headers.get("Set-Cookie").includes?("c=d").should be_true cookies.add_response_headers(headers) headers.get("Set-Cookie").size.should eq 1 - headers.get("Set-Cookie")[0].should eq "x=y; path=/" + headers.get("Set-Cookie")[0].should eq "x=y" end it "sets one Set-Cookie header per cookie" do @@ -476,8 +479,8 @@ module HTTP cookies.add_response_headers(headers) headers.get?("Set-Cookie").should_not be_nil - headers.get("Set-Cookie").includes?("a=b; path=/").should be_true - headers.get("Set-Cookie").includes?("c=d; path=/").should be_true + headers.get("Set-Cookie").includes?("a=b").should be_true + headers.get("Set-Cookie").includes?("c=d").should be_true end it "uses encode_www_form on Set-Cookie value" do @@ -485,13 +488,13 @@ module HTTP cookies = Cookies.new cookies << Cookie.new("a", "b+c") cookies.add_response_headers(headers) - headers.get("Set-Cookie").includes?("a=b+c; path=/").should be_true + headers.get("Set-Cookie").includes?("a=b+c").should be_true end describe "when no cookies are set" do it "does not set a Set-Cookie header" do headers = Headers.new - headers.add("Set-Cookie", "a=b; path=/") + headers.add("Set-Cookie", "a=b") cookies = Cookies.new headers.get?("Set-Cookie").should_not be_nil diff --git a/spec/std/http/server/response_spec.cr b/spec/std/http/server/response_spec.cr index 15353f6bea3b..b9388bc01eaf 100644 --- a/spec/std/http/server/response_spec.cr +++ b/spec/std/http/server/response_spec.cr @@ -217,14 +217,14 @@ describe HTTP::Server::Response do response = Response.new(io) response.cookies["Bar"] = "Foo" response.close - io.to_s.should eq("HTTP/1.1 200 OK\r\nContent-Length: 0\r\nSet-Cookie: Bar=Foo; path=/\r\n\r\n") + io.to_s.should eq("HTTP/1.1 200 OK\r\nContent-Length: 0\r\nSet-Cookie: Bar=Foo\r\n\r\n") io = IO::Memory.new response = Response.new(io) response.cookies["Bar"] = "Foo" response.print("Hello") response.close - io.to_s.should eq("HTTP/1.1 200 OK\r\nContent-Length: 5\r\nSet-Cookie: Bar=Foo; path=/\r\n\r\nHello") + io.to_s.should eq("HTTP/1.1 200 OK\r\nContent-Length: 5\r\nSet-Cookie: Bar=Foo\r\n\r\nHello") end it "closes when it fails to write" do diff --git a/src/http/cookie.cr b/src/http/cookie.cr index d4f6de7c365e..c86a16bdf7e7 100644 --- a/src/http/cookie.cr +++ b/src/http/cookie.cr @@ -17,7 +17,7 @@ module HTTP getter name : String getter value : String - property path : String + property path : String? property expires : Time? property domain : String? property secure : Bool @@ -30,7 +30,7 @@ module HTTP # Creates a new `Cookie` instance. # # Raises `IO::Error` if *name* or *value* are invalid as per [RFC 6265 ยง4.1.1](https://tools.ietf.org/html/rfc6265#section-4.1.1). - def initialize(name : String, value : String, @path : String = "/", + def initialize(name : String, value : String, @path : String? = nil, @expires : Time? = nil, @domain : String? = nil, @secure : Bool = false, @http_only : Bool = false, @samesite : SameSite? = nil, @extension : String? = nil) @@ -174,7 +174,7 @@ module HTTP Cookie.new( match["name"], match["value"], - path: match["path"]? || "/", + path: match["path"]?, expires: expires, domain: match["domain"]?, secure: match["secure"]? != nil,