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

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

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

jackmuskopf
Copy link
Contributor

@jackmuskopf jackmuskopf commented Oct 25, 2023

Addresses this issue:
#86

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

…n; otherwise, other errors might be silenced
@jackmuskopf
Copy link
Contributor Author

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
Copy link

tfrancois commented Nov 24, 2023

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

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

@jackmuskopf
Copy link
Contributor Author

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
Copy link
Owner

thank you!

@ca98am79 ca98am79 merged commit 9712f5d into ca98am79:master Nov 27, 2023
@MaxMEllon
Copy link

MaxMEllon commented May 21, 2024

@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
Copy link

MaxMEllon commented May 21, 2024

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 [email protected] from [email protected].
However, this may not be a connect-dynamodb problem.

refs: hoangvvo/next-session#369
refs: hoangvvo/next-session#386

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.

4 participants