git.iko.soy/typeable/oauth2-proxy
on master help sign in
Count complete cookie content in byte splitting
48a2aaaNick Meves
pkg/sessions/cookie/session_store.go+17−9
compare with master
unifiedsplit
@@ -15,6 +15,13 @@ import (
15 "github.com/oauth2-proxy/oauth2-proxy/pkg/logger"
15 "github.com/oauth2-proxy/oauth2-proxy/pkg/logger"
16)
16)
17
17
18+const (
19+ // Cookies are limited to 4kb for all parts
20+ // including the cookie name, value, attributes; IE (http.cookie).String()
21+ // Most browsers' max is 4096 -- but we give ourselves some leeway
22+ maxCookieLength = 4000
23+)
24+
18// Ensure CookieSessionStore implements the interface
25// Ensure CookieSessionStore implements the interface
19var _ sessions.SessionStore = &SessionStore{}
26var _ sessions.SessionStore = &SessionStore{}
20
27
@@ -102,9 +109,7 @@ func (s *SessionStore) makeSessionCookie(req *http.Request, value string, now ti
102 }
109 }
103 c := s.makeCookie(req, s.CookieOptions.Name, value, s.CookieOptions.Expire, now)
110 c := s.makeCookie(req, s.CookieOptions.Name, value, s.CookieOptions.Expire, now)
104
111
105− // Cookies are limited to 4kb including the length of the cookie name,
112+ if len(c.String()) > maxCookieLength {
106− // the cookie name can be up to 256 bytes
107− if len(c.Value) > 4096-len(s.CookieOptions.Name) {
108 return splitCookie(c)
113 return splitCookie(c)
109 }
114 }
110 return []*http.Cookie{c}
115 return []*http.Cookie{c}
@@ -139,7 +144,7 @@ func NewCookieSessionStore(opts *options.SessionOptions, cookieOpts *options.Coo
139// it into a slice of cookies which fit within the 4kb cookie limit indexing
144// it into a slice of cookies which fit within the 4kb cookie limit indexing
140// the cookies from 0
145// the cookies from 0
141func splitCookie(c *http.Cookie) []*http.Cookie {
146func splitCookie(c *http.Cookie) []*http.Cookie {
142− if len(c.Value) < 4096-len(c.Name) {
147+ if len(c.String()) < maxCookieLength {
143 return []*http.Cookie{c}
148 return []*http.Cookie{c}
144 }
149 }
145
150
@@ -153,13 +158,16 @@ func splitCookie(c *http.Cookie) []*http.Cookie {
153 newCookie.Name = splitCookieName(c.Name, count)
158 newCookie.Name = splitCookieName(c.Name, count)
154 count++
159 count++
155
160
156− maxCookieLength := 4096 - len(newCookie.Name)
161+ newCookie.Value = string(valueBytes)
162+ cookieLength := len(newCookie.String())
157− if len(valueBytes) < maxCookieLength {
163+ if cookieLength <= maxCookieLength {
158− newCookie.Value = string(valueBytes)
159 valueBytes = []byte{}
164 valueBytes = []byte{}
160 } else {
165 } else {
166+ overflow := cookieLength - maxCookieLength
167+ valueSize := len(valueBytes) - overflow
168+
161− newValue := valueBytes[:maxCookieLength]
169+ newValue := valueBytes[:valueSize]
162− valueBytes = valueBytes[maxCookieLength:]
170+ valueBytes = valueBytes[valueSize:]
163 newCookie.Value = string(newValue)
171 newCookie.Value = string(newValue)
164 }
172 }
165 cookies = append(cookies, newCookie)
173 cookies = append(cookies, newCookie)
pkg/sessions/cookie/session_store_test.go+36−40
@@ -54,59 +54,55 @@ func Test_copyCookie(t *testing.T) {
54}
54}
55
55
56func Test_splitCookie(t *testing.T) {
56func Test_splitCookie(t *testing.T) {
57− testCases := map[string]struct {
57+ testCases := map[string]*http.Cookie{
58− Cookie *http.Cookie
59− ExpectedSizes []int
60− }{
61 "Short cookie name": {
58 "Short cookie name": {
62− Cookie: &http.Cookie{
63− Name: "short",
59+ Name: "short",
64− Value: strings.Repeat("v", 10000),
60+ Value: strings.Repeat("v", 10000),
65− },
66− ExpectedSizes: []int{4089, 4089, 1822},
67 },
61 },
68 "Long cookie name": {
62 "Long cookie name": {
69− Cookie: &http.Cookie{
70− Name: strings.Repeat("n", 251),
63+ Name: strings.Repeat("n", 251),
71− Value: strings.Repeat("a", 10000),
64+ Value: strings.Repeat("a", 10000),
72− },
73− ExpectedSizes: []int{3843, 3843, 2314},
74 },
65 },
75 "Max cookie name": {
66 "Max cookie name": {
76− Cookie: &http.Cookie{
77− Name: strings.Repeat("n", 256),
67+ Name: strings.Repeat("n", 256),
78− Value: strings.Repeat("a", 10000),
68+ Value: strings.Repeat("a", 10000),
79− },
80− ExpectedSizes: []int{3840, 3840, 2320},
81 },
69 },
82 "Suffix overflow cookie name": {
70 "Suffix overflow cookie name": {
83− Cookie: &http.Cookie{
84− Name: strings.Repeat("n", 255),
71+ Name: strings.Repeat("n", 255),
85− Value: strings.Repeat("a", 10000),
72+ Value: strings.Repeat("a", 10000),
86− },
73+ },
87− ExpectedSizes: []int{3840, 3840, 2320},
74+ "Double digit suffix cookie name overflow": {
75+ Name: strings.Repeat("n", 253),
76+ Value: strings.Repeat("a", 50000),
88 },
77 },
89− "Double digit suffix cookie name results in different sizes": {
78+ "With short name and attributes": {
90− Cookie: &http.Cookie{
79+ Name: "short",
91− Name: strings.Repeat("n", 253),
92− Value: strings.Repeat("a", 50000),
80+ Value: strings.Repeat("v", 10000),
93− },
81+ Path: "/path",
94− ExpectedSizes: []int{3841, 3841, 3841, 3841, 3841, 3841, 3841, 3841, 3841, 3841,
82+ Domain: "x.y.z",
95− 3840, 3840, 3840, 70},
83+ Secure: true,
84+ HttpOnly: true,
85+ SameSite: http.SameSiteLaxMode,
96 },
86 },
97− "Double digit suffix overflow cookie name results in same sizes": {
87+ "With max length name and attributes": {
98− Cookie: &http.Cookie{
99− Name: strings.Repeat("n", 254),
88+ Name: strings.Repeat("n", 256),
100− Value: strings.Repeat("a", 50000),
89+ Value: strings.Repeat("v", 10000),
101− },
90+ Path: "/path",
102− ExpectedSizes: []int{3840, 3840, 3840, 3840, 3840, 3840, 3840, 3840, 3840, 3840,
91+ Domain: "x.y.z",
103− 3840, 3840, 3840, 80},
92+ Secure: true,
93+ HttpOnly: true,
94+ SameSite: http.SameSiteLaxMode,
104 },
95 },
105 }
96 }
106 for testName, tc := range testCases {
97 for testName, tc := range testCases {
107 t.Run(testName, func(t *testing.T) {
98 t.Run(testName, func(t *testing.T) {
99+ splitCookies := splitCookie(tc)
108− for i, cookie := range splitCookie(tc.Cookie) {
100+ for i, cookie := range splitCookies {
109− assert.Equal(t, tc.ExpectedSizes[i], len(cookie.Value))
101+ if i < len(splitCookies)-1 {
102+ assert.Equal(t, 4000, len(cookie.String()))
103+ } else {
104+ assert.GreaterOrEqual(t, 4000, len(cookie.String()))
105+ }
110 }
106 }
111 })
107 })
112 }
108 }