search

ca98am79 / connect-dynamodb

DynamoDB session store for Connect
http://ca98am79.github.com/connect-dynamodb/
MIT License
144 stars 66 forks source link

Fix expiry based on TTL; fix error handling of `dynamodb:DescribeTable` #87

Closed jackmuskopf closed 7 months ago

jackmuskopf commented 8 months ago

Addresses this issue: https://github.com/ca98am79/connect-dynamodb/issues/86

Also changes error handling for DescribeTable to assume the table does not exist only if the error is ResourceNotFoundException

jackmuskopf commented 8 months ago

In this PR https://github.com/ca98am79/connect-dynamodb/pull/83/files changes everything from ms to seconds, which is correct, since DynamoDB TTL uses epoch time in seconds. However, you can see that sess.cookie.maxAge does not change in this PR, which it should because the Express session cookie maxAge is specified in ms. It needs to be converted to seconds to work with DynamoDB TTL.

I am also running this patch now, let me know if anything needs clarified for fixed. Happy to look at it more.

Thanks, Jack

tfrancois commented 7 months ago

The PR is actually wrong gents. The update should be this:

? now + sess.cookie.maxAge / 1000 : now + oneDayInSeconds;

jackmuskopf commented 7 months ago

The PR is actually wrong gents. The update should be this:

? now + sess.cookie.maxAge / 1000 : now + oneDayInSeconds;

Isn't that what the PR has? The : now + oneDayInSeconds; is just on the next line (which was not changed)

ca98am79 commented 7 months ago

thank you!

MaxMEllon commented 1 month ago

@jackmuskopf @tfrancois @ca98am79

This patch include a issue. Because cookie max-age expected second (is not mill second) by cookie specification. A cookie expired time become multiply thousandfold If this patch used with cookie.maxAge option.

The Max-Age attribute indicates the maximum lifetime of the cookie, represented as the number of seconds until the cookie expires. refs : https://httpwg.org/specs/rfc6265.html#max-age-attribute

MaxMEllon commented 1 month ago

I've noticed something. It seems that express-session sets “cookie.maxAge” in mill seconds while next-session sets “cookie.maxAge” in seconds. So the problem caused in my application when upgraded connect-dynamodb@3.0.3 from connect-dynamodb@3.0.2. However, this may not be a connect-dynamodb problem.

refs: https://github.com/hoangvvo/next-session/issues/369 refs: https://github.com/hoangvvo/next-session/pull/386