fix: manually persisting the session should make isModified return false
#201
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SimenB
commented
Jul 3, 2023
| @@ -898,13 +898,60 @@ test('when rolling is false, only save session when it changes', async t => { | |||
| const response2 = await fastify.inject({ path: '/', headers: { cookie: `sessionId=${sessionId}` } }) | |||
| const setCookieHeader2 = response2.headers['set-cookie'] | |||
|
|
|||
| t.equal(response1.statusCode, 200) | |||
| t.equal(response2.statusCode, 200) | |||
There was a problem hiding this comment.
noticed my old test checked the wrong object 😅
SimenB
changed the title
isModified return falseisModified return false
|
This might have caused a regression where the cookie isn't set as it should in the I won't be able to look into it until tomorrow |
4 tasks
bodinsamuel
pushed a commit
to specfy/specfy
that referenced
this pull request
Aug 21, 2023
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@fastify/session](https://togithub.com/fastify/session) | [`10.2.0` -> `10.4.0`](https://renovatebot.com/diffs/npm/@fastify%2fsession/10.2.0/10.4.0) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>fastify/session (@​fastify/session)</summary> ### [`v10.4.0`](https://togithub.com/fastify/session/releases/tag/v10.4.0) [Compare Source](https://togithub.com/fastify/session/compare/v10.3.1...v10.4.0) #### What's Changed - fix: make sure to set cookie on manual session saves by [@​SimenB](https://togithub.com/SimenB) in [fastify/session#203 **Full Changelog**: fastify/session@v10.3.1...v10.4.0 ### [`v10.3.1`](https://togithub.com/fastify/session/releases/tag/v10.3.1) [Compare Source](https://togithub.com/fastify/session/compare/v10.3.0...v10.3.1) #### What's Changed - ci: only trigger on pushes to main branches by [@​Fdawgs](https://togithub.com/Fdawgs) in [fastify/session#198 - build(deps-dev): bump [@​types/node](https://togithub.com/types/node) from 18.16.5 to 20.1.0 by [@​dependabot](https://togithub.com/dependabot) in [fastify/session#199 - fix: manually persisting the session should make `isModified` return false by [@​SimenB](https://togithub.com/SimenB) in [fastify/session#201 **Full Changelog**: fastify/session@v10.3.0...v10.3.1 ### [`v10.3.0`](https://togithub.com/fastify/session/releases/tag/v10.3.0) [Compare Source](https://togithub.com/fastify/session/compare/v10.2.1...v10.3.0) #### What's Changed - build(deps-dev): bump sinon from 13.0.2 to 14.0.0 by [@​dependabot](https://togithub.com/dependabot) in [fastify/session#88 - update to Fastify v4 by [@​mcollina](https://togithub.com/mcollina) in [fastify/session#89 - feat: Update benchmark. by [@​ShogunPanda](https://togithub.com/ShogunPanda) in [fastify/session#91 - ci: migrate to fastify reusable workflow by [@​Fdawgs](https://togithub.com/Fdawgs) in [fastify/session#90 - build(deps-dev): bump fastify-cookie from 5.6.1 to 5.7.0 by [@​dependabot](https://togithub.com/dependabot) in [fastify/session#92 - docs(readme): remove snyk badge by [@​Fdawgs](https://togithub.com/Fdawgs) in [fastify/session#93 - chore(.gitignore): use updated skeleton template by [@​Fdawgs](https://togithub.com/Fdawgs) in [fastify/session#96 - build(deps-dev): bump [@​types/node](https://togithub.com/types/node) from 17.0.45 to 18.0.0 by [@​dependabot](https://togithub.com/dependabot) in [fastify/session#97 - build(deps-dev): bump tsd from 0.20.0 to 0.21.0 by [@​dependabot](https://togithub.com/dependabot) in [fastify/session#98 - build(deps-dev): bump tsd from 0.21.0 to 0.22.0 by [@​dependabot](https://togithub.com/dependabot) in [fastify/session#102 - style(types/types.test-d.ts): explicitly declare semicolon by [@​Fdawgs](https://togithub.com/Fdawgs) in [fastify/session#103 - refactor: use optional chaining by [@​Fdawgs](https://togithub.com/Fdawgs) in [fastify/session#104 - build(deps): bump fastify-plugin from 3.0.1 to 4.0.0 by [@​dependabot](https://togithub.com/dependabot) in [fastify/session#105 - remove metadata.js by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#107 - separate lint from npm test script, activate lint and license-check in ci pipeline by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#108 - refactor unit tests, replace ava with tap, remove undici and typescript as devDependencies by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#109 - move benchmark to corresponding folder, slim it down by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#110 - tap should only run test.js files by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#115 - Clean up tests by [@​rclmenezes](https://togithub.com/rclmenezes) in [fastify/session#117 - Add missing documentation for README.md by [@​rclmenezes](https://togithub.com/rclmenezes) in [fastify/session#116 - Simplify shouldSaveSession code a little bit by [@​rclmenezes](https://togithub.com/rclmenezes) in [fastify/session#119 - Move `expires` from session to session.cookies by [@​rclmenezes](https://togithub.com/rclmenezes) in [fastify/session#120 - refactor store to es6 class by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#122 - simplify callback typing by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#124 - Add cookiePrefix as an option to allow for compatibility with express-session by [@​rclmenezes](https://togithub.com/rclmenezes) in [fastify/session#113 - perf: improve idgenerator (about 25-30 % faster) by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#123 - chore: remove addDataToSession by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#127 - add autocannon example by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#128 - feat: use [@​fastify/cookie-signer-logic](https://togithub.com/fastify/cookie-signer-logic) for signing and unsigning by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#129 - move signer require into ensureDefaults by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#130 - fix unit test by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#132 - fix lgtm alert by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#133 - Remove session id round 4 by [@​rclmenezes](https://togithub.com/rclmenezes) in [fastify/session#134 - simplify getExpires, export getExpires from cookie by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#135 - remove internal used session restore() function by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#136 - Revert "refactor store to es6 class" by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#138 - remove sinon as dev dependency by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#139 - add redis example by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#142 - add pre-commit by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#145 - add unit test for maxAge by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#144 - create a separate unit test file for memorystore by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#150 - rename secret.test.js to fastifySession.checkOptions.test.js and refactor it by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#151 - prevent encryptedSessionId from being stored in the sessionStore by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#143 - use maxAge over expires by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#152 - build(deps-dev): bump tsd from 0.22.0 to 0.23.0 by [@​dependabot](https://togithub.com/dependabot) in [fastify/session#153 - Resave session cookie if saveUninitialized is false and rolling is true by [@​rclmenezes](https://togithub.com/rclmenezes) in [fastify/session#149 - rename session to fastifySession by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#160 - Make it possible to call save() in decryptSession by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#161 - Implement originalMaxAge by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#162 - Remove deprecation notice for fastify-Instance decryptSession by [@​shrihari-prakash](https://togithub.com/shrihari-prakash) in [fastify/session#164 - add typings for decryptSession by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#165 - simplify cookie handling by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#166 - minor cosmetic change by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#167 - ensure that secure false is not overwritten by null by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#168 - security: verify path of cookie/session by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#169 - build(deps-dev): bump tsd from 0.23.0 to 0.24.1 by [@​dependabot](https://togithub.com/dependabot) in [fastify/session#171 - Little remark fastify-cookie -> [@​fastify/cookie](https://togithub.com/fastify/cookie) by [@​Luchanso](https://togithub.com/Luchanso) in [fastify/session#172 - fix rolling location in doc by [@​HPieters](https://togithub.com/HPieters) in [fastify/session#175 - fix: set domain when clearing cookie by [@​TheWashiba](https://togithub.com/TheWashiba) in [fastify/session#174 - fix: align cookie implementation with express-session by [@​climba03003](https://togithub.com/climba03003) in [fastify/session#177 - nodenext compatibility by [@​Uzlopak](https://togithub.com/Uzlopak) in [fastify/session#178 - build(deps-dev): bump tsd from 0.24.1 to 0.25.0 by [@​dependabot](https://togithub.com/dependabot) in [fastify/session#179 - types: fix Session interface reference by [@​FabianFrank](https://togithub.com/FabianFrank) in [fastify/session#182 - chore(.gitignore): add clinic by [@​Fdawgs](https://togithub.com/Fdawgs) in [fastify/session#183 - chore(.gitignore): add bun lockfile by [@​Fdawgs](https://togithub.com/Fdawgs) in [fastify/session#185 - build(deps-dev): bump connect-redis from 6.1.3 to 7.0.0 by [@​dependabot](https://togithub.com/dependabot) in [fastify/session#186 - build(deps-dev): bump tsd from 0.25.0 to 0.27.0 by [@​dependabot](https://togithub.com/dependabot) in [fastify/session#187 - build(deps-dev): bump tsd from 0.27.0 to 0.28.0 by [@​dependabot](https://togithub.com/dependabot) in [fastify/session#188 - fix: express session type compatibility by [@​ojeytonwilliams](https://togithub.com/ojeytonwilliams) in [fastify/session#190 - feat: add ignoreFields option to regenerate method by [@​RafaelGSS](https://togithub.com/RafaelGSS) in [fastify/session#189 - fix: separate an actual path from url search part before verification by [@​ohchi](https://togithub.com/ohchi) in [fastify/session#192 - fix: lint by [@​Eomm](https://togithub.com/Eomm) in [fastify/session#195 - feat!: stricter types by [@​climba03003](https://togithub.com/climba03003) in [fastify/session#196 #### New Contributors - [@​mcollina](https://togithub.com/mcollina) made their first contribution in [fastify/session#89 - [@​ShogunPanda](https://togithub.com/ShogunPanda) made their first contribution in [fastify/session#91 - [@​Uzlopak](https://togithub.com/Uzlopak) made their first contribution in [fastify/session#107 - [@​rclmenezes](https://togithub.com/rclmenezes) made their first contribution in [fastify/session#117 - [@​shrihari-prakash](https://togithub.com/shrihari-prakash) made their first contribution in [fastify/session#164 - [@​Luchanso](https://togithub.com/Luchanso) made their first contribution in [fastify/session#172 - [@​HPieters](https://togithub.com/HPieters) made their first contribution in [fastify/session#175 - [@​TheWashiba](https://togithub.com/TheWashiba) made their first contribution in [fastify/session#174 - [@​FabianFrank](https://togithub.com/FabianFrank) made their first contribution in [fastify/session#182 - [@​ojeytonwilliams](https://togithub.com/ojeytonwilliams) made their first contribution in [fastify/session#190 - [@​RafaelGSS](https://togithub.com/RafaelGSS) made their first contribution in [fastify/session#189 - [@​ohchi](https://togithub.com/ohchi) made their first contribution in [fastify/session#192 - [@​Eomm](https://togithub.com/Eomm) made their first contribution in [fastify/session#195 **Full Changelog**: fastify/session@v8.3.0...v10.3.0 ### [`v10.2.1`](https://togithub.com/fastify/session/releases/tag/v10.2.1) [Compare Source](https://togithub.com/fastify/session/compare/v10.2.0...v10.2.1) #### What's Changed - fix: separate an actual path from url search part before verification by [@​ohchi](https://togithub.com/ohchi) in [fastify/session#192 - fix: lint by [@​Eomm](https://togithub.com/Eomm) in [fastify/session#195 #### New Contributors - [@​ohchi](https://togithub.com/ohchi) made their first contribution in [fastify/session#192 - [@​Eomm](https://togithub.com/Eomm) made their first contribution in [fastify/session#195 **Full Changelog**: fastify/session@v10.2.0...v10.2.1 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 10pm every weekday" in timezone Europe/Paris, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/specfy/specfy). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi40My4yIiwidXBkYXRlZEluVmVyIjoiMzYuNDMuMiIsInRhcmdldEJyYW5jaCI6ImNob3JlL3Jlbm92YXRlQmFzZUJyYW5jaCJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
](https://renovatebot.com){kind=link}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
npm run testandnpm run benchmarkand the Code of conduct
Following up #80. We're seeing some data loss after updating the session in a distributed system, and the suspect is
onSendin this library storing old data, overwriting the update performed by either a different concurrent request, or a request handled by another pod.Note that this is slightly different from what
express-sessiondoes.https://github.com/expressjs/session/blob/1010fadc2f071ddf2add94235d72224cf65159c6/index.js#L403-L407
It tracks
modifiedandsavedseparately. I'm not sure if that is necessary?