expressjs / session

Simple session middleware for Express
MIT License
6.26k stars 979 forks source link

Don't `Set-Cookie` for static/public files? #964

Closed Download closed 11 months ago

Download commented 11 months ago

I have enabled rolling. I would expect the default value for it to be true, since rolling behavior is how I always understood sessions to behave... But ok default was false so I changed it to true and watched what happened. I had enabled debug for express-session and in the logs I saw this:

GET /images/photos/browser.png 304 - - 0.883 ms
  express-session set-cookie connect.sid=####OBFUSCATED####

This to me is unexpected. Why does it Set-Cookie on a response for a static .png file?

Now maybe I am getting this all wrong. But I expected the Set-Cookie to be added only to HTML/JSON responses. Or at least, NOT to static files. And actually, I expect it only for responses on restricted/personalized pages. So for public pages (that are not different based on your login) we normally try to avoid the Set-Cookie header, because it will make the response not cacheable. For the same reasons, we would not set the Set-Cookie header for responses to public static files. If you check out the behavior of CDNs such as CloudFlare, they usually only cache responses that have no Set-Cookie header. Which makes sense. But when I enable rolling, Express starts to send the Set-Cookie header for every response. This means the CDN will be bypassed for all these responses and they will all hit my origin server. This seems unnecessary and unwanted.

Now I understand that it is tricky maybe to know if the response is 'public' or not, since auth is a separate (but related!) concern. Similarly I understand it might be equally tricky to know if the response is for a static file or not. But I do think that the current behavior of rolling (and indeed, the very existence of the option or at least its default value) are pretty unexpected. They were for me. But maybe I am simply doing it wrong? Do you have thoughts?

dougwilson commented 11 months ago

Yes, express-session has no way to know what are dynamic responses, private, public, static, etc etc. You have to scope where you use the middleware to not use it on resources you don't want it to be active on in your app.

Download commented 11 months ago

That's a very good point!