Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect handling of Cookie.SetMaxAge with value 0 or less #1900

Closed
anthonydresser opened this issue Nov 13, 2024 · 2 comments
Closed

Incorrect handling of Cookie.SetMaxAge with value 0 or less #1900

anthonydresser opened this issue Nov 13, 2024 · 2 comments

Comments

@anthonydresser
Copy link

anthonydresser commented Nov 13, 2024

Per the spec that was reference during the cookie max-age implementation (#184), https://datatracker.ietf.org/doc/html/rfc6265#section-5.2.2, a max-age value of 0 or less is valid and is interpreted as forcing the cookie to expire immediately.

In the current implementation, setting max-age to 0 or less causes the max age attribute to be completely ignored, making it impossible to delete a cookie via the max-age attribute. https://github.com/valyala/fasthttp/blob/master/cookie.go#L281

Given 0 is the default value of int in go, probably doesn't make sense to support max-age=0, but supporting setting maxAge to <0 should be possible by changing the maxAge > 0 to maxAge != 0.

	if c.maxAge != 0 {
		dst = append(dst, ';', ' ')
		dst = append(dst, strCookieMaxAge...)
		dst = append(dst, '=')
		dst = AppendUint(dst, c.maxAge)
	} else if !c.expire.IsZero() {
		c.bufV = AppendHTTPDate(c.bufV[:0], c.expire)
		dst = append(dst, ';', ' ')
		dst = append(dst, strCookieExpires...)
		dst = append(dst, '=')
		dst = append(dst, c.bufV...)
	}

Alternatively, the maxAge property could be changed to a *int to support setting max-age to 0 as well and check for nil during the creation of the cookie header.

@ksw2000
Copy link
Contributor

ksw2000 commented Nov 14, 2024

In golang net/http package. They set Max-Age to 0 when the c.MaxAge < 0.

net/http fasthttp
maxAge = 0 unset unset
maxAge > 0 Max-Age=maxAge Max-Age=maxAge
maxAge < 0 Max-Age=0 unset

https://github.com/golang/go/blob/2eac154b1c8b51d05fa5b110ae065d3610a61e06/src/net/http/cookie.go#L265-L270

if c.MaxAge > 0 {
	b.WriteString("; Max-Age=")
	b.Write(strconv.AppendInt(buf[:0], int64(c.MaxAge), 10))
} else if c.MaxAge < 0 {
	b.WriteString("; Max-Age=0")
}

@erikdubbelboer
Copy link
Collaborator

I think just supporting <0 and converting it to 0 like net/http does is best. Can you make a pull request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants