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

Session persistence broken from 9.4.13+ #3597

Closed
stephenc opened this issue Apr 26, 2019 · 4 comments
Closed

Session persistence broken from 9.4.13+ #3597

stephenc opened this issue Apr 26, 2019 · 4 comments
Assignees

Comments

@stephenc
Copy link

stephenc commented Apr 26, 2019

Background

#2964 introduced switchable classloaders for session data.

I understand the intent of such switchable classloaders, but the simple switching mechanism is flawed and potentially breaks lots of web applications (and certainly breaks a lot of frameworks when running jetty in embedded mode or when installing the framework as a module)

Bug

The basic flaw in the current implementation is in https://github.com/eclipse/jetty.project/blob/29b960551f59d3c455fabdd657b207fc5898fb4c/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java#L74-L95

Basically, the switch of the classloader happens based on the classloader of the session value.

Where this can break web applications is for example, if I store an ArrayList<MyClass> in the session. Because only the top reference's classloader is checked, this will be serialized with isServerLoader == true because java.util.ArrayList always comes from the server classloader. Then when deserializing, the server classloader will be used, and as that doesn't have the MyClass definition (it's only in the webapp classloader) the session will fail to deserialize and be marked as invalid... or worse, in embedded mode the MyClass definition will be loaded from the server classloader and then the web application will get a ClassCastException when they try to access the session state.

Workaround

Use Jetty 9.4.12.v20180830

Impact

This would seem to be a rather severe regression as it has the capacity to break user's web application session serialization.

stephenc added a commit to stephenc/jetty.project that referenced this issue Apr 26, 2019
* Hard to get a test case that works in a unit test because
  `ClassLoadingObjectInputStream` falls back to `ObjectInputStream` when
  a class cannot be resolved, and as the tests are always running
  from a classloader that includes the test resources, you discover
  the test resources always as an artifact of running as a unit test.

* With this change the regression detected in writing a Jetty plugin for
  Apache OpenWebBeans is resolved.
stephenc added a commit to stephenc/jetty.project that referenced this issue Apr 26, 2019
* Hard to get a test case that works in a unit test because
  `ClassLoadingObjectInputStream` falls back to `ObjectInputStream` when
  a class cannot be resolved, and as the tests are always running
  from a classloader that includes the test resources, you discover
  the test resources always as an artifact of running as a unit test.

* With this change the regression detected in writing a Jetty plugin for
  Apache OpenWebBeans is resolved.

Signed-off-by: Stephen Connolly <stephen.alan.connolly@gmail.com>
@joakime
Copy link
Contributor

joakime commented Apr 26, 2019

PR #3598 opened

@joakime
Copy link
Contributor

joakime commented Apr 26, 2019

Do you have a demo webapp that produces this issue?

@stephenc
Copy link
Author

stephenc commented Apr 26, 2019

I have some unit tests in Apache OpenWebBeans!

If you change the jetty.version property at https://github.com/apache/openwebbeans/pull/14/files#diff-600376dffeb79835ede4a0b285078036R74 to anything newer than 9.4.12 then this unit test https://github.com/apache/openwebbeans/pull/14/files#diff-ffb9e65df6fd13bd90699a9a596fe45aR50 will start to fail.

It took @struberg and I a while to diagnose but the root cause is this:

  • When running Jetty in embedded mode (or with OWB deployed as a jetty module rather than bundled in the .WAR) then OWB's classes will come from the server classloader.
  • OWB stores its SessionContext class in the Session so that it can maintain the @SessionScoped beans
  • When the session is persisted, Jetty's SessionData sees the value as having a classloader that is not the webapp classloader, but either the system or parent classloader (depending on embedded or module deployment and the direction of the wind) so it records the class as being isServerLoader
  • When deserializing in embedded mode (like in a test) - because the classes are also available from the embedded classloader the Bean Manager is able to bootstrap but we get a class cast exception because it bootstrapped the wrong context
  • When deserializing in Jetty module mode (or in embedded mode where you provide a WAR) the Bean Manager bootstraps an empty context or fails to deserialize the classes because they cannot be found.

janbartel added a commit that referenced this issue Apr 27, 2019
…with system classes.

Signed-off-by: Jan Bartel <janb@webtide.com>
janbartel added a commit that referenced this issue Apr 29, 2019
Signed-off-by: Jan Bartel <janb@webtide.com>
janbartel added a commit that referenced this issue Apr 29, 2019
…3602)

* Issue #3597 Fix session persistence classloading for deep structures with system classes.
lachlan-roberts pushed a commit to lachlan-roberts/jetty.project that referenced this issue May 1, 2019
Signed-off-by: Jan Bartel <janb@webtide.com>
@janbartel
Copy link
Contributor

Fixed in 9.4.18 and beyond.

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

No branches or pull requests

3 participants