From c8dd89ed3dde52933d981d7d7c4c200160f47ec3 Mon Sep 17 00:00:00 2001 From: John Kelly Date: Thu, 29 Jul 2021 15:47:23 -0400 Subject: [PATCH] net/http: add Cookie.Valid method The (*http.Cookie).String method used by SetCookie will silently discard or sanitize any fields it deems invalid, making it difficult to tell whether a cookie will be sent as expected. This change introduces a new (*http.Cookie).Valid method which may be used to check if any cookie fields will be discarded or sanitized prior to calling (*http.Cookie).String. Fixes #46370 Change-Id: I2db80078de190d267a9c675a9717c8be8acc8704 Reviewed-on: https://go-review.googlesource.com/c/go/+/338590 Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Trust: Cherry Mui Trust: Damien Neil Reviewed-by: Damien Neil --- api/next.txt | 1 + src/net/http/cookie.go | 33 +++++++++++++++++++++++++++++++++ src/net/http/cookie_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/api/next.txt b/api/next.txt index b1b9c1d7b1..1192fc9069 100644 --- a/api/next.txt +++ b/api/next.txt @@ -142,3 +142,4 @@ pkg testing, type FuzzResult struct, T time.Duration pkg testing, type InternalFuzzTarget struct pkg testing, type InternalFuzzTarget struct, Fn func(*F) pkg testing, type InternalFuzzTarget struct, Name string +pkg net/http, method (*Cookie) Valid() error diff --git a/src/net/http/cookie.go b/src/net/http/cookie.go index 02b40315de..cb37f2351f 100644 --- a/src/net/http/cookie.go +++ b/src/net/http/cookie.go @@ -5,6 +5,8 @@ package http import ( + "errors" + "fmt" "log" "net" "net/http/internal/ascii" @@ -236,6 +238,37 @@ func (c *Cookie) String() string { return b.String() } +// Valid reports whether the cookie is valid. +func (c *Cookie) Valid() error { + if c == nil { + return errors.New("http: nil Cookie") + } + if !isCookieNameValid(c.Name) { + return errors.New("http: invalid Cookie.Name") + } + if !validCookieExpires(c.Expires) { + return errors.New("http: invalid Cookie.Expires") + } + for i := 0; i < len(c.Value); i++ { + if !validCookieValueByte(c.Value[i]) { + return fmt.Errorf("http: invalid byte %q in Cookie.Value", c.Value[i]) + } + } + if len(c.Path) > 0 { + for i := 0; i < len(c.Path); i++ { + if !validCookiePathByte(c.Path[i]) { + return fmt.Errorf("http: invalid byte %q in Cookie.Path", c.Path[i]) + } + } + } + if len(c.Domain) > 0 { + if !validCookieDomain(c.Domain) { + return errors.New("http: invalid Cookie.Domain") + } + } + return nil +} + // readCookies parses all "Cookie" values from the header h and // returns the successfully parsed Cookies. // diff --git a/src/net/http/cookie_test.go b/src/net/http/cookie_test.go index 959713a0dc..257dc57420 100644 --- a/src/net/http/cookie_test.go +++ b/src/net/http/cookie_test.go @@ -529,6 +529,31 @@ func TestCookieSanitizePath(t *testing.T) { } } +func TestCookieValid(t *testing.T) { + tests := []struct { + cookie *Cookie + valid bool + }{ + {nil, false}, + {&Cookie{Name: ""}, false}, + {&Cookie{Name: "invalid-expires"}, false}, + {&Cookie{Name: "invalid-value", Value: "foo\"bar"}, false}, + {&Cookie{Name: "invalid-path", Path: "/foo;bar/"}, false}, + {&Cookie{Name: "invalid-domain", Domain: "example.com:80"}, false}, + {&Cookie{Name: "valid", Value: "foo", Path: "/bar", Domain: "example.com", Expires: time.Unix(0, 0)}, true}, + } + + for _, tt := range tests { + err := tt.cookie.Valid() + if err != nil && tt.valid { + t.Errorf("%#v.Valid() returned error %v; want nil", tt.cookie, err) + } + if err == nil && !tt.valid { + t.Errorf("%#v.Valid() returned nil; want error", tt.cookie) + } + } +} + func BenchmarkCookieString(b *testing.B) { const wantCookieString = `cookie-9=i3e01nf61b6t23bvfmplnanol3; Path=/restricted/; Domain=example.com; Expires=Tue, 10 Nov 2009 23:00:00 GMT; Max-Age=3600` c := &Cookie{