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

res.cookie does not allow setting Max-Age only, without Expires (One should be able to set maxAge alone, with expires: 0) #5150

Open
nbkhope opened this issue Mar 19, 2023 · 4 comments

Comments

@nbkhope
Copy link

nbkhope commented Mar 19, 2023

I want to set a cookie with Max-Age only, without having Expires. But the following lines 875-876 keep adding the unwanted Expires:

express/lib/response.js

Lines 871 to 878 in 0debedf

if (opts.maxAge != null) {
var maxAge = opts.maxAge - 0
if (!isNaN(maxAge)) {
opts.expires = new Date(Date.now() + maxAge)
opts.maxAge = Math.floor(maxAge / 1000)
}
}

Setting expires: 0 does no good to override that behavior.

I think one should be allowed to only set Max-Age without automatically having Expires also set.

res.cookie('MyCookie', 'TheValue', {
  expires: 0, // I don't want any Expires in the resulting Set-Cookie statement
  maxAge: 60000,
});

Actual result:

MyCookie=TheValue; Max-Age=60; Path=/; Expires=Sun, 26 Mar 2023 06:00:31 GMT

Desired result:

MyCookie=TheValue; Max-Age=60; Path=/; 

Workaround

The workaround for this is our having to manually write the Set-Cookie statement, possibly mimicking the same logic in the express code.

Possible solution

if (opts.expires !== 0) {
  opts.expires = new Date(Date.now() + maxAge)
}
@tjarbo
Copy link

tjarbo commented Apr 4, 2023

The RFC6265 - section 4.1.2.2 describes the following behavior:

If a cookie has both the Max-Age and the Expires attribute, the Max-Age attribute has precedence and controls the expiration date of the cookie.

Therefore in the usecase that you described, your proposed solution won't make a difference except the used "user agent"/browser is not implementing RFC6265 properly.

@dougwilson
Copy link
Contributor

Hi @tjarbo that is correct, user agents that support max-age attribute should ignore expires attribute. But since max-age was an add on to set-cookie later in life, not all clients support it, so they will use expires instead (since they don't understand max-age. This is why Express is adding both, so all clients will know when the cookie expires and won't accidentally set it as a session-length cookie. Maybe better understanding of @nbkhope use-case will help.

@tjarbo
Copy link

tjarbo commented Apr 5, 2023

Hi @dougwilson, wow - I really had to go far back in history to find a well-known specification which describes Set-Cookie without 'max-age' 😆 Netscape sends it greetings: https://curl.se/rfc/cookie_spec.html. For my understanding, is it one of the objectives of express to support these outdated ("outdated" as today's RFCs describe a different behavior for handling 'Set-Cookie) user agents?

So @nbkhope, I also do not see an issue with the current behavior of express. As dougwilson said, maybe an explanation of your use-case could help.

@dougwilson
Copy link
Contributor

dougwilson commented Apr 5, 2023

Hi @tjarbo there are many other user agents then web browsers. And many still ezist today that never kept up with the changes in specs. It was added to express in reponse to users have issues is all. As for web browsers, I believe Internet Explorer never supported max-age until IE8, well after Netscape stopped being relevant.

And I wouldn't say express objective is to support every web browser in existence or anything. In this case it just seemed like people where reporting an issue, and adding expires fixed it and we didn't see any harm, as all modern clients just ignore it and use max age (and even thenusing expires would have the same result as long as the two clocks are not out of sync).

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

No branches or pull requests

4 participants