🧹 chore: Improve Cookie() validation (#3546)

* validate cookies with net/http

* refactor: sanitize cookie via net/http

* Address review comments

* refactor: set cookie attrs via setters

* Update ctx.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Update ctx.go

* Update ctx.go

* Update ctx.go

* Add more unit-tests

* Fix test

* Add default case for samesite

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

Author: gaby

ctx.go

@@ -397,36 +397,80 @@ func (c *DefaultCtx) RequestCtx() *fasthttp.RequestCtx {
 
 // Cookie sets a cookie by passing a cookie struct.
 func (c *DefaultCtx) Cookie(cookie *Cookie) {
-	fcookie := fasthttp.AcquireCookie()
-	fcookie.SetKey(cookie.Name)
-	fcookie.SetValue(cookie.Value)
-	fcookie.SetPath(cookie.Path)
-	fcookie.SetDomain(cookie.Domain)
-	// only set max age and expiry when SessionOnly is false
-	// i.e. cookie supposed to last beyond browser session
-	// refer: https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#define_the_lifetime_of_a_cookie
-	if !cookie.SessionOnly {
-		fcookie.SetMaxAge(cookie.MaxAge)
-		fcookie.SetExpire(cookie.Expires)
+	if cookie.Path == "" {
+		cookie.Path = "/"
+	}
+
+	if cookie.SessionOnly {
+		cookie.MaxAge = 0
+		cookie.Expires = time.Time{}
 	}
-	fcookie.SetSecure(cookie.Secure)
-	fcookie.SetHTTPOnly(cookie.HTTPOnly)
+
+	var sameSite http.SameSite
 
 	switch utils.ToLower(cookie.SameSite) {
 	case CookieSameSiteStrictMode:
-		fcookie.SetSameSite(fasthttp.CookieSameSiteStrictMode)
+		sameSite = http.SameSiteStrictMode
 	case CookieSameSiteNoneMode:
-		fcookie.SetSameSite(fasthttp.CookieSameSiteNoneMode)
+		sameSite = http.SameSiteNoneMode
 	case CookieSameSiteDisabled:
-		fcookie.SetSameSite(fasthttp.CookieSameSiteDisabled)
+		sameSite = 0
+	case CookieSameSiteLaxMode:
+		sameSite = http.SameSiteLaxMode
 	default:
+		sameSite = http.SameSiteLaxMode
+	}
+
+	// create/validate cookie using net/http
+	hc := &http.Cookie{
+		Name:        cookie.Name,
+		Value:       cookie.Value,
+		Path:        cookie.Path,
+		Domain:      cookie.Domain,
+		Expires:     cookie.Expires,
+		MaxAge:      cookie.MaxAge,
+		Secure:      cookie.Secure,
+		HttpOnly:    cookie.HTTPOnly,
+		SameSite:    sameSite,
+		Partitioned: cookie.Partitioned,
+	}
+
+	if err := hc.Valid(); err != nil {
+		// invalid cookies are ignored, same approach as net/http
+		return
+	}
+
+	// create fasthttp cookie
+	fcookie := fasthttp.AcquireCookie()
+	fcookie.SetKey(hc.Name)
+	fcookie.SetValue(hc.Value)
+	fcookie.SetPath(hc.Path)
+	fcookie.SetDomain(hc.Domain)
+
+	if !cookie.SessionOnly {
+		fcookie.SetMaxAge(hc.MaxAge)
+		fcookie.SetExpire(hc.Expires)
+	}
+
+	fcookie.SetSecure(hc.Secure)
+	fcookie.SetHTTPOnly(hc.HttpOnly)
+
+	switch sameSite {
+	case http.SameSiteLaxMode:
 		fcookie.SetSameSite(fasthttp.CookieSameSiteLaxMode)
+	case http.SameSiteStrictMode:
+		fcookie.SetSameSite(fasthttp.CookieSameSiteStrictMode)
+	case http.SameSiteNoneMode:
+		fcookie.SetSameSite(fasthttp.CookieSameSiteNoneMode)
+	case http.SameSiteDefaultMode:
+		fcookie.SetSameSite(fasthttp.CookieSameSiteDefaultMode)
+	default:
+		fcookie.SetSameSite(fasthttp.CookieSameSiteDisabled)
 	}
 
-	// CHIPS allows to partition cookie jar by top-level site.
-	// refer: https://developers.google.com/privacy-sandbox/3pcd/chips
-	fcookie.SetPartitioned(cookie.Partitioned)
+	fcookie.SetPartitioned(hc.Partitioned)
 
+	// Set resp header
 	c.fasthttp.Response.Header.SetCookie(fcookie)
 	fasthttp.ReleaseCookie(fcookie)
 }

ctx_test.go

@@ -920,6 +920,108 @@ func Test_Ctx_Cookie(t *testing.T) {
 	require.Equal(t, expect, c.Res().Get(HeaderSetCookie))
 }
 
+// go test -run Test_Ctx_Cookie_PartitionedSecure
+func Test_Ctx_Cookie_PartitionedSecure(t *testing.T) {
+	t.Parallel()
+	app := New()
+	c := app.AcquireCtx(&fasthttp.RequestCtx{})
+
+	ck := &Cookie{
+		Name:        "ps",
+		Value:       "v",
+		Secure:      true,
+		SameSite:    CookieSameSiteNoneMode,
+		Partitioned: true,
+	}
+	c.Res().Cookie(ck)
+	require.Equal(t, "ps=v; path=/; secure; SameSite=None; Partitioned", c.Res().Get(HeaderSetCookie))
+}
+
+// go test -run Test_Ctx_Cookie_Invalid
+func Test_Ctx_Cookie_Invalid(t *testing.T) {
+	t.Parallel()
+	app := New()
+
+	cases := []*Cookie{
+		{Name: "", Value: "a"},                                                        // empty name
+		{Name: "foo bar", Value: "a"},                                                 // invalid char in name
+		{Name: "n", Value: "bad\nval"},                                                // invalid value byte
+		{Name: "d", Value: "b", Domain: "in valid"},                                   // invalid domain spaces
+		{Name: "d", Value: "b", Domain: "example..com"},                               // invalid domain dots
+		{Name: "i", Value: "b", Domain: "2001:db8::1"},                                // ipv6 not allowed
+		{Name: "p", Value: "b", Path: "\x00"},                                         // invalid path byte
+		{Name: "e", Value: "b", Expires: time.Date(1500, 1, 1, 0, 0, 0, 0, time.UTC)}, // invalid expires
+		{Name: "s", Value: "b", Partitioned: true},                                    // partitioned but not secure
+	}
+
+	for _, invalid := range cases {
+		c := app.AcquireCtx(&fasthttp.RequestCtx{})
+		c.Res().Cookie(invalid)
+		require.Empty(t, c.Res().Get(HeaderSetCookie))
+		c.Response().Header.Reset()
+		app.ReleaseCtx(c)
+	}
+}
+
+// go test -run Test_Ctx_Cookie_DefaultPath
+func Test_Ctx_Cookie_DefaultPath(t *testing.T) {
+	t.Parallel()
+	app := New()
+	c := app.AcquireCtx(&fasthttp.RequestCtx{})
+
+	ck := &Cookie{
+		Name:  "p",
+		Value: "v",
+		// Path intentionally empty to verify defaulting
+	}
+
+	c.Res().Cookie(ck)
+	require.Equal(t,
+		"p=v; path=/; SameSite=Lax",
+		c.Res().Get(HeaderSetCookie),
+	)
+}
+
+// go test -run Test_Ctx_Cookie_MaxAgeOnly
+func Test_Ctx_Cookie_MaxAgeOnly(t *testing.T) {
+	t.Parallel()
+	app := New()
+	c := app.AcquireCtx(&fasthttp.RequestCtx{})
+
+	ck := &Cookie{
+		Name:   "ttl",
+		Value:  "v",
+		MaxAge: 3600,
+	}
+	c.Res().Cookie(ck)
+
+	require.Equal(t,
+		"ttl=v; max-age=3600; path=/; SameSite=Lax",
+		c.Res().Get(HeaderSetCookie),
+	)
+}
+
+// go test -run Test_Ctx_Cookie_StrictPartitioned
+func Test_Ctx_Cookie_StrictPartitioned(t *testing.T) {
+	t.Parallel()
+	app := New()
+	c := app.AcquireCtx(&fasthttp.RequestCtx{})
+
+	ck := &Cookie{
+		Name:        "sp",
+		Value:       "v",
+		Secure:      true,
+		SameSite:    CookieSameSiteStrictMode,
+		Partitioned: true,
+	}
+	c.Res().Cookie(ck)
+
+	require.Equal(t,
+		"sp=v; path=/; secure; SameSite=Strict; Partitioned",
+		c.Res().Get(HeaderSetCookie),
+	)
+}
+
 // go test -v -run=^$ -bench=Benchmark_Ctx_Cookie -benchmem -count=4
 func Benchmark_Ctx_Cookie(b *testing.B) {
 	app := New()