exp/cookiejar: store cookies under TLD+1 on nil public suffix list

The current implementation would store all cookies received from
any .com domain under "com" in the entries map if a nil public
suffix list is used in constructing the Jar. This is inefficient.

This CL uses the TLD+1 of the domain if the public suffix list
is nil which has two advantages:
 - It uses the entries map efficiently.
 - It prevents a host foo.com to set cookies for bar.com.
   (It may set the cookie, but it won't be returned to bar.com.)
A domain like www.british-library.uk may still set a domain
cookie for .british-library.uk in this case.

The behavior for a non-nil public suffix list is unchanged, cookies
are stored under eTLD+1 in this case.

R=nigeltao
CC=golang-dev
https://golang.org/cl/7312105

diff --git a/src/pkg/exp/cookiejar/jar.go b/src/pkg/exp/cookiejar/jar.go
index 73036e0d65a7eb776d58b9e1365f93b4a1540430..c41851b2b93cddd58bca1812d25fde918680791a 100644 (file)
--- a/src/pkg/exp/cookiejar/jar.go
+++ b/src/pkg/exp/cookiejar/jar.go
@@ -48,8 +48,8 @@ type Options struct {
        // an HTTP server can set a cookie for a domain.
        //
        // A nil value is valid and may be useful for testing but it is not
-       // secure: it means that the HTTP server for foo.com can set a cookie
-       // for bar.com.
+       // secure: it means that the HTTP server for foo.co.uk can set a cookie
+       // for bar.co.uk.
        PublicSuffixList PublicSuffixList
 }
 
@@ -333,19 +333,24 @@ func jarKey(host string, psl PublicSuffixList) string {
        if isIP(host) {
                return host
        }
+
+       var i int
        if psl == nil {
-               // Key cookies under TLD of host.
-               return host[1+strings.LastIndex(host, "."):]
-       }
-       suffix := psl.PublicSuffix(host)
-       if suffix == host {
-               return host
-       }
-       i := len(host) - len(suffix)
-       if i <= 0 || host[i-1] != '.' {
-               // The provided public suffix list psl is broken.
-               // Storing cookies under host is a safe stopgap.
-               return host
+               i = strings.LastIndex(host, ".")
+               if i == -1 {
+                       return host
+               }
+       } else {
+               suffix := psl.PublicSuffix(host)
+               if suffix == host {
+                       return host
+               }
+               i = len(host) - len(suffix)
+               if i <= 0 || host[i-1] != '.' {
+                       // The provided public suffix list psl is broken.
+                       // Storing cookies under host is a safe stopgap.
+                       return host
+               }
        }
        prevDot := strings.LastIndex(host[:i-1], ".")
        return host[prevDot+1:]

diff --git a/src/pkg/exp/cookiejar/jar_test.go b/src/pkg/exp/cookiejar/jar_test.go
index f17b0d44a5ae07b3def0c83bfcf1371cf80cbbc0..13f8949a39dd62dd4b5f4569b18a7f324ed7adf6 100644 (file)
--- a/src/pkg/exp/cookiejar/jar_test.go
+++ b/src/pkg/exp/cookiejar/jar_test.go
@@ -99,10 +99,25 @@ func TestJarKey(t *testing.T) {
                        t.Errorf("%q: got %q, want %q", host, got, want)
                }
        }
+}
 
-       for _, host := range []string{"www.example.com", "example.com", "com"} {
-               if got := jarKey(host, nil); got != "com" {
-                       t.Errorf(`%q: got %q, want "com"`, host, got)
+var jarKeyNilPSLTests = map[string]string{
+       "foo.www.example.com": "example.com",
+       "www.example.com":     "example.com",
+       "example.com":         "example.com",
+       "com":                 "com",
+       "foo.www.bbc.co.uk":   "co.uk",
+       "www.bbc.co.uk":       "co.uk",
+       "bbc.co.uk":           "co.uk",
+       "co.uk":               "co.uk",
+       "uk":                  "uk",
+       "192.168.0.5":         "192.168.0.5",
+}
+
+func TestJarKeyNilPSL(t *testing.T) {
+       for host, want := range jarKeyNilPSLTests {
+               if got := jarKey(host, nil); got != want {
+                       t.Errorf("%q: got %q, want %q", host, got, want)
                }
        }
 }