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

Followup to Issue #2021 - Default config still causes errors with corrupted redis sessions #2809

Closed
laurin-erlacher-studyly opened this issue Feb 20, 2024 · 1 comment
Assignees
Labels

Comments

@laurin-erlacher-studyly

Why the followup is needed
The bug reported in Issue #2021 still persists with default configuration, and it is NOT clear how to configure spring-session to prevent the issue.

I suggest to alter the default config to prevent the bug, or at least highlight in the documentation how to properly prevent this issue.
(Also, i'd like to confirm that the sort-of-documented fix from the final post of Issue #2021 is actually the correct way to fix the issue.)

Original Bug

Sessions may come into a "partially existing state", which causes all requests trying to access such a session to fail, without possibility of recovering other than manual removal of the session from redis. In my case the "creationTime" attribute is missing.
The previous report was resolved by "Allowing Customizing Redis Session Mapper" and hinting at a solution, where a custom mapper is configured to delete such invalid/corrupted sessions. However, there is still no indication if this is the correct way of dealing with this issue, or if the "partially existing" sessions should be treated as a bug.

Expected behavior
The default configuration of spring session should not cause errors where an entire session and potentially an entire user account is "stuck" and unusable without manual intervention.

To Reproduce
To "corrupt" a session do e.g. HDEL spring:session:sessions:SOME_EXISTING_SESSION_ID creationTime in redis.
Then trigger loading of the session - a web request with that session should suffice.
I currently have no way to produce such sessions in the wild.

Sample
Not quite sure how to provide one, since the sample requires a redis database.

@laurin-erlacher-studyly laurin-erlacher-studyly added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Feb 20, 2024
@marcusdacoregio
Copy link
Contributor

Hi, @laurin-erlacher-studyly. Thanks for the report.

We cannot change the default behavior of the application in a minor version, that is one of the reasons why the default didn't change. Additionally, concurrency modifications does not happen in every application, and, if it happens, each business rule might have a different strategy to handle it.

The documentation gives an example of when you'd probably want to customize the mapper. Probably a more robust solution to the general problem of session deserialization would be provided by #529, if you'd like to see it implemented in the framework, please give a thumbs up to that issue.

I'll close this as there are ways to workaround the serialization problem and that we might consider #529 instead of some custom code on each session repository implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

No branches or pull requests

2 participants