Skip to content

Failed rename for deleted session is not ignored with Redisson #1743

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

Closed
eager opened this issue Dec 10, 2020 · 4 comments
Closed

Failed rename for deleted session is not ignored with Redisson #1743

eager opened this issue Dec 10, 2020 · 4 comments
Assignees
Labels
Milestone

Comments

@eager
Copy link

eager commented Dec 10, 2020

This is same underlying issue as #1270, with a small twist—the exception message has more detail.

Describe the bug
We’re using RedissonConnectionFactory with our RedisOperationsSessionRepository (we’re still on Spring Session 2.1.13). Redisson’s exception handling adds a little extra context to the exception message, so this check in handleErrNoSuchKeyError fails:

if (!"ERR no such key".equals(NestedExceptionUtils.getMostSpecificCause(ex).getMessage())) {
	// NestedExceptionUtils.getMostSpecificCause(ex).getMessage() →
	//   "ERR no such key. channel: [id: 0xec125091, L:/100.96.17.194:46330 - R:redis/100.67.2.163:6379] command: (RENAME), params: [[115, 112, 114, 105, 110, 103, 58, 115, 101, 115, ...], [115, 112, 114, 105, 110, 103, 58, 115, 101, 115, ...]]"

	throw ex;
}

and the exception bubbles up.

To Reproduce
Make concurrent requests with the same session ID, which both result in changing the session ID—typically via session fixation prevention.

Expected behavior
The session repository gracefully handle concurrent requests that change the session ID.

@eager eager added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Dec 10, 2020
@Blockost
Copy link

Blockost commented May 30, 2022

Hi @eager ! We just encountered this same issue on our production environment. Were you able to find a workaround ? I wonder if it's safe to catch (and discard) it on our end until this is handled properly by spring-session.

@eager
Copy link
Author

eager commented Jun 4, 2022

@Blockost unfortunately, it's been a couple of years since I worked on the project that hit this issue, but I don't think we had a good workaround.

@Blockost
Copy link

Blockost commented Jun 4, 2022

Thanks @eager for you answer, we'll try to handle this on our end.

@marcusdacoregio marcusdacoregio added in: redis and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 6, 2023
@marcusdacoregio marcusdacoregio self-assigned this Jun 6, 2023
@marcusdacoregio marcusdacoregio modified the milestones: 2.7, 2.7.2 Jun 6, 2023
@marcusdacoregio marcusdacoregio added type: enhancement A general enhancement and removed type: bug A general bug labels Jun 6, 2023
@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Jun 6, 2023

Hi folks, I think we should consider providing some strategy to handle the scenario where a concurrent modification might happen.

However, I am not discarding changing the condition to use startsWith instead of equals. Is anyone else aware if there is another Redis client that might change the exception message?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants