IllegalStateException when forwarding to jsp with new session #4156
Assignees
Labels
Projects
Comments
|
How is your Session Management / Handling setup in Jetty? |
|
This is a standalone aka "jetty-home" setup with no customized session handling. |
|
I have been able to replicate with demo project at https://github.com/jetty-project/session-jsp-issue Full stacktrace ... |
joakime
added a commit
that referenced
this issue
Oct 4, 2019
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
|
A unit test that replicates the issue is now present in branch I'll give it a go on fixing this, but ultimately this is in @janbartel's wheelhouse. |
|
The problem appears to be that with the refactor we are now always doing a context level dispatch for forward. The SessionHandler is calling doScope again and nulling out the existing session... then for some reason when it looks for an existing session, it is finding a different one??? Perhaps we need to make SessionHandler.doScope a noop if we are already in the same session scope? But we also need to understand why it is not finding the same session? |
|
Ah the problem is that because the existing session has been invalidated and a new session created before the forward, the new session array inside the request now has 2 sessions for the context - one invalid. But the getSession(SessionHandler handler) method doesn't check for validity and just returns the first one. So simple fix is probably to just check for validity in that method. That should resolve the bug we just added. But a better fix may be to look at why we are re-initializing the session for a dispatch to the same context. |
gregw
added a commit
that referenced
this issue
Oct 4, 2019
+ check validity of sessions in getSession(SessionHandler) + do not replace session in doScope if SessionHandler is the same. Signed-off-by: Greg Wilkins <gregw@webtide.com>
|
@janbartel I've pushed a possible fix in 8b08b0f, but I think we need to walk through it again just in case I've missed something |
|
Opened PR #4159 |
gregw
added a commit
that referenced
this issue
Oct 14, 2019
* Issue #4156 - Adding test to replicate issue Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com> * Issue #4156 Session already in cache + check validity of sessions in getSession(SessionHandler) + do not replace session in doScope if SessionHandler is the same. Signed-off-by: Greg Wilkins <gregw@webtide.com>
|
We keep getting this error also in 9.4.22: java.lang.NullPointerException |
|
@ofrias include the complete stacktrace please. |
|
Of course, this is an example of the full stack trace: |
|
Please post the log output - I'm looking for a warning message just before this stack trace. Please also describe the sequence of what is happening - I'm guessing from the stack trace that you send a request that results in a forward to the welcome servlet, but I can't tell if there is already supposed to be a session or not. Please also provide your welcome file set up and servlet mapping set up. In fact, it would be ideal if you could provide a small reproduction. |
|
Hello. We don't know the exact sequence because these are logs that we get randomly in production, we don't know how to reproduce the issue. I think it is not related to the welcome servlet because it happens also with other servlets. This is another example log: I think this is the warning that you were referring to: And this is the error: |
|
@ofrias are you absolutely positive that you're running 9.4.22? All of the tests that we put in for this bug are passing in the 9.4.22 release, and a test I put in yesterday that does the sequence I guessed your code is doing also passed, so if your server logs confirm you're running 9.4.22, then we will need your help to pin down exactly what is happening. I suggest you turn on debug logging for sessions (-Dorg.eclipse.jetty.server.session.LEVEL=DEBUG) and post the log. Also start thinking about a small repro webapp/unit test. |
|
The line numbers on his stacktrace ... ... correspond to Jetty 9.4.22 release Example: if we use 9.4.21 Request.java on line 1608 we get ... |
|
@ofrias can you also please report what your session configuration is? Session time-out value etc etc. |
|
This is our session-config: |
|
@ofrias can you attach jetty log output that contains the error please? |
|
I attach Jetty log. |
|
@ofrias sorry, I meant a log that shows the error with debug logging for sessions turned on as I commented earlier. I need to see the lifecycle of these errored sessions. PS you might want to rationalize the jars you have in web-inf/lib: seems like there are multiple jars with the same classes, hence all the warnings from the annotation scanner. |
|
@janbartel Which is the configuration that we should use to enable session logging? |
|
I'm not near a computer so this will have to be brief. Please look back to my comments 5 days ago, it contains the session debug log command line arguments to use. |
|
@ofrias to enable debug logging for sessions when using the jetty stderr logger, set this system property: org.eclipse.jetty.server.session.LEVEL=DEBUG. Either put it on the command line, or if you have a jetty-logging.properties file you can define it in there instead. If using a different logging system, ensure that the package org.eclipse.jetty.server.session is set to the most verbose logging level for that system. |
|
I attach a new log now with session logging enabled. |
|
@ofrias thanks, that is very helpful. Can you please post more of the log? I need to see the full lifecycle for session id bran1mhhe4al4weg9182u738ifhgjy6833. In particular, I want to see where it was created, accessed and if it timed out. Can you also please tell me:
|
|
Hello. Thanks for your help. I include the session logs for bran server and also for arya server. jetty-sessions-bran.zip Answers follow:
This should not happen because we use haproxy load balancer with source IP balancing.
Posted.
We balance based on source IP so the same session should always go to the same server if it is available.
I don't know.
Yes.
This depends on the case, sometimes we add an attribute and sometimes not.
We are not setting it, so it should have its default value.
We are not setting it, so it should have its default value. |
|
@ofrias thanks for those docs and the info. I believe I've worked out what the problem could be, and I'm working on a fix. |
janbartel
added a commit
that referenced
this issue
Nov 13, 2019
… loading. Also cleanup of unneeded code/comments in tests. Signed-off-by: Jan Bartel <janb@webtide.com>
|
Created PR #4304 |
janbartel
added a commit
that referenced
this issue
Nov 13, 2019
Signed-off-by: Jan Bartel <janb@webtide.com>
janbartel
added a commit
that referenced
this issue
Nov 14, 2019
Signed-off-by: Jan Bartel <janb@webtide.com>
janbartel
added a commit
that referenced
this issue
Nov 17, 2019
|
@ofrias I've committed a fix to the jetty-9.4.x branch. Can you build and test with in in your environment? We'd like to make it part of the soon-to-be-released 9.4.23 version, but I'd like some level of assurance that it's been tested in the field first, if possible. |
|
Thanks a lot for your help. Unfortunately we cannot use a development build in production so I am afraid we cannot test it on our side yet. We could test it as soon as 9.4.23 is released (or even with the release candidate if available). |
|
@ofrias 9.4.23 is now released on maven central http://central.maven.org/maven2/org/eclipse/jetty/jetty-distribution/9.4.23.v20191118/ |
|
We have been running 9.4.23 in production for more than 24 hours, and we haven't seen any of these errors. Thanks a lot for your help! |
|
@ofrias thanks for that info. I'll close this issue as resolved. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We encountered a regression when upgrading from 9.4.20 to 9.4.21 in the session handling code.
When a servlet creates a new HttpSession and then forwards to another servlet/jsp that tries to access this session the AbstractSessionCache throws a IllegalStateException which leads to a NPE in SessionHandler.getExtendedId().
Simple test case simulating a login workflow:
and
Did not happen with 9.4.20 so maybe a side effect of #3947
The text was updated successfully, but these errors were encountered: