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

WARNING: An illegal reflective access operation has occurred #101

Open
DJSmy opened this issue Oct 24, 2017 · 87 comments
Open

WARNING: An illegal reflective access operation has occurred #101

DJSmy opened this issue Oct 24, 2017 · 87 comments
Assignees

Comments

@DJSmy
Copy link

DJSmy commented Oct 24, 2017

F:\Program Files\Landscape Editor 2D>java -cp lib/xstream.jar;lib/xpp3.jar;mapeditor.jar com.GUI
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.thoughtworks.xstream.core.util.Fields (file:/F:/Program%20Files/Landscape%20Editor%202D/lib/xstream.jar) to field java.util.Properties.defaults
WARNING: Please consider reporting this to the maintainers of com.thoughtworks.xstream.core.util.Fields
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

@joehni
Copy link
Member

joehni commented Oct 24, 2017

Yes, XStream can either serialize only half of the object or avoid the illegal access.

@AlanBateman
Copy link

AlanBateman commented Nov 4, 2017

The ReflectionFactory API was updated in JDK 8 and JDK 9 to help custom serialization libraries work with modules that want to encapsulate their internals. Look for the methods that return a method handle to the common readXXX/writeXXX methods.

@joehni
Copy link
Member

joehni commented Nov 17, 2017

@alan: So, the application can open its class types ... that will not help to access the default properties of a Properties instance, right?

@AlanBateman
Copy link

@joejni - Properties does not define a method to return the defaults in bulk. If there is a strong case to add such a method then it should be brought to core-libs-dev for discussion.

@DJSmy
Copy link
Author

DJSmy commented Nov 28, 2017

Tell me, what is the work around for this? With the release of Java 9, is XStreem going to become an invalid library?

@joehni
Copy link
Member

joehni commented Nov 28, 2017

The current workaround is to permit illegal access.

You will always be able to serialize your own classes, since it is up to you to open that module for XStream. In case of 3rd party classes this might be different and you will have to write your own converters in future instead of using XStream's reflection-based converters assuming you are able to implement those converters so, that you can recreate the object.

For some types in the JDK this might be no longer possible or only with limitations (e.g. currently no default entries for Property objects).

This reflects at least my current knowledge of the restriction with the new module system.

@Katharsas
Copy link

Katharsas commented Mar 28, 2018

I want to refactor my code, so that XStream does not need to use reflection as much. I get
Illegal reflective access by com.thoughtworks.xstream.converters.collections.TreeMapConverter.

I guess it's caused by reflection-based deserialization of Java collections like HashSet, could this be right (i don't think i serialize any TreeMaps directly)? Is there a way to exatcly determine the class that i need to write custom converters for, or to log where exactly xstream uses such "illegal" reflection?

@joehni
Copy link
Member

joehni commented Mar 31, 2018

You may overwrite the setupConverters method and register only the converters you're going to use. Note, there's no problem using reflection-based converters for your own classes, if you open your module for XStream.

mokun added a commit to mars-sim/mars-sim that referenced this issue Jun 8, 2018
r4282
2018-06-08

Note 1: the following warning shows up when starting mars-sim :
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by
com.thoughtworks.xstream.core.util.Fields
(file:/C:/Users/mk/.m2/repository/com/thoughtworks/xstream/xstream/1.4.9/xstream-1.4.9.jar)
to field java.util.TreeMap.comparator

See issue x-stream/xstream#101

Note 2: weblaf has resolved a java 9 compatibility issue due to the
exception by "AA_TEXT_PROPERTY_KEY" in 1.2.9-SNAPSHOT. However,
the xtream library is still using reflection that's deemed "illegal"
in Java 9.

See issue mgarin/weblaf#452

# CHANGES
1. Revert back weblaf-ui and weblaf-core to 1.2.9-SNAPSHOT for Java 9
   compatibility.

2. Revise class relating to sponsors' mission objectives. Will code in
   the use of these objectives in future.
@joehni joehni mentioned this issue Jun 13, 2018
@ulfjack
Copy link

ulfjack commented Jul 5, 2018

I think this is probably what you meant:

XStream xstream = new XStream(new StaxDriver()) {
      @Override
      protected void setupConverters() {
      }
    };
    xstream.registerConverter(new ReflectionConverter(xstream.getMapper(), xstream.getReflectionProvider()), XStream.PRIORITY_VERY_LOW);
    xstream.registerConverter(new IntConverter(), XStream.PRIORITY_NORMAL);
    xstream.registerConverter(new StringConverter(), XStream.PRIORITY_NORMAL);
    xstream.registerConverter(new CollectionConverter(xstream.getMapper()), XStream.PRIORITY_NORMAL);

@GedMarc
Copy link

GedMarc commented Jul 7, 2018

XStream reads package org.xmlpull.v1 from both xmlpull and xpp3.min

Error:java: module xpp3.min reads package org.xmlpull.v1 from both xmlpull and xpp3.min
Error:java: module xstream reads package org.xmlpull.v1 from both xmlpull and xpp3.min
Error:java: module xmlpull reads package org.xmlpull.v1 from both xmlpull and xpp3.min

:'( Sad Panda

@davewichers
Copy link

I just ran into this same issue with Java 10. As an FYI, this is still considered a WARNING in Java 10 as well (java version "10.0.1" 2018-04-17), so no difference between Java 9 and 10, yet anyway.
[INFO] --- maven-war-plugin:2.2:war (default-war) @ benchmark ---
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.thoughtworks.xstream.core.util.Fields (file:toMy/.m2/repository/com/thoughtworks/xstream/xstream/1.3.1/xstream-1.3.1.jar) to field java.util.Properties.defaults
WARNING: Please consider reporting this to the maintainers of com.thoughtworks.xstream.core.util.Fields
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

@khmarbaise
Copy link

khmarbaise commented Jul 21, 2018

@davewichers You should use a more recent version of maven-war-plugin cause this version you are using is about six years old...The most recent version 3.2.2 uses the most recent version of xstream 1.4.10 ....

@handsimeboy
Copy link

now,i have got the same problem when i use "mvn install",and it says"illegal reflective access by com.thoughtworks.xsteram.core.util.fields to field java.util.properties.defaults". it's very similar to your problem, so how do you solve this problem?i didn't get it from your talking....thanks

@handsimeboy
Copy link

i should tell you first that jdk10 and tomcat8.5 were used in my pc.

@gschrader
Copy link

I'm getting the same split package issue that @GedMarc is above, are people using --patch-module to address this?

@martinm1000
Copy link

@gschrader

What worked for me was to only use xstream without any other of its dependencies, which by luck skipped those problems. Serialization might be slower but that's not a problem for me.

XStream x = new XStream(new DomDriver());
<!-- https://x-stream.github.io -->
            <dependency>
                <groupId>com.thoughtworks.xstream</groupId>
                <artifactId>xstream</artifactId>
                <version>1.4.10</version>
                <exclusions>
                  <exclusion>
                    <groupId>xpp3</groupId>
                    <artifactId>xpp3_min</artifactId>
                  </exclusion>
                  <exclusion>
                    <groupId>xmlpull</groupId>
                    <artifactId>xmlpull</artifactId>
                  </exclusion>
                </exclusions>
            </dependency>

@gschrader
Copy link

Thanks @martinm1000 I will try that as well since performance isn't a concern

@joehni
Copy link
Member

joehni commented Sep 27, 2018

XStream is not responsible for problems in depending libraries. If you use the Xpp3Driver directly, you can omit exclude xmlpull alone.

@aaime
Copy link

aaime commented Oct 24, 2018

I've tried to follow @joehni suggestion and override the setupConverters method.
Ended up having some small difficulties with the approach:

  • The InternalBlackList seems something "important", but it's not visible to subclasses, I had to copy over its code in my XStream subclass
  • The registerConverterDynamically is not visible, again, had to make a copy of it
  • The TreeMapConverter and TreeSetConverter use some reflection that helps with performance, but it's not strictly required, I ended up copying over the class contents for tree and set converter and removing the optimizations. It may be nice to have that split in base class and optimized subclass, so that JDK 11 users do not have to do this operation?
  • The collection converter does not handle automatically a few type of private classes, such as java.util.Arrays$ArrayList, java.util.Collections$UnmodifiableList, java.util.Collections$UnmodifiableSet, that should be easy to marshal as common lists without having to do deep reflection (which causes warnings). This is likely a bit is specific, as I don't care them to come back in their original wrapped class, but in case you're interested, here

@joehni if you agree with some of the above changes I could try to make a PR (time permitting).

@joehni
Copy link
Member

joehni commented Oct 24, 2018

Hi,

let's make some comments about your topics:

  • the approach to overload setupConverters is used for your optimized scenario, i.e. when you know which converters you're going to use
  • the InternalBlackList is a bad hack for 1.4.x that does not exists in 1.5, therefore it is not part of any API and it is only used if you stubbornly refuse to initialize the security framework
  • since you know, what converters you need, there's no need to register them dynamically
  • the reflection used for the maps and sets is essential and not only an optimization. It is required if you have circular dependencies between your objects and keep them in a hashed or sorted collection. In such a case you have to avoid absolutely that the hashCode or compare methods of those objects are called when you add them to the collection, because they might not have been completely initialized yet. This might break an application possibly in some very subtle ways (JIRA 184). A NPE might be quite obvious, but it might also happen that the hash code of an object changes after it has been added to a hashed which will compromise the functionality of that collection. Same applies to sorted collections, where the elements are no longer sorted after deserialization.
  • those special collections have special functionality. XStream aims for a 1:1 marshalling

See, it is not, that I don't know what to do to make the compiler happy, I care more for the user's applications.

@mikehearn
Copy link

@AlanBateman I got curious about your comments on ReflectionFactory. Nobody has responded to them, perhaps because it's not obvious what the JDK maintainers have in mind for libraries like XStream. As far as I can tell ReflectionFactory is a sort of pseudo-API that is not actually supported by the JDK, sits in the sun package, but also isn't really fully internal? That's rather confusing, which might be why your comments about it have been ignored. It does not appear in the JavaDocs for any recent Java release, and sits in the jdk.unsupported module. There's no obvious way to learn about its existence, and it's not clear for how long this API will stick around.

The JavaDoc says: "ReflectionFactory supports custom serialization. Its methods support the creation of uninitialized objects, invoking serialization private methods for readObject, writeObject, readResolve, and writeReplace."

Can you clarify whether it's acceptable for libraries to use this API and whether it'll stick around for the long run?

It's not clear that would help resolve these issues, because XStream is indeed using internal fields of collections, apparently to try and avoid problems with constructing cyclic object graphs involving collections where there's no ability to correctly pre-calculate the hashCode for everything ahead of time (I can't say I fully understand this problem, as the object graphs must have been built originally without modifying hashCode post-insertion). But then again built-in Java object serialisation presumably supports such object graphs, so perhaps it is sufficient?

@mikehearn
Copy link

Looking at the code in more detail, it's not entirely obvious why reflective access to the comparator field is still required on modern Javas. Rationale:

At some point the JDK added an optimization where the comparator isn't called during putAll(). XStream knows about this and tests the JDK to see if it does skip calling compareTo() when using putAll(). When on such a JDK special codepaths are taken.

In TreeMapConverter marshalling is straightforward and doesn't seem to require any special access, because comparator has a getter.

When deserialising the code paths get quite confusing. It looks like this has evolved over time and maybe can be simplified in ways that resolve at least one of the problems. Firstly, we either create a treemap before deserializing the comparator, if the comparator field was found via reflection, or alternatively it's created after the comparator is loaded if not. Then we populate the tree by first deserializing into a PresortedMap, and then using this code:

if (JVM.hasOptimizedTreeMapPutAll()) {
                if (comparator != null && Reflections.comparatorField != null) {
                    Reflections.comparatorField.set(result, comparator);
                }
                typedResult.putAll(sortedMap); // internal optimization will not call comparator
            }

On modern JVMs with Jigsaw this path will always be taken. We forcibly set the comparator internal field, and then do an operation that doesn't actually use the comparator at all. If the comparator isn't used at all then calling hashCode() on partly constructed objects doesn't seem like it will occur. Yet we could have already constructed the TreeMap with the comparator via passing it in using the constructor, and this does happen ... but only when the comparator field doesn't exist.

Due to this optimisation it seems that objects that are only partly deserialized can be added to a TreeMap even when their hashCode may change post-adding, resolving the concern @joehni discusses above. And at least from a quick look at the code, it doesn't seem like this requires accessing private fields.

@jansohn
Copy link

jansohn commented Nov 4, 2020

Hi all,

I also encounter this problem:
I use the latest version of Jenkins.
Ubuntu 18.04.5 LTS
jenkins 2.262
openjdk 11.0.8 2020-07-14
OpenJDK Runtime Environment (build 11.0.8+10-post-Ubuntu-0ubuntu118.04.1)
OpenJDK 64-Bit Server VM (build 11.0.8+10-post-Ubuntu-0ubuntu118.04.1, mixed mode, sharing)

2020-10-18 12:25:59.890+0000 [id=1] INFO o.e.j.s.s.DefaultSessionIdManager#doStart: No SessionScavenger set, using defaults
2020-10-18 12:25:59.891+0000 [id=1] INFO o.e.j.server.session.HouseKeeper#startScavenging: node0 Scavenging every 660000ms
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.thoughtworks.xstream.core.util.Fields (file:/var/cache/jenkins/war/WEB-INF/lib/xstream-1.4.7-jenkins-1.jar) to field java.util.TreeMap.comparator
WARNING: Please consider reporting this to the maintainers of com.thoughtworks.xstream.core.util.Fields
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

this is about to get fixed: see jenkinsci/jenkins#4944

@Renanh
Copy link

Renanh commented Nov 5, 2020

I had the same problem, so I updated the 1.4.11.1 dependency to version 1.4.13 and solved the problem.

Java 11
Spring boot : 2.3.5.RELEASE
dependency on axon framework 4.4.3 using xstream 1.4.11.1 version
Soluction:

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

jglick added a commit to jenkinsci/jep that referenced this issue Nov 6, 2020
@RandyAvis
Copy link

RandyAvis commented Dec 18, 2020

We fixed the problem by including the latest maven-war-plugin. In the pom:

<build>
	    <plugins>
	      <plugin>
	        <groupId>org.apache.maven.plugins</groupId>
	        <artifactId>maven-war-plugin</artifactId>
	        <version>3.3.1</version>
	        <configuration>
	          <archive>
	            <manifest>
	              <addClasspath>true</addClasspath>
	            </manifest>
	          </archive>
	        </configuration>
	      </plugin>
	    </plugins>
<build>

mokun added a commit to mars-sim/mars-sim that referenced this issue Dec 19, 2020
r5490
2020-12-18

## ISSUE
1. See x-stream/xstream#101 (comment)

## NEW
1. Add maven-war-plugin in pom.xml in an attempt to address
   illegal reflective access operation at the start when running
   mars-sim in Eclipse IDE.
@kennymacleod
Copy link

kennymacleod commented Aug 20, 2021

It's worth nothing hat as of Java 17, the option to ignore the illegal-access warnings will no longer be there. Java 17 will always deny those accesses, unless you start slapping --add-opens clauses everywhere you need them.

Java 16 is a shot across the bows, in that it denies the illegal accesses by default, whereas Javas 9-15 would by default log a warning on the first access and then continue as before.

@cStuartHardwick
Copy link

After wasting a week learning that there is simply no hope of ever getting my app to serialize using Moxy or Xstream, I'm ready to give up and just write code to directly emit xml. Can anyone suggest a better alternative?

I inherited this code that worked perfectly using Castor, but Castor depended on methods not part of the official public Java API that are no longer available, and the amount of work needed to refactor the app to work with Moxy is beyond all hope since it doesn't support non-static inner classes and even when these are refactored out, it just throws one cryptic error after another. XStream seems similar, and throws the illegal-access right out of the chute. Honestly, I wish the original developer of this app had just written their own code to serialize the objects. Dependence on these Open Source non-mainstream libraries seems hopeless.

Am I wrong?

@melloware
Copy link

@cStuartHardwick I have been using XStream successfully since 2002. Its a great library and serves the purpose perfectly. A developer 20 years ago when it was still JDK3 can't know that the JDK16 is going to completely remove a feature it counts on. I get your bind, but Xstream is not the only library in this bind. Kryo, XStream, Moxy, etc are all facing these same issues.

I personally think what you will see is a LOT of companies not move to JDK17 because of this and have so much legacy software that they can't upgrade they will just stay on JDK11 for years to come.

@GedMarc
Copy link

GedMarc commented Aug 22, 2021

XStream was great for JDK 1.3->1.7, but lets be fair guys, every single other framework caught up.
There is no more illegal access, and that is a wonderful thing!

Properties over Fields - it really is as simple as that. XStream has a strict field first policy, hence... issues.

I must say though, I've seen the exact opposite hey @melloware / I'm hiring and the great ones are all going why JDK 8 it's legacy due to it's EOL date :)
Then you mention your JDK 11/16/17 projects, and the first major question is "Is it modular, Is it native runtime, Is it a service"

Banks I think, the really slow moving decision chain, will get really stuck if they don't embrace, I see a massive movement towards modularity now that people are getting comfortable and are understanding it better

@melloware
Copy link

@GedMarc you might be right but I think its not just banks its a lot of government too. They don't have budgets to rewrite software that works fine just because a JDK upgrade so I think JDK11 LTS is what is going to be the long time lived JDK. Just my thought. It will take years before tons of software gets rewritten. I have a client that just moved to JDK8 2 years ago. 😄

@joehni
Copy link
Member

joehni commented Aug 22, 2021

Properties over Fields - it really is as simple as that. XStream has a strict field first policy, hence... issues.

Nobody stops you from registering the JavaBeanConverter as default for any type you were not willing to handle in an own converter. Yes, you will have to say goodby to the expectation, that XStream can handle any arbitrary type in the Java runtime out of the box. However, if you configure it to handle your own types (as you have to configure other frameworks as well), it will work.

I must say though, I've seen the exact opposite hey @melloware / I'm hiring and the great ones are all going why JDK 8 it's legacy due to it's EOL date :)
Then you mention your JDK 11/16/17 projects, and the first major question is "Is it modular, Is it native runtime, Is it a service"

Banks I think, the really slow moving decision chain, will get really stuck if they don't embrace, I see a massive movement towards modularity now that people are getting comfortable and are understanding it better

All fine for new projects, but nobody is willing to invest a major amount of money just to get an application running on Java 17 that is running already for years without any problems. At first sight a customer gets no real benefit for all the money it takes to port such an application.

@cStuartHardwick
Copy link

I'm going to give Moxy a little more time to prove itself one way or the other, if only because its bindings file is a closer analog to the mapping file used by Castor and because that approach make more sense to me. If XStream only has good support for fields, that's a non-starter for me. I haven't exposed public fields in any language since....a long long time, and the original developers of my application didn't either.

What I'm trying to do right now, in fact, is use a utility function to copy all the properties from the mega-class I want to marshal into a interim class that has only lightweight, serializable properties, in hopes I can get Moxy to work with that. The reason being that the real object contains lots and lots and lots of code for manipulating all the data that Moxy chokes on, and even when I refactored to expose only the properties through an interface, Moxy still tries to internally process all the code in the real object which still exists behind the reference, and no-go.

Of course what I'm wondering is, if I'm going to the extra trouble of making a copy of the object just to make it serializeable, might I not be better just to modify that code to directly serialize it. But I haven't explored the use of factory classes yet, and I'm only a lightweight Java developer, so I may be letting frustration get the better of me.

Thanks all.

@GedMarc
Copy link

GedMarc commented Aug 22, 2021

@joehni it certainly started like that I'm surprised though especially in the digitalization of everything movement?, What I've found lately (and for my comment), has the security vulnerabilities push not started to enforce the movement for you yet? Certainly from our side, security alone granted the entire budget to get off JDK 8 :)

No one wants to get legally bound to oracle xD I think if it weren't for the latest global catastrophes 100% it would have gone for at least a decade more, but Auditors are getting very precise and pedantic, and all systems digitizing presents the enforcement of keeping with the time or not being compliant on any given front?
Wow that is off topic, but I'm genuinely interested

@manuelddahmen
Copy link

manuelddahmen commented Oct 12, 2021

Should I compile with:
maven - - add-opens com.thoughtworks.xstream.core.util=ALL-UNNAMED
?
I try with version 1.4.15 in my nexus publishing plugin dependency and it is OK with Java 11.

@TWiStErRob
Copy link

See also #262 for more info.

@TWiStErRob
Copy link

TWiStErRob commented Mar 12, 2023

I'm sorry but this thread is a mess so I didn't read it all, but I'd like to point out that I'm not using xstream directly, only through PowerMock, nor I am using AWT, but I'm still getting:

WARNING: Illegal reflective access by com.thoughtworks.xstream.core.util.Fields (file:/Z:/caches/gradle/caches/transforms-3/d13dc098b15d22c8431769140d116fe6/transformed/jetified-xstream-1.4.10.jar) to field java.awt
.font.TextAttribute.instanceMap
        at com.thoughtworks.xstream.core.util.Fields.locate(Fields.java:40)
        at com.thoughtworks.xstream.converters.reflection.AbstractAttributedCharacterIteratorAttributeConverter.readResolve(AbstractAttributedCharacterIteratorAttributeConverter.java:111)
        at com.thoughtworks.xstream.converters.reflection.AbstractAttributedCharacterIteratorAttributeConverter.<init>(AbstractAttributedCharacterIteratorAttributeConverter.java:65)
        at com.thoughtworks.xstream.converters.extended.TextAttributeConverter.<init>(TextAttributeConverter.java:33)
        at com.thoughtworks.xstream.converters.extended.FontConverter.<init>(FontConverter.java:56)
        at com.thoughtworks.xstream.XStream.setupConverters(XStream.java:1008)

at least the reflection should be done on an if-need-be basis, not eagerly on first call to xstream.

If all xstream has to work with are some primitive types, maybe a HashMap, it doesn't need to reflectively setAccessible all known types in the universe. Even if the converters are created on startup, they don't have to setAccessible all the fields they encounter eagerly.

It's the weirdest developer experience in the world, having to add --add-opens=java.desktop/java.awt.font=ALL-UNNAMED in an Android project's unit tests. 😅

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