Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Session cookies issue when upgrading from v1 to v2 #45

Open
viraptor opened this issue Aug 23, 2022 · 5 comments
Open

Session cookies issue when upgrading from v1 to v2 #45

viraptor opened this issue Aug 23, 2022 · 5 comments

Comments

@viraptor
Copy link

viraptor commented Aug 23, 2022

When trying to upgrade from v1 to v2 I get the following stacktrace when using the express-session:

TypeError: Cannot read properties of undefined (reading 'expires') (Most recent call first)
at MemcachedStore.Store.createSession (/var/www/app/node_modules/express-session/session/store.js line 87 col 28)
at inflate (/var/www/app/node_modules/express-session/index.js line 372 col 12)
at <unknown> (/var/www/app/node_modules/express-session/index.js line 499 col 10)
at Object.<anonymous> (/var/www/app/node_modules/connect-memcached/lib/connect-memcached.js line 99 col 1)
at exports.AsyncResource.runInAsyncScope (node:async_hooks line 202 col 8)
...

I'm still trying to dig into the details, but the missing session.cookies doesn't look great.
This is for code not using encryption.

@balor
Copy link
Owner

balor commented Aug 25, 2022

@viraptor Thanks for the input. Please submit a little more information about the runtime environment: node version, used libraries with their versions, some meaningful code fragments etc.

@balor
Copy link
Owner

balor commented Aug 25, 2022

Just for the record, I've tested scenarios with live v1->v2 migration (without encryption) and there were no errors, so I need a little more data to recreate the scenario.

@ostigley
Copy link

On behalf of @viraptor, we are using Node 16.16.0

These are our dependencies.

 "dependencies": {
    "async": "~3.2",
    "bluebird": "^3.7.2",
    "body-parser": "~1.20.0",
    "bootable": "^0.2.4",
    "bunyan": "^1.8.12",
    "coffeescript": "~2.7.0",
    "connect-flash": "^0.1.1",
    "connect-memcached": "^2.0.0",
    "connect-timeout": "^1.7.0",
    "cookie-parser": "~1.4.6",
    "cors": "^2.5.3",
    "csurf": "^1.10.0",
    "dd-trace": "^2.12.2",
    "ecad": "^0.2.0",
    "errorhandler": "^1.5.1",
    "express": "^4.18.1",
    "express-cluster": "0.0.5",
    "express-http-proxy": "^1.6.3",
    "express-session": "^1.17.3",
    "heroku-ssl-redirect": "0.0.2",
    "lodash": ">=4.17.20",
    "memcached": "^2.1.0",
    "method-override": "~3.0.0",
    "mockery": "^2.1.0",
    "morgan": "~1.10.0",
    "newrelic": "^8.17.1",
    "oauth20-provider": "envato/node-oauth20-provider#envato-api-gateway",
    "parse-database-url": "^0.3.0",
    "pg": "^8.7.3",
    "pg-hstore": "^2.3.4",
    "pug": "^3.0.2",
    "randomstring": "^1.2.2",
    "raw-body": "^2.5.1",
    "require-subvert": "^0.1.0",
    "rollbar": "^2.25.1",
    "sequelize": "^5.22.5",
    "sequelize-cli": "^6.4.1",
    "type-is": "^1.6.12",
    "x-frame-options": "1.0.0"
  },

Our implementation is simple, no encryption:

session = require('express-session')
MemcachedStore = require('connect-memcached')(session)

app.use(Middleware.sessionStore(
  secret: process.env.EXPRESS_SESSION_SECRET,
  resave: false,
  saveUninitialized: false,
  store: new MemcachedStore({
    hosts: global.memcacheHosts,
    prefix: 'api-gateway:session:'
  })
))

Can I ask, in v2.0.0, it seems like the parseable string is not JSON parsed anymore. Is this a possible cause? v1.0.0...v2.0.0#diff-60110081947a55d72cbc180c13f565fd978c43ad0dbb393ff9719c654193b634L88

Here's a screenshot of our rollbar trace:
Screenshot 2022-09-16 at 2 41 32 PM

@balor
Copy link
Owner

balor commented Sep 20, 2022

To my knowledge, in such case, Memcached client should directly pass Object as the data in callback function, which is then passed using variable parseable_string (quite misleading name) to createSession. String parsing is occurring only during decryption for obvious reasons. Also look that the error is suggesting that the whole data is undefined, which suggest the problem is somewhere else? If the problem would be that the store is passing string instead object, then whole different error would appear. Previously 1.0.0 the data would be unnecessary dumped to string and again converted to Object, so the current behavior makes more sense.

These are just some fast observations, will run some experiments later. By the time don't hesitate to put some breakpoints or logs and dubug your specific case.

@jas-
Copy link
Contributor

jas- commented Sep 20, 2022

Serialization of the cookie data may be at play here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants