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

Undertow - Session context lifecycle events should be observable from session scoped beans #46364

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

Conversation

manovotn
Copy link
Contributor

Fixes #46363

Contains a test with a reproducer and a draft of a fix.

I don't really like having to store the session like so but that's the only working solution I have found so far...

@manovotn manovotn requested a review from mkouba February 19, 2025 15:43
@manovotn manovotn changed the title Undertow session context fix Undertow - Session context lifecycle events should be observable from session scoped beans Feb 19, 2025
Arc.container().beanManager().getEvent().select(HttpSession.class, Initialized.Literal.SESSION).fire(se.getSession());
HttpSession createdSession = se.getSession();
CREATED_SESSION.set(createdSession);
Arc.container().beanManager().getEvent().select(HttpSession.class, Initialized.Literal.SESSION).fire(createdSession);
Copy link
Contributor

Choose a reason for hiding this comment

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

So we need a thread local because at this point the ServletRequestContext.requireCurrent().getServletRequest()).getSession(true) returns null or a different session?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A different session - with true it always creates a new one.
Plus if you try the same invocation with false (to see the session for which you just called session created callback), you will get null at this point 🤷
So we need some way to remember that we already created a session - at least that was my thinking.

Copy link
Contributor

Choose a reason for hiding this comment

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

A different session - with true it always creates a new one.

That's weird, it should only create a new one if it does not exist, right? And given that it's called from jakarta.servlet.http.HttpSessionListener.sessionCreated(HttpSessionEvent) I would expect the session to exist...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's weird, it should only create a new one if it does not exist, right?

That is pretty much my yesterday thinking in a nutshell :)
Somehow, it ends up creating new session anyway.
You can debug this all the way to io.undertow.servlet.spec.ServletContextImpl#getSession where undertow tries to retrieve existing session first but I didn't manage to understand why it doesn't find it or whether it is right or wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we can crack this with our knowledge of servlet/undertow...
I've sent an email question to @fl4via who should hopefully know more about Undertow and how it should behave it this case.

@manovotn manovotn force-pushed the undertowSessionContextFix branch from 11897f8 to 3db654a Compare February 20, 2025 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Undertow - Session context lifecycle events should be observable from session scoped beans
4 participants