Chrome/FF follow redirect before session is fully saved

[antishok]

Here's a simple testcase incrementing a counter: https://gist.github.com/antishok/fb3d003d16eb72f672a7cc36401657d9
On chrome I have to refresh after incrementing in order to see the new count.

In response to a request, I write to the session and send a redirect to another page, but the new page is rendered with the old session data, and the changes I just wrote are not visible. The new data appears only after refreshing the page.

This only happens in Chrome, maybe due to an optimization they added that follows a redirect before the redirect response even ended.

I'm using connect-session-knex with postgresql to store my session data. If I use the MemoryStore, or use sqlite3 instead of postgresql, it works fine, but probably only because writing to them is much quicker.

The gist also includes debug logs for the request (one when I tested with chrome, and one with firefox). You can see that for chrome, the new url is fetched before the session has finished saving, and before the redirect response ends.
(The logs include a log I manually added in the beginning of express-session's writeend() function, to indicate when a response actually ends)

Thanks! And thanks to @joepie91 for figuring out where the issue lies

[joepie91]

To clarify - express-session only delays the end of the response itself, not the chunks that are sent... meaning that Chrome will technically obtain the Location header before the session completes saving, and it seems to use that to pre-fetch the next page ahead of time.

I'm not sure whether that's allowed by the HTTP specification, but it's causing issues here either way. Other session handling libraries might also be affected...

[antishok]

When trying to make a testcase without a database, I ran into some more weirdness. I tried to use a MemoryStore and delaying the write:

var store = new session.MemoryStore();
var fake = {
  set: (...args) => setTimeout(() => store.set(...args), 2000),
  get: (...args) => store.get(...args),
  createSession: (...args) => store.createSession(...args),
  touch: (...args) => store.touch(...args),
  destroy: (...args) => store.destroy(...args),
  length: (...args) => store.length(...args),
  list: (...args) => store.list(...args),
  expired: (...args) => store.expired(...args),
  on: (...args) => store.on(...args)
};

and in this case the same issue seems to happen, but this time both in Chrome and in Firefox.
On the other hand, IE11 works just fine.

Versions: Chrome 52.0.2743.116 m, Firefox 48.0.2

[gabeio]

how are you redirecting the user? client side (javascript/angular?) or server side (express?)

if client side you might be redirecting without the browser actually accepting the full header.
if server side what redirection code are you using?

both: it could be that the cookie is not being sent high enough in the header (above the new location) and the browser preemptively opens the next connection without the new cookie (though in chrome I believe they still will wait and then send the cookies... so, strange.)

also what OS are you using? (might be related 😕 )

---

if the browser is just not waiting and then still not sending the cookies there isn't anything we can do to fix this in this library.

[joepie91]

how are you redirecting the user? client side (javascript/angular?) or server side (express?)

Server-side, using res.redirect.

both: it could be that the cookie is not being sent high enough in the header (above the new location) and the browser preemptively opens the next connection without the new cookie (though in chrome I believe they still will wait and then send the cookies... so, strange.)

Per the debug log, the request for the redirect target is made before the redirect response is ended. The problem seems to be that express-session makes the assumption that "the browser won't attempt to act on the response until we've closed the connection", and Chrome and Firefox are violating that assumption - at least, for redirects.

also what OS are you using? (might be related 😕 )

If I recall correctly, @antishok is using Windows.

if the browser is just not waiting and then still not sending the cookies there isn't anything we can do to fix this in this library.

The problem isn't sending cookies - it's that the second request is made before the first request has completed updating the session data. The timeline should look like this:

1. Request 1 sent
2. Response 1 sent, but connection left open
3. Session data for response 1 started saving
4. Session data for response 1 completed saving
5. Response 1 closed
6. Browser follows Location header
7. Request 2 sent
8. Response 2 sent

However, instead, the timeline looks like this:

1. Request 1 sent
2. Response 1 sent, but connection left open
3. Session data for response 1 started saving
4. Browser follows Location header
5. Request 2 sent
6. Response 2 sent
7. Session data for response 1 completed saving
8. Response 1 closed

Thus, the second request and response are sent before the session data changes in the first response have a chance to complete saving, and consequently the second request/response see 'stale' session data.

Possible solutions include buffering up all writes until the session data is saved (probably not practical), or at least delaying res.redirect calls until after the session completes saving, if that's the only known case of browsers being a little too clever about pre-fetching.

[dougwilson]

@joepie91 there are a bunch of existing issues about this same thing already, even a (stalled) PR: #157

[joepie91]

@dougwilson I'll have a look at that PR soon, and see if I can work out a way to move it forwards. Thanks!

[gabeio]

@joepie91

If you notice most typical large applications like forums and others use a bounce page which is literally meant to setCookies from and then the browser is redirected (typically using a javascript and or meta tag redirect) from there instead of from a redirect which is typically bad practice as you also can't easily tell if the user is just blocking cookies in general. Google, Twitter & many more also uses this practice of a bounce page with logins (check the network locations & headers)

it is usually very small page:

<html>
  <head><meta http-equiv="refresh" content="5; url=http://example.com/"></head>
  <body onload="window.location = 'http://example.com';"></body>
</html>

[antishok]

@gabeio the issue isn't about cookies. The cookie only needs to be written once when creating the session (cookie only contains session-id). When the session is updated there is no need to set any cookie again - only to write to the session-store (server-side). The issue is that the browser requests the new page before the session-store has been updated, and so it sees old data.

I don't really understand what you mean about a "bounce page" solution or where it's used, but in any case it doesn't really seem like an adequate workaround..

One workaround is to save the session before redirecting, but that doesn't help me when using a library such as passportjs which does the redirects for me, on login success/fail.

[gabeio]

Sorry I was misunderstanding that this wasn't a login which if you don't have a cookie and you try to give a cookie in a redirect it doesn't really work all the time especially if the location comes across first. But as for the browser viewing old data... Sounds like you might need a read lock of sorts.

[joepie91]

@gabeio The only thing that needs changing to resolve this issue, is to not write the response headers at all until the session save has completed. The question is just how to do this effectively without breaking the API.

[gabeio]

@joepie91 Couldn't this be avoided by adding req.session.save() right after line#30 I get that, that is annoying to have to do for all redirects but I feel like buffering the entire request and waiting for a full write for anyone who redirects isn't exactly good performance wise either, especially since you currently can strategically place the force saves.

[joepie91]

Couldn't this be avoided by adding req.session.save() right after line#30

Yes, it could, but this would be a leaky abstraction and a footgun.

but I feel like buffering the entire request and waiting for a full write for anyone who redirects isn't exactly good performance wise either

I understand the concerns, but this would not be any different when calling save manually - you still have to wait for the session save to complete before writing the header. Since express-session already short-circuits when there's no modified session data to save, a patch in express-session for redirects would not incur a performance penalty.

especially since you currently can strategically place the force saves.

That's a workaround, not a fix.