search

roccomuso / memorystore

express-session full featured MemoryStore layer without leaks!
MIT License
120 stars 20 forks source link

Session cookie options (such as maxAge/expires) are wiped out on touch() #1

Closed windexlight closed 7 years ago

windexlight commented 7 years ago

memorystore 1.5.1

After trying out this module as a session store, I noticed that my SID cookies were being sent as session cookies (no expire time), even though I was specifying a maxAge in the cookie options when initializing express-session. The same initialization with maxAge was being respected when using the default MemoryStore. After some debugging, I traced the problem to line 181 in memorystore.js, in the touch() implementation. After retrieving and parsing the session data from the underlying LRU cache, the temporary object's "cookie" property is set equal to the entire "sess" object passed into the function. The "sess" object contains a "cookie", so the result is that the real cookie data resides at sess.cookie.cookie, instead of sess.cookie.

Changing this line (181 in memorystore.js):

s.cookie = sess

to:

s.cookie = sess.cookie

resolves the issue for me.

roccomuso commented 7 years ago

Delivered with v1.6.0.

roccomuso commented 7 years ago

Initially this module was intended to replace the express-session built-in MemoryStore module. That was "buggy" by default and not 100% compliant with their store documentation.

We now avoid support for node v0.8 bumping the lru-cache module to v4.0.3. Everything shipped with v1.6.0.

windexlight commented 7 years ago

Thanks for the prompt fix!