CookieJar.setCookie does not slide expiration for set-cookie headers with max-age

[jstewmon]

The creation time from old-cookie is carried forward per the spec, but this causes the expiryTime() to be unaffected by subsequent set-cookie headers, so the cookie will expire when its creation + max-age > now, even if subsequent set-cookie headers specified max-age with the intent of making the expiration now + max-age.

I'm currently working around this by just having a Store implementation that passes null to the callback, so that the creation is not carried forward.

[stash]

I reviewed the code and the RFC and I am confident that what tough-cookie is doing is the standard behaviour. I don't think there's a bug to fix here so I'd like to close this issue.

In both the code and in the RFC, creation time doesn't get changed when updating a cookie. RFC 6265 Section 5.3 defines the logic for storing a cookie and step 11.3 in particular describes what happens if a cookie is being updated. For convenience:

   11.  If the cookie store contains a cookie with the same name,
        domain, and path as the newly created cookie:

        1.  Let old-cookie be the existing cookie with the same name,
            domain, and path as the newly created cookie.  (Notice that
            this algorithm maintains the invariant that there is at most
            one such cookie.)

        2.  If the newly created cookie was received from a "non-HTTP"
            API and the old-cookie's http-only-flag is set, abort these
            steps and ignore the newly created cookie entirely.

        3.  Update the creation-time of the newly created cookie to
            match the creation-time of the old-cookie.

        4.  Remove the old-cookie from the cookie store.

And in the setCookie() code:

    if (oldCookie) {
      // S5.3 step 11 - "If the cookie store contains a cookie with the same name,
      // domain, and path as the newly created cookie:"
      if (options.http === false && oldCookie.httpOnly) { // step 11.2
        err = new Error("old Cookie is HttpOnly and this isn't an HTTP API");
        return cb(options.ignoreError ? null : err);
      }
      cookie.creation = oldCookie.creation; // step 11.3
      cookie.creationIndex = oldCookie.creationIndex; // preserve tie-breaker
      cookie.lastAccessed = now;
      // Step 11.4 (delete cookie) is implied by just setting the new one:
      store.updateCookie(oldCookie, cookie, next); // step 12


    } else {
      cookie.creation = cookie.lastAccessed = now;
      store.putCookie(cookie, next); // step 12
    }
  }

You can see that the creation time is deliberately not updated. Therefore, Max-Age really only takes effect on creation. Another way to put it is that (in the absence of an Expires cookie-attribute), the expiry for Max-Age cookies is always creation + max-age and never now + max-age.

I don't think there's a bug to fix since tough-cookie is following the RFC. Still, if you can demonstrate that there are browsers that are following the behaviour you describe (i.e., the now + max-age sliding expiry), then I'd accept a pull request that adds this as a non-default option to setCookie.

[jstewmon]

Sorry, I should have mentioned that I was reporting the issue because the spec doesn't mention the affect of max-age on updates, but browsers treat it as a sliding expiration.

Chrome, Firefox, and I suspect all other browsers, will update the expiration every time a response contains a set-cookie with max-age.

All you need to see for yourself is a server that sends a cookie with max-age. Open the dev inspector in your browser of choice and observe the expiration of the cookie change every time you refresh.

const http = require("http");
http
  .createServer((req, res) => {
    res.setHeader("set-cookie", "foo=bar;max-age=30");
    res.end("check cookies");
  })
  .listen(3000);

[jstewmon]

@awaterma while you’re looking through my PRs, would you mind taking a look at this too? I still think it is a valid issue.

[tennox]

I also see this problematic behaviour.
My Server sets a Cookie into Insomnia's Cookie jar, the next day I query it again - the creation time is not updated, and thus I can't use the cookie in template variables, as it won't pass the expiry check.

In a browser, the old cookie's creation time is updated, as @jstewmon mentioned

[ghost]

My reading of RFC6265 is that expiry-time and creation-time are two distinct fields (Section 5.3), and creation-time is not used for expiration at all. Steps 5.3.2 and 5.3.11.3 cover creation-time and expiry-time is covered by 5.3.3, and sections 5.2.1 and 5.2.2.

[ricellis]

We've just run into this problem too. I agree that the spec is a little ambiguous on this point because it doesn't explicitly describe the behavior of Max-Age when a cookie is updated. However, as pointed out by @mehvvy section 5.2.2 seems to cover this when describing parsing of the Set-Cookie header; it says:

... Otherwise, let the
expiry-time be the current date and time plus delta-seconds seconds.

Append an attribute to the cookie-attribute-list with an attribute
-name of Max-Age and an attribute-value of expiry-time.

This seems to suggest that Max-Age is added to the current time when the Set-Cookie header is parsed, so in that context a sliding update of the expiry-time attribute-value of an existing cookie when parsing a new Set-Cookie header probably makes sense.

[ricellis]

@ShivanKaul are you looking into this or any chance this is going to be re-opened?

[ShivanKaul]

@ricellis I recently left Salesforce and sorting out my contribution plans, I'll look at this soon.

[byroncoetsee]

Experiencing this too.
Also, because storage can't be accessed from the cookieJar object, one can't use handy functions like updateCookie or removeCookie to get around the problem.

Any ideas or updates? Maybe this has been solved since and I'm missing something?

[awaterma]

We'll discuss this in our next meeting of the team; I'll try my best to look into this before hand. :)

Thanks for asking about this @byroncoetsee and @ricellis

[byroncoetsee]

For anyone landing here, and looking for a simple workaround while this issue gets looked at, here's what I did.
NOTE: Doing it this way will drop things like creationIndex etc. It essentially recreates the cookie from scratch - and in doing so, sets the creationDate to now.

class CustomMemoryCookieStore extends MemoryCookieStore {
    updateCookie(oldCookie: Cookie, newCookie: Cookie, cb: (err: Error | null) => void): void
    updateCookie(oldCookie: Cookie, newCookie: Cookie): Promise<void>;
    updateCookie(_: any, newCookie: any, cb?: any): void | Promise<void> {
        newCookie.creation = new Date()
        this.putCookie(newCookie, cb)
        if (cb == null) {
            return Promise.resolve()
        }
    }
}

[VelynM]

My reading of RFC6265 is that expiry-time and creation-time are two distinct fields (Section 5.3), and creation-time is not used for expiration at all. Steps 5.3.2 and 5.3.11.3 cover creation-time and expiry-time is covered by 5.3.3, and sections 5.2.1 and 5.2.2.

As far as I can tell, this issue still exists, assuming my past interpretation of RFC6265 was correct.