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

How much longer for XStream? #262

Open
NicholasWMRitchie opened this issue Aug 26, 2021 · 20 comments
Open

How much longer for XStream? #262

NicholasWMRitchie opened this issue Aug 26, 2021 · 20 comments
Assignees
Labels

Comments

@NicholasWMRitchie
Copy link

XStream is a critical part of my application's serialization procedures and has been a wonderful thing 👍 so I'm hesitant to ask such a downer of a question.

However, each subsequent version of Java seems to shut down more and more of the APIs on which XStream depends. I can't argue with the Java designer's desire to eliminate security flaws by eliminating problematic APIs, however, it does raise the question: How long can XStream survive? Are there alternative APIs that will permit re-instantiating objects while maintaining security? This can't only be a problem for XStream. What do other serialization APIs do?

I've been slowly implementing fixes as I've moved from JDK version to version. Each time, I've been able to find some work around using the XStream whitelist security API. However, I'm afraid that I might be at the end of the line.

I just upgraded from JDK 15 to JDK 16 and now serialization via XStream blows up spectacularly. The first error I get seems to be caused by this:

Unable to make field private final java.util.Comparator java.util.TreeMap.comparator accessible: module java.base does not "opens java.util" to unnamed module @2353b3e6

This would seem to be related to new restrictions implement in Java 16.

I'd like to know should I be considering alternative serialization schemes or will it be possible to address these problems within XStream?

@joehni joehni self-assigned this Aug 28, 2021
@joehni
Copy link
Member

joehni commented Aug 28, 2021

Well, XStream mimics Java serialization, but it does stuff that is not available in public API and it even violates the Java specification (by calling for an object the ctor of the unserializable parent only). My guess is in combination with the security flaws, that it will be cut off in the long run also.

What does it mean for XStream? Actually it can still use reflection for types in modules that can be opened to it. You will be able to use it for your own types. For any stuff in the java.base module XStream will have to provide custom converters - reflection will no longer work. However, in consequence XStream will no longer be able to serialize some types out of the box at all (e.g. XStream will no longer be able to marshal a synchronized collection, because it cannot detect, what actual collection type is wrapped). Users of XStream will definitely have to write more custom converters in future.

@jyoshimi
Copy link

jyoshimi commented Sep 7, 2021

Hi all, and Jörg in particular. I'm also a long time and very happy xstream user. I teach a course with software using xstream. Students with java 16 can't run it, I think for the reasons brought up by the op. For now it's working by having them go back to java 14 or earlier, so we're good.

Longer term, you say we "will definitely have to write more custom converters in the future". Am I right that if that is done for most of the major java classes, that we can keep using it in the future?

I just want to make sure I have a sense of the long game here. I really don't want to lose xstream! I love how neat and tidy my code is and also how nice and readable the xml I generate is.

Thanks for the tireless efforts Jörg and xstream team, much appreciated.

@joehni
Copy link
Member

joehni commented Sep 10, 2021

At least for Java 16 you can set --illegal-access=permit.

As long as a module is open to XStream you will still be able to do most of the types in it. Obviously this is no longer true for java.base - here we'll need more converters in future. However, not everything can be handled in future. Think about synchronized collections:

List<String> list = Collections.synchronizedList(new LinkedList<>());

XStream has no possibility to detect the real type of the wrapped collection, it can no longer handle this object graph out of the box. You will have to write/register a converter on your own. This might even be true for a singleton wrapper, depending on the access method of the wrapped collection.

Another problem area are final fields. When XStream no longer can write into these by reflection, it won't be able to handle the type properly. This includes any non-static inner class type.

@celcius112
Copy link

For Java 16 you can probably make it works with the option --illegal-access=permit. However this command line switch is removed starting in Java 17 (more info here), making XStream incompatible with the latest LTS 😢. By transitivity, Jenkins CI is also incompatible 😄.

@timja
Copy link

timja commented Sep 29, 2021

By transitivity, Jenkins CI is also incompatible 😄.

Well, you can override it pretty easily with --add-opens, the Jenkins docker image will add it automatically for you:

jenkins/jenkins:2.314-jdk17-preview

(preview is to gather early feedback as not much testing has been done on it yet)

https://hub.docker.com/layers/jenkins/jenkins/2.314-jdk17-preview/images/sha256-30a195e4260fa86986fd9f1d98ed7c2c463d30a8d7291cf1bf2a3c7854a63aa6?context=explore

@celcius112
Copy link

Thanks for the heads up, we will try it

@timja
Copy link

timja commented Sep 29, 2021

Thanks for the heads up, we will try it

https://github.com/jenkinsci/docker for any issues with the image, https://issues.jenkins.io/ component core if there's an issue with Jenkins, label it with java17-compatibility, feel free to tag me for visibility

@galderz
Copy link

galderz commented Oct 15, 2021

nexus-staging-maven-plugin, which depends on xstream, also having similar issues (see NEXUS-27902). I've added a comment there based on the hint above on using --add-opens but I've not tried it.

@kriegaex
Copy link

My project also has this problem on JDK 16+, despite XSTREAM.addPermission(AnyTypePermission.ANY); in my code:

Exception in thread "main" java.lang.ExceptionInInitializerError
	at com.thoughtworks.xstream.converters.collections.TreeMapConverter.unmarshal(TreeMapConverter.java:73)
	at com.thoughtworks.xstream.core.TreeUnmarshaller.convert(TreeUnmarshaller.java:72)
	at com.thoughtworks.xstream.core.AbstractReferenceUnmarshaller.convert(AbstractReferenceUnmarshaller.java:72)
	at com.thoughtworks.xstream.core.TreeUnmarshaller.convertAnother(TreeUnmarshaller.java:66)
	at com.thoughtworks.xstream.core.TreeUnmarshaller.convertAnother(TreeUnmarshaller.java:50)
	at com.thoughtworks.xstream.core.TreeUnmarshaller.start(TreeUnmarshaller.java:134)
	at com.thoughtworks.xstream.core.AbstractTreeMarshallingStrategy.unmarshal(AbstractTreeMarshallingStrategy.java:32)
	at com.thoughtworks.xstream.XStream.unmarshal(XStream.java:1391)
	at com.thoughtworks.xstream.XStream.unmarshal(XStream.java:1376)
	at com.thoughtworks.xstream.XStream.fromXML(XStream.java:1270)
	at de.scrum_master.galileo.Book.readConfig(Book.java:86)
	at de.scrum_master.galileo.Options.parse(Options.java:46)
	at de.scrum_master.galileo.OpenbookCleaner.processArgs(OpenbookCleaner.java:36)
	at de.scrum_master.galileo.OpenbookCleaner.main_aroundBody0(OpenbookCleaner.java:29)
	at de.scrum_master.galileo.OpenbookCleaner.main_aroundBody1$advice(OpenbookCleaner.java:12)
	at de.scrum_master.galileo.OpenbookCleaner.main(OpenbookCleaner.java:1)
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make field private final java.util.Comparator java.util.TreeMap.comparator accessible: module java.base does not "opens java.util" to unnamed module @77847718
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:357)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
	at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:177)
	at java.base/java.lang.reflect.Field.setAccessible(Field.java:171)
	at com.thoughtworks.xstream.core.util.Fields.locate(Fields.java:40)
	at com.thoughtworks.xstream.converters.collections.TreeMapConverter$Reflections.<clinit>(TreeMapConverter.java:135)
	... 16 more

The only workaround is to specify the JVM parameter --add-opens java.base/java.util=ALL-UNNAMED.

@kriegaex
Copy link

Related problems in Nexus Staging Maven Plugin:

@tdauth
Copy link

tdauth commented Jun 7, 2022

Hi, is there any solution to this issue? We are facing the same issue and would like to upgrade to Java 17.

@basil
Copy link
Contributor

basil commented Sep 14, 2022

The workaround is to add all the --add-opens directives from

<surefire.argline>--add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED --add-opens java.base/java.time=ALL-UNNAMED --add-opens java.base/java.time.chrono=ALL-UNNAMED --add-opens java.base/java.lang.invoke=ALL-UNNAMED --add-opens java.base/java.lang.ref=ALL-UNNAMED --add-opens java.base/java.lang.reflect=ALL-UNNAMED --add-opens java.base/java.text=ALL-UNNAMED --add-opens java.base/javax.security.auth.x500=ALL-UNNAMED --add-opens java.base/sun.util.calendar=ALL-UNNAMED --add-opens java.desktop/java.beans=ALL-UNNAMED --add-opens java.desktop/java.awt=ALL-UNNAMED --add-opens java.desktop/java.awt.font=ALL-UNNAMED --add-opens java.desktop/javax.swing=ALL-UNNAMED --add-opens java.desktop/javax.swing.border=ALL-UNNAMED --add-opens java.desktop/javax.swing.event=ALL-UNNAMED --add-opens java.desktop/javax.swing.table=ALL-UNNAMED --add-opens java.desktop/javax.swing.plaf.basic=ALL-UNNAMED --add-opens java.desktop/javax.swing.plaf.metal=ALL-UNNAMED --add-opens java.desktop/javax.imageio=ALL-UNNAMED --add-opens java.desktop/javax.imageio.spi=ALL-UNNAMED --add-opens java.desktop/sun.swing=ALL-UNNAMED --add-opens java.desktop/sun.swing.table=ALL-UNNAMED --add-opens java.xml/javax.xml.datatype=ALL-UNNAMED --add-opens java.xml/com.sun.xml.internal.stream=ALL-UNNAMED --add-opens java.xml/com.sun.org.apache.xerces.internal.parsers=ALL-UNNAMED --add-opens java.xml/com.sun.org.apache.xerces.internal.util=ALL-UNNAMED</surefire.argline>
to your Java invocation or to an Add-Opens: directive in MANIFEST.MF. In practice you will not need all of these, just a few depending on your usage. The following six cover 95% of the XStream core test suite, for example:

  • --add-opens java.base/java.lang=ALL-UNNAMED
  • --add-opens java.base/java.io=ALL-UNNAMED
  • --add-opens java.base/java.text=ALL-UNNAMED
  • --add-opens java.base/java.time=ALL-UNNAMED
  • --add-opens java.base/java.time.chrono=ALL-UNNAMED
  • --add-opens java.base/java.util=ALL-UNNAMED

In Jenkins core we are only using these three:

  • --add-opens java.base/java.lang=ALL-UNNAMED
  • --add-opens java.base/java.io=ALL-UNNAMED
  • --add-opens java.base/java.util=ALL-UNNAMED

For our use case, which generally does not involve marshalling and unmarshalling timestamps, this works well enough.

The classes in java.util.concurrent have been a problem for us, especially AtomicBoolean (tracked in #308). We are hesitant to add an --add-opens directive for java.util.concurrent because we would generally prefer to avoid serializing concurrent types, but the large number of existing usages of AtomicBoolean in Jenkins plugins makes us wish for native support in XStream (proposed in #309). We had a single usage of ConcurrentSkipListMap, which we rewrote to use an in-memory ConcurrentSkipListMap and a persistent HashMap in jenkinsci/workflow-cps-plugin#518. A number of our tests tried to serialize CountDownLatch, largely in error, and we have been fixing those by marking the field transient in those tests. I also found a Jenkins plugin that was trying to serialize a java.util.logging.Level field, which I worked around in the plugin itself by serializing a string representation instead.

@NicholasWMRitchie
Copy link
Author

I gave this suggestion a try and it works! The documentation for --add-opens is terrible and for Add-Opens: in the manifest file even worse. However, by following the link in basil's comment, I was able to identify that my program required only java.base/java.lang and java.base/java.util. I added them to the manifest file as

Add-Opens: java.base/java.lang java.base/java.util

Using =ALL-UNNAMED in the manifest didn't work but wasn't required.

All is good with the world. I'm able to use JRE 18.

@pkriens
Copy link

pkriens commented Oct 6, 2022

@NicholasWMRitchie where did you find any documentation about an Add-Opens manifest header?

@MrEasy
Copy link

MrEasy commented Oct 6, 2022

@pkriens

@NicholasWMRitchie where did you find any documentation about an Add-Opens manifest header?

Jumping-in: It is documented here: https://openjdk.org/jeps/261#Packaging:-Modular-JAR-files
But note: The manifest entries only can be used if you start via the JAR directly (i.e. java -jar jarfile.jar)

@NicholasWMRitchie
Copy link
Author

Thanks @MrEasy. Your suggestion to @pkriens for documentation is better than anything I could find. My application is a Swing desktop app and as @MrEasy notes, it is started with java(w) -jar jarfile.jar. I can't speak to other situations.

@TWiStErRob
Copy link

I'm baffled this is not linked with #101 yet...

@Groostav
Copy link

When I started using xstream in 2014 it was a godsend to my project. We have some fairly complex data types (trees DAGs with visitors) that needed to be serialized. I struggled to design a DAL and some method of kind've straightening our data before serialization, but it was error prone and I knew that my solution would be laden with junior developers making a change on the model and then discovering things going missing when we reloaded from a persisted object.

Then I found xstream: I could simply point xstream at a node on the graph the the whole thing (preserving references!) was serialized to XML. after a little cleanup (we use a lot of immutable data) the XML that xstream was producing was clean and fast enough for our purposes. I was very happy; this model has worked well for the last 10 years of development.

Then of course java 8 to java 9, then to java 11 that we started targeting, then to java 13 for reasons. Now I'm trying to move to java 17, and I'm seeing failures from xstream in all sorts of corner cases I hadnt considered. the simple fact is our codebase is now large and we have many entry points. Sometimes our app is run as a jar, sometimes its run after a formal installation, sometimes from an intelliJ play button. It is difficult to get all the necessary --add-opens in to every reasonable run configuration.

If anybody has a suggestion for a way to serialize fairly large object graphs that preserve their internal identity & value semantics, please tell me. I think asking code-generation to replace reflection is too much, but with the way the reflection APIs are going, i think it may be the only reasonable strategy.

@cordisvictor
Copy link

cordisvictor commented Mar 4, 2024

If anybody has a suggestion for a way to serialize fairly large object graphs that preserve their internal identity & value semantics, please tell me. I think asking code-generation to replace reflection is too much, but with the way the reflection APIs are going, i think it may be the only reasonable strategy.

@Groostav you could try https://github.com/cordisvictor/easyml-lib
@jyoshimi @NicholasWMRitchie @TWiStErRob maybe it's a fit for you guys too

@gc-1111
Copy link

gc-1111 commented Jul 30, 2024

This old version of xstream works with JDK 17.

<dependency>
  <groupId>com.thoughtworks.xstream</groupId>
  <artifactId>xstream</artifactId>
  <version>1.4.20</version>
</dependency>

I am going to try to find the latest version that still works with jdk17.

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