Skip to content

Allow session update for existing session ID even if max sessions limit is reached #35013

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

msnsaeed71
Copy link

Previously, when saving a session, the system did not check whether the session ID already existed. As a result, even if the session being saved was an update to an existing one, it was incorrectly treated as a new session, and a "maximum sessions exceeded" error was triggered.

This fix ensures that if a session with the same ID already exists, it will be updated rather than counted as a new session, thereby preventing unnecessary session limit violations.

…sessions limit is reached

Signed-off-by: Mohammad Saeed Nouri <[email protected]>
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 8, 2025
Copy link
Contributor

@DhruvTheDev1 DhruvTheDev1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR addresses a important logic in session management.

  • The updated condition makes sure that only truly new sessions are blocked when the maximum limit is reached.
  • If a session with the same ID already exists, it’s treated as an update, not a new session, and is allowed.
  • The fix is minimal, targeted, and clearly improves correctness.

Solid improvement!

@sbrannen
Copy link
Member

sbrannen commented Jun 9, 2025

In order to process this PR, we will need a test to verify whether the proposed change is necessary.

Thus, please introduce a test that fails before the change and passes after the change.

Thanks

@sbrannen sbrannen added status: waiting-for-feedback We need additional information before we can continue in: web Issues in web modules (web, webmvc, webflux, websocket) labels Jun 9, 2025
@sbrannen sbrannen changed the title Fix: Allow session update when session ID already exists even if max … Allow session update for existing session ID even if max sessions limit is reached Jun 9, 2025
…x sessions limit is reached

Signed-off-by: Mohammad Saeed Nouri <[email protected]>
@msnsaeed71
Copy link
Author

This PR addresses a important logic in session management.

  • The updated condition makes sure that only truly new sessions are blocked when the maximum limit is reached.
  • If a session with the same ID already exists, it’s treated as an update, not a new session, and is allowed.
  • The fix is minimal, targeted, and clearly improves correctness.

Solid improvement!

Thank you for the thoughtful review and kind words 🙏
I'm glad the logic refinement is clear and appreciated.

Appreciate your feedback and support!

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jun 9, 2025
@msnsaeed71
Copy link
Author

In order to process this PR, we will need a test to verify whether the proposed change is necessary.

Thus, please introduce a test that fails before the change and passes after the change.

Thanks

This PR adds a test to confirm that updating an existing session (with the same session ID) is still allowed even when the maximum number of sessions has been reached.
Thanks for the guidance. 🙏

@sbrannen sbrannen self-assigned this Jun 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged or decided on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants