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

ensure session.set_expiry adds or removes the Max-Age attribute to … #191

Merged
merged 1 commit into from
Apr 14, 2024

Conversation

Katze864
Copy link
Contributor

…or from the cookie

This addresses a bug where using set_expiry on a session with no initial expiry time would not add the Max-age attribute to the cookie leading to an inconsitency between the cookie and the database. Fix: #178

…or from the cookie

This addresses a bug where using `set_expiry` on a session with no initial expiry time would not add the Max-age attribute to the cookie leading to an inconsitency between the cookie and the database.
Fix: #178
Copy link

codecov bot commented Apr 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.88%. Comparing base (d7b7e13) to head (b61bc7d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #191      +/-   ##
==========================================
+ Coverage   85.75%   85.88%   +0.12%     
==========================================
  Files           5        5              
  Lines         337      340       +3     
==========================================
+ Hits          289      292       +3     
  Misses         48       48              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@maxcountryman maxcountryman merged commit ea29bbc into maxcountryman:main Apr 14, 2024
7 checks passed
@maxcountryman
Copy link
Owner

Thanks for this.

Re the expiry logic more generally: I think ultimately we should pass Expiry, via the Record to the stores and allow them to decide how to persist expiration (for example, "Session" max age could follow some store-specific protocol for expiry, rather than using the two-week default constant the session logic currently imposes).

There's also a type within cookie that we should probably use instead or wrap in some way.

Anyway these are breaking changes but it's worth ensuring better coherence here prior to a 1.0 release.

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.

Max-Age of Cookie not updates on set_expiry()
2 participants