Skip to content

Commit 4ba114e

Browse files
vdoblerneild
vdobler
authored and
neild
committed
net/http/cookiejar: allow cookies with an IP address in the domain attribute
A set domain attribute in a cookie in a Set-Cookie header is intended to create a domain cookie, i.e. a cookie that is not only sent back to the domain the Set-Cookie was received from, but to all subdomains thereof too. Sometimes people set this domain attribute to an IP address. This seems to be allowed by RFC 6265 albeit it's not really sensible as there are no "subdomains" of an IP address. Contemporary browsers allow such cookies, currently Jar forbids them. This CL allows to persist such cookies in the Jar and send them back again in subsequent requests. Jar allows those cookies that all contemporary browsers allow (not all browsers behave the same and none seems to conform to RFC 6265 in regards to these cookies, see below). The following browsers in current version) were tested: - Chrome (Mac and Windows) - Firefox (Mac and Windows) - Safari (Mac) - Opera (Mac) - Edge (Windows) - Internet Explorer (Windows) - curl (Mac, Linux) All of them allow a cookie to be set via the following HTTP header if the request was made to e.g. http://35.206.97.83/ : Set-Cookie: a=1; domain=35.206.97.83 They differ in handling a leading dot "." before the IP address as in Set-Cookie: a=1; domain=.35.206.97.83 sets a=1 only in curl and in Internet Explorer, the other browsers just reject such cookies. As far as these internals can be observed the browsers do not treat such cookies as domain cookies but as host cookies. RFC 6265 would require to treat them as domain cookies; this is a) nonsensical and b) doesn't make an observable difference. As we do not expose Jar entries and their HostOnly flag it probably is still okay to claim that Jar implements a RFC 6265 cookie jar. RFC 6265 would allow cookies with dot-prefixed domains like domain=.35.206.97.83 but it seems as if this feature of RFC 6265 is not used in real life and not requested by users of package cookiejar (probably because it doesn't work in browsers) so we refrain from documenting this detail. Fixes #12610 Change-Id: Ibd883d85bde6b958b732cbc3618a1238ac4fc84a Reviewed-on: https://go-review.googlesource.com/c/go/+/326689 Run-TryBot: Dmitri Shuralyov <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Michael Knyszek <[email protected]> Reviewed-by: Damien Neil <[email protected]>
1 parent 2c3cb19 commit 4ba114e

File tree

2 files changed

+59
-9
lines changed

2 files changed

+59
-9
lines changed

‎src/net/http/cookiejar/jar.go

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,9 @@ func (e *entry) shouldSend(https bool, host, path string) bool {
121121
return e.domainMatch(host) && e.pathMatch(path) && (https || !e.Secure)
122122
}
123123

124-
// domainMatch implements "domain-match" of RFC 6265 section 5.1.3.
124+
// domainMatch checks whether e's Domain allows sending e back to host.
125+
// It differs from "domain-match" of RFC 6265 section 5.1.3 because we treat
126+
// a cookie with an IP address in the Domain always as a host cookie.
125127
func (e *entry) domainMatch(host string) bool {
126128
if e.Domain == host {
127129
return true
@@ -455,10 +457,36 @@ func (j *Jar) domainAndType(host, domain string) (string, bool, error) {
455457
}
456458

457459
if isIP(host) {
458-
// According to RFC 6265 domain-matching includes not being
459-
// an IP address.
460-
// TODO: This might be relaxed as in common browsers.
461-
return "", false, errNoHostname
460+
// RFC 6265 is not super clear here, a sensible interpretation
461+
// is that cookies with an IP address in the domain-attribute
462+
// are allowed.
463+
464+
// RFC 6265 section 5.2.3 mandates to strip an optional leading
465+
// dot in the domain-attribute before processing the cookie.
466+
//
467+
// Most browsers don't do that for IP addresses, only curl
468+
// version 7.54) and and IE (version 11) do not reject a
469+
// Set-Cookie: a=1; domain=.127.0.0.1
470+
// This leading dot is optional and serves only as hint for
471+
// humans to indicate that a cookie with "domain=.bbc.co.uk"
472+
// would be sent to every subdomain of bbc.co.uk.
473+
// It just doesn't make sense on IP addresses.
474+
// The other processing and validation steps in RFC 6265 just
475+
// collaps to:
476+
if host != domain {
477+
return "", false, errIllegalDomain
478+
}
479+
480+
// According to RFC 6265 such cookies should be treated as
481+
// domain cookies.
482+
// As there are no subdomains of an IP address the treatment
483+
// according to RFC 6265 would be exactly the same as that of
484+
// a host-only cookie. Contemporary browsers (and curl) do
485+
// allows such cookies but treat them as host-only cookies.
486+
// So do we as it just doesn't make sense to label them as
487+
// domain cookies when there is no domain; the whole notion of
488+
// domain cookies requires a domain name to be well defined.
489+
return host, true, nil
462490
}
463491

464492
// From here on: If the cookie is valid, it is a domain cookie (with

‎src/net/http/cookiejar/jar_test.go

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -306,8 +306,8 @@ var domainAndTypeTests = [...]struct {
306306
{"foo.sso.example.com", "sso.example.com", "sso.example.com", false, nil},
307307
{"bar.co.uk", "bar.co.uk", "bar.co.uk", false, nil},
308308
{"foo.bar.co.uk", ".bar.co.uk", "bar.co.uk", false, nil},
309-
{"127.0.0.1", "127.0.0.1", "", false, errNoHostname},
310-
{"2001:4860:0:2001::68", "2001:4860:0:2001::68", "2001:4860:0:2001::68", false, errNoHostname},
309+
{"127.0.0.1", "127.0.0.1", "127.0.0.1", true, nil},
310+
{"2001:4860:0:2001::68", "2001:4860:0:2001::68", "2001:4860:0:2001::68", true, nil},
311311
{"www.example.com", ".", "", false, errMalformedDomain},
312312
{"www.example.com", "..", "", false, errMalformedDomain},
313313
{"www.example.com", "other.com", "", false, errIllegalDomain},
@@ -328,7 +328,7 @@ func TestDomainAndType(t *testing.T) {
328328
for _, tc := range domainAndTypeTests {
329329
domain, hostOnly, err := jar.domainAndType(tc.host, tc.domain)
330330
if err != tc.wantErr {
331-
t.Errorf("%q/%q: got %q error, want %q",
331+
t.Errorf("%q/%q: got %q error, want %v",
332332
tc.host, tc.domain, err, tc.wantErr)
333333
continue
334334
}
@@ -593,6 +593,21 @@ var basicsTests = [...]jarTest{
593593
"a=1",
594594
[]query{{"http://192.168.0.10", "a=1"}},
595595
},
596+
{
597+
"Domain cookies on IP.",
598+
"http://192.168.0.10",
599+
[]string{
600+
"a=1; domain=192.168.0.10", // allowed
601+
"b=2; domain=172.31.9.9", // rejected, can't set cookie for other IP
602+
"c=3; domain=.192.168.0.10", // rejected like in most browsers
603+
},
604+
"a=1",
605+
[]query{
606+
{"http://192.168.0.10", "a=1"},
607+
{"http://172.31.9.9", ""},
608+
{"http://www.fancy.192.168.0.10", ""},
609+
},
610+
},
596611
{
597612
"Port is ignored #1.",
598613
"http://www.host.test/",
@@ -927,10 +942,17 @@ var chromiumBasicsTests = [...]jarTest{
927942
{
928943
"TestIpAddress #3.",
929944
"http://1.2.3.4/foo",
930-
[]string{"a=1; domain=1.2.3.4"},
945+
[]string{"a=1; domain=1.2.3.3"},
931946
"",
932947
[]query{{"http://1.2.3.4/foo", ""}},
933948
},
949+
{
950+
"TestIpAddress #4.",
951+
"http://1.2.3.4/foo",
952+
[]string{"a=1; domain=1.2.3.4"},
953+
"a=1",
954+
[]query{{"http://1.2.3.4/foo", "a=1"}},
955+
},
934956
{
935957
"TestNonDottedAndTLD #2.",
936958
"http://com./index.html",

0 commit comments

Comments
 (0)