koajs / session

Simple session middleware for koa
MIT License
903 stars 113 forks source link

bug: maxAge:'session' is not work as expected #133

Closed valaxy closed 6 years ago

valaxy commented 6 years ago

reproduce the problem:

const Koa = require('koa')
const KoaSesson = require('koa-session')
const app = new Koa()

app.keys = ['a296ac73-192d-4632-a0cc-dba28b821820']

app.use(KoaSesson({
    key: 'some-session-key',
    maxAge: 'session'
}, app))

app.use(async function(ctx) {
    if (ctx.path === '/favicon.ico') return

    let n = ctx.session.views || 0
    ctx.session.views = ++n
    ctx.body = 'Hello Koa' + n
})

app.listen(13001)
  1. open browser in http://localhost:13001/ and refresh the page many times
  2. delete cookies
  3. also open http://localhost:13001/ and refresh many times
  4. close browser and open it again you can see: the cookie is not erased as expected!

Where could the bug come from? I doubt that bug may be produced at https://github.com/koajs/session/blob/master/lib/context.js#L293

if (maxAge === 'session') {
    opts.maxAge = undefined; // ← maybe maxAge should not be overridden by undefined
    json._session = true;
}
linxea commented 6 years ago

how do you close the browser. you need to quit it completely if you are using macbook. Clicking on the close button does not quit the app. I had the same problem before. Thought I closed the browser.

s1v7nteen commented 6 years ago

i use the maxAge: 'session' is not work as expected too. i have to set maxAge = 'session' every time, when i want to set session to user.

erroric commented 6 years ago

+1

fix from https://github.com/koajs/session/commit/c487944c22056fdd37433bdeab3d665dbd116744 is not working

I have workaround for this behaviour:

app.use(KoaSesson({
    key: 'some-session-key',
    maxAge: 'session',
    beforeSave() {
        this.maxAge = 'session';
    },
}, app))

beforeSave callback restoring maxAge value correctly, otherwise it is replaced with ONE_DAY value

whxaxes commented 6 years ago

https://github.com/koajs/session/pull/136 fixed this bug