fix: non-overwriting touch semantic support. See issue #27

[sullivanpt]

Background:

• the express session store touch semantic is used to update the session expiration and assumes the session body will not be changed. This semantic is necessary to support requests handlers that reference but do not modify the session that execute in parallel with a request handler that does modify the session.
• memcached server natively supports the expected touch semantic in version 1.4.14 (actually 1.4.8 but it is buggy). Anecdotally, memcached native touch is not widely deployed, so it cannot be counted on.

Change description:

• the existing incorrect implementation of using "set" as an alias for "touch" is enhanced to use "get then set" which reduces the surface area of the race condition from the entire request cycle down to the few milliseconds needed for memcached access.
• a new optional flag serverSupportsTouch is provided to enable native memcached touch support.

Impact of change:

• existing consumers not needing the corrected semantic are not impacted.
• consumers needing corrected semantic who do not opt-in to the native touch support become less likely to experience data overwrite loss after this change as the race condition window is reduced.
• consumers who opt-in to the native touch support will get the optimal behavior.

See also issue #27.

[jasonkarns]

Yes please. The entire point of touch is to allow session stores to not resave the entire session object when unmodified.

[balor]

Looking good, will merge this soon and publish it as a next version of the lib.

[sullivanpt]

I hate to throw a wet blanket on my own PR, but as a public service announcement this change wasn't enough to avoid hitting the race condition for our older (pre 1.4.8) memcached servers. It's worth considering adding an option to completely disable and remove the (broken) touch handler for these environments. I'm not sure to code such an option though because express-session does some tricky decision making based on our exported object..

(Obviously the new memcached servers benefit from this PR as is).

[balor]

Any details about these "tricky decisions"? Can be also a link do the resource. Sorry for bugging but for the next few days i'm super-busy so that would be helpful <3

[sullivanpt]

Yes, line 167 of https://github.com/expressjs/session/blob/master/index.js determines if the store is "touch" enabled by testing if it has a "touch" function. So there is no way to conditionally remove the touch support from our object unless it can be removed before the store is initialized. Technically this might be possible, but I'm not sure of the details.

var storeImplementsTouch = typeof store.touch === 'function';

Aside, it would be really cool to use the memcached version function to set the appropriate behavior, but in my anecdotal testing not only was version identification not consistent across memcached builds but the version function sometimes threw fatal exceptions when called.