Skip to content

Allow update of existing WebSession after 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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Allow update of existing WebSession after max sessions limit is reached #35013

wants to merge 2 commits into 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.

Fix: Allow session update when session ID already exists even if max …
de0e0c8 
…sessions limit is reached

Signed-off-by: Mohammad Saeed Nouri <msnsaeed71@gmail.com>
@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
DhruvTheDev1
DhruvTheDev1 reviewed Jun 9, 2025
View reviewed changes
Copy link
Contributor

@DhruvTheDev1 DhruvTheDev1 left a comment •
edited
Loading

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
Test case for Allow session update for existing session ID even if ma…
25e6642 
…x sessions limit is reached

Signed-off-by: Mohammad Saeed Nouri <msnsaeed71@gmail.com>
@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
@sbrannen sbrannen added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Jun 10, 2025
@sbrannen sbrannen changed the title Allow session update for existing session ID even if max sessions limit is reached Allow update of existing session after max sessions limit is reached Jun 10, 2025
@sbrannen sbrannen added this to the 6.2.8 milestone Jun 10, 2025
@sbrannen sbrannen changed the title Allow update of existing session after max sessions limit is reached Allow update of existing WebSession after max sessions limit is reached Jun 10, 2025
@sbrannen sbrannen mentioned this pull request Jun 10, 2025
Closed
@sbrannen sbrannen added the status: backported An issue that has been backported to maintenance branches label Jun 10, 2025
sbrannen added a commit that referenced this pull request Jun 10, 2025
@sbrannen
Polish contribution
4d2cc4a 
See gh-35013
@sbrannen sbrannen closed this in c04902f Jun 10, 2025
@sbrannen
Copy link
Member

This has been merged into 6.2.x and main in c04902f and slightly revised in 4d2cc4a.

This will also be backported to 6.1.x in conjunction with #35018.

Thanks for finding this bug and fixing it! 👍

sbrannen pushed a commit that referenced this pull request Jun 10, 2025
@sbrannen
Allow update of existing WebSession after max sessions limit is reached
cd44efa 
Previously, when saving a WebSession, 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 WebSession with the same ID already exists,
it will be updated rather than counted as a new session, thereby
preventing unnecessary session limit violations.

See gh-35013
Closes gh-35018

Signed-off-by: Mohammad Saeed Nouri <msnsaeed71@gmail.com>
(cherry picked from commit c04902f)
sbrannen added a commit that referenced this pull request Jun 10, 2025
@sbrannen
Polish contribution
8ecc553 
See gh-35013
See gh-35018

(cherry picked from commit 4d2cc4a)
@msnsaeed71
Copy link
Author

This has been merged into 6.2.x and main in c04902f and slightly revised in 4d2cc4a.

This will also be backported to 6.1.x in conjunction with #35018.

Thanks for finding this bug and fixing it! 👍

Thank you very much for merging the code and for your kind words! I appreciate your careful review and the backport effort as well. Glad I could help fix the bug! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DhruvTheDev1 DhruvTheDev1 DhruvTheDev1 left review comments

Assignees

@sbrannen sbrannen

Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Milestone
6.2.8
Development

Successfully merging this pull request may close these issues.
4 participants
@msnsaeed71 @sbrannen @DhruvTheDev1 @spring-projects-issues