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

Set initial maxAge #175

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Set initial maxAge #175

wants to merge 1 commit into from

Conversation

zavr-1
Copy link

@zavr-1 zavr-1 commented Jun 29, 2019

The initial cookie that this middleware drops does not have expires, because maxAge is never set in properties. It is only set later when decoding the cookie, and only if it has been updated, therefore if no data was updated, the cookie is always limited to the session.

Screenshot 2019-06-29 at 04 17 48

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 0bd88b7 on idiocc:master into a4dcdc4 on koajs:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 0bd88b7 on idiocc:master into a4dcdc4 on koajs:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 0bd88b7 on idiocc:master into a4dcdc4 on koajs:master.

@jmitchell38488
Copy link
Contributor

jmitchell38488 commented Aug 18, 2019

There's logic in context.js to handle setting the default maxAge for a cookie, if it isn't explicitly set to session:

session/lib/context.js

Lines 290 to 307 in 10bb122

async save(changed) {
const opts = this.opts;
const key = opts.key;
const externalKey = this.externalKey;
let json = this.session.toJSON();
// set expire for check
let maxAge = opts.maxAge ? opts.maxAge : ONE_DAY;
if (maxAge === 'session') {
// do not set _expire in json if maxAge is set to 'session'
// also delete maxAge from options
opts.maxAge = undefined;
json._session = true;
} else {
// set expire for check
json._expire = maxAge + Date.now();
json._maxAge = maxAge;
}

ONE_DAY is defined in context.js:

const ONE_DAY = 24 * 60 * 60 * 1000;

@zavr-1
Copy link
Author

zavr-1 commented Aug 18, 2019

@jmitchell38488 yes but

session/lib/context.js

Lines 241 to 250 in 10bb122

_shouldSaveSession() {
const prevHash = this.prevHash;
const session = this.session;
// force save session when `session._requireSave` set
if (session._requireSave) return 'force';
// do nothing if new and not populated
const json = session.toJSON();
if (!prevHash && !Object.keys(json).length) return '';

plus my screenshot clearly shows that session's max age is not set.

@jmitchell38488
Copy link
Contributor

What's your option config?

@zavr-1
Copy link
Author

zavr-1 commented Dec 22, 2019

@jmitchell38488 there's no config. why don't you just try it for yourself and see

@zavr-1
Copy link
Author

zavr-1 commented Dec 22, 2019

setup

import Koa from 'koa'
import { aqt } from 'rqt'
import session from 'koa-session'

const koa = new Koa()
const s = session(koa, {
  signed: false,
})
koa.use(s)
koa.use((ctx, next) => {
  if (ctx.path == '/max-age') {
    ctx.session.maxAge = 60 * 60 * 1000
  }
  if (ctx.path == '/confirm') {
    ctx.session.user = 'update'
  } else {
    ctx.session.user = 'hello'
  }
  ctx.body = '# ' + ctx.path
})

test

koa.listen(async function() {
  const a = 'http://localhost:' + this.address().port
  let res
  res = await aqt(a)
  log(res)
  res = await aqt(a + '/max-age')
  const { headers: { 'set-cookie': setCookie } } = res
  log(res)
  res = await aqt(a + '/test', {
    headers: { cookie: setCookie },
  })
  // console.log(res)
  log(res)
  res = await aqt(a + '/confirm')
  log(res)
  this.close()
})

const log = (res) => {
  const { body, headers: { 'set-cookie': cookie = [] } } = res
  console.log(body)
  console.log(cookie.map(s => s.split('; ').join('\n ')).join('\n'))
}

output

  1. set session without max age
# /
koa:sess=eyJ1c2VyIjoiaGVsbG8iLCJfZXhwaXJlIjoxNTc3MDY4Nzk1NjA5LCJfbWF4QWdlIjo4NjQwMDAwMH0=
 path=/
 httponly
  1. set with max age
# /max-age
koa:sess=eyJ1c2VyIjoiaGVsbG8iLCJfZXhwaXJlIjoxNTc2OTg1OTk1NjcyLCJfbWF4QWdlIjozNjAwMDAwfQ==
 path=/
 expires=Sun, 22 Dec 2019 03:39:55 GMT
 httponly
  1. accessing the page with cookies without updating them
# /test
  1. step 1 again for blank session, no max age
# /confirm
koa:sess=eyJ1c2VyIjoidXBkYXRlIiwiX2V4cGlyZSI6MTU3NzA2ODc5NTY5NCwiX21heEFnZSI6ODY0MDAwMDB9
 path=/
 httponly

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

Successfully merging this pull request may close these issues.

3 participants