connect-flash icon indicating copy to clipboard operation
connect-flash copied to clipboard

Req.flash() automatic set session when it's not required to do so.

Open ducdigital opened this issue 8 years ago • 5 comments

When in read-only scenario. Which flash is not yet existed. running req.flash('name') will set req.session.flash to {} which will prevent caching.

Solution:

  • Make sure req.session.flash is set only when req.flash() have 2 args.
  • Make sure fetching flash data return empty array if req.session.flash does not exists

ducdigital avatar May 04 '16 09:05 ducdigital

Here's a slightly more elegant solution:

module.exports = function flash(options) {
    options = options || {};
    var safe = (options.unsafe === undefined) ? true : !options.unsafe;

    return function (req, res, next) {
        if (req.flash && safe) { return next(); }
        req.flash = _flash;
        // BEGIN ADD
        var _end = res.end;
        res.end = function() {
            if (Object.keys(req.session.flash || {}) == 0) {
                delete req.session.flash;
            }
            _end.apply(this, arguments);
        };
        // END ADD
        next();
    }
}

This proxies the end response and removes the flash object from the session if it doesn't have any keys. A little less juggling than @ducdigital had in his PR

brendonparker avatar May 18 '16 18:05 brendonparker

We needed to patch connect-flash for similar reasons. Here's a quick one-line addition that should do the trick:

    function _flash(type, msg) {
      if (this.session === undefined) throw Error('req.flash() requires sessions');
+     if (arguments.length < 2 && !this.session.flash) return [];
      var msgs = this.session.flash = this.session.flash || {};
      ...
    }

jmar777 avatar Apr 04 '17 21:04 jmar777

Updates on this issue?

manuel-di-iorio avatar Sep 04 '18 11:09 manuel-di-iorio

Hi, I am using Session in conjunction with PassportJS with failureFlash set to true, which internally uses this connect-flash module. I did not want the uninitialized sessions to be persisted in my session store, so I have my session setup with saveUninitialized set to false. When I try to read any errors - req.flash('error'), connect-flash is adding an empty flash object to the session, which will be treated as a modification to the session, causing it to be saved.

A similar issue existed in Passport JS itself wherein it used to add an empty Passport object to the session for use after a user is authenticated, which was later fixed. I think that the session should not be touched until a value is stored in the flash area.

pravinkalekar avatar Oct 02 '18 01:10 pravinkalekar

This issue really could get more attention. It's causing all those bot requests to create empty sessions as well as sessions (+cookies) for every user, even before they consent to any. Setting saveUninitialized to false was a whole reason to prevent this behavior, but there's no way around not setting uninitialized sessions with this issue unresolved other than disabling flash altogether. I noticed there was 1 or 2 pull requests regarding this one.

lukaromih avatar Oct 21 '19 10:10 lukaromih