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

JDK 16 Illegal reflective access for Throwable.setCause() with PropertyNamingStrategy.UPPER_CAMEL_CASE #3275

Closed
jsharper opened this issue Sep 13, 2021 · 16 comments
Milestone

Comments

@jsharper
Copy link

jsharper commented Sep 13, 2021

Describe the bug
Mapping a json string to an instance of RuntimeException with JDK 16 (which defaults to denying illegal reflective access) while using UPPER_CAMEL_CASE property naming strategy, fails with:

Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Failed to call `setAccess()` on Method 'setCause' due to `java.lang.reflect.InaccessibleObjectException`, problem: Unable to make final void java.lang.Throwable.setCause(java.lang.Throwable) accessible: module java.base does not "opens java.lang" to unnamed module

This manifests in parts of the AWS v1 SDK when an error is received from the AWS API endpoints. Note example report: #2464 (comment)

The simplified reproduce case below is based on what the failing AWS SDK code is doing internally in com.amazonaws.transform.JsonErrorUnmarshaller.unmarshall.

Version information
2.12.3, 2.12.5, 2.13.0-rc2

To Reproduce
Execute this code without any --add-opens params and with default --illegal-access setting of deny.

public class Test {
	public static void main(String[] args) throws Exception {
		ObjectMapper mapper = new ObjectMapper();
		mapper.setPropertyNamingStrategy(PropertyNamingStrategies.UPPER_CAMEL_CASE);

		String jsonString = "{\"message\":\"This is my runtime exception message\"}";
		JsonNode jsonContent = mapper.readTree(jsonString);

		throw mapper.treeToValue(jsonContent, RuntimeException.class);
	}
}

Expected behavior
It is expected that the code above would throw a RuntimeException with a message of "This is my runtime exception message". This is what happens when working around the issue with java parameter --add-opens java.base/java.lang=ALL-UNNAMED

Additional context
Using java parameter --illegal-access=debug results in the following additional info:

WARNING: Illegal reflective access by com.fasterxml.jackson.databind.util.ClassUtil (file:/xxxxxx/jackson-databind-2.13.0-rc2.jar) to method java.lang.Throwable.setCause(java.lang.Throwable)
	at com.fasterxml.jackson.databind.util.ClassUtil.checkAndFixAccess(ClassUtil.java:994)
	at com.fasterxml.jackson.databind.introspect.AnnotatedMember.fixAccess(AnnotatedMember.java:139)
	at com.fasterxml.jackson.databind.deser.impl.MethodProperty.fixAccess(MethodProperty.java:95)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBuilder._fixAccess(BeanDeserializerBuilder.java:522)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBuilder.build(BeanDeserializerBuilder.java:373)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.buildThrowableDeserializer(BeanDeserializerFactory.java:455)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.createBeanDeserializer(BeanDeserializerFactory.java:112)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer2(DeserializerCache.java:415)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer(DeserializerCache.java:350)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:264)
	at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
	at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
	at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:642)
	at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:4751)
	at com.fasterxml.jackson.databind.ObjectMapper._readValue(ObjectMapper.java:4596)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2815)
	at com.fasterxml.jackson.databind.ObjectMapper.treeToValue(ObjectMapper.java:3279)
	at Test.main(Test.java:13)

Note that the issue does not manifest without mapper.setPropertyNamingStrategy(PropertyNamingStrategies.UPPER_CAMEL_CASE);

@jsharper jsharper added the to-evaluate Issue that has been received but not yet evaluated label Sep 13, 2021
@cowtowncoder
Copy link
Member

Hmmh. This may be tricky, due to the way ThrowableDeserializer works. I wish I had time to figure out proper way to add a test case for JDK 16 (or maybe 17 since it's out).

Some thoughts, notes:

  • I suspect that naming strategy just serves to prevent message from mapping: changing that to, bogus should (I think) exhibit same behavior
  • Similarly straight readValue() (instead of readTree() + convertValue()) should reproduce the problem.

If so, the challenge here is, I think, that ThrowableDeserializer (re)uses POJO introspection for methods like setCause() -- which unfortunately won't be forcible. With some work it should be possible to cover this specific cause since setCause() is well-known.
But I don't have bandwidth right now. If anyone would like to see if there's a way to change code in a simple way, I'd be happy to help with review etc however.

This also looks like it could possibly be fixed in a patch after 2.13.0 is released (which I intend to do this week), not requiring a minor version update (that is, I think fix does not require API changes).

@cowtowncoder cowtowncoder added 2.13 and removed to-evaluate Issue that has been received but not yet evaluated labels Sep 28, 2021
@gsinghlulu
Copy link

I'm getting this error with AWS KMS SDK and Java 17

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Failed to call `setAccess()` on Method 'setCause' due to `java.lang.reflect.InaccessibleObjectException`, problem: Unable to make final void java.lang.Throwable.setCause(java.lang.Throwable) accessible: module java.base does not "opens java.lang" to unnamed module @129a8472
at [Source: UNKNOWN; byte offset: #UNKNOWN]
at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67) ~[jackson-databind-2.13.1.jar!/:2.13.1]
at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1904) ~[jackson-databind-2.13.1.jar!/:2.13.1]
at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:268) ~[jackson-databind-2.13.1.jar!/:2.13.1]
at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244) ~[jackson-databind-2.13.1.jar!/:2.13.1]
at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142) ~[jackson-databind-2.13.1.jar!/:2.13.1]
at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:642) ~[jackson-databind-2.13.1.jar!/:2.13.1]
at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:4805) ~[jackson-databind-2.13.1.jar!/:2.13.1]
at com.fasterxml.jackson.databind.ObjectMapper._readValue(ObjectMapper.java:4650) ~[jackson-databind-2.13.1.jar!/:2.13.1]
at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2831) ~[jackson-databind-2.13.1.jar!/:2.13.1]
at com.fasterxml.jackson.databind.ObjectMapper.treeToValue(ObjectMapper.java:3295) ~[jackson-databind-2.13.1.jar!/:2.13.1]
at com.amazonaws.transform.JsonErrorUnmarshaller.unmarshall(JsonErrorUnmarshaller.java:61) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.JsonErrorResponseHandler.doLegacyUnmarshall(JsonErrorResponseHandler.java:185) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.JsonErrorResponseHandler.unmarshallException(JsonErrorResponseHandler.java:147) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.JsonErrorResponseHandler.createException(JsonErrorResponseHandler.java:131) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.JsonErrorResponseHandler.handle(JsonErrorResponseHandler.java:94) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.JsonErrorResponseHandler.handle(JsonErrorResponseHandler.java:40) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.AwsErrorResponseHandler.handleAse(AwsErrorResponseHandler.java:58) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.AwsErrorResponseHandler.handle(AwsErrorResponseHandler.java:45) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.AwsErrorResponseHandler.handle(AwsErrorResponseHandler.java:27) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.AmazonHttpClient$RequestExecutor.handleErrorResponse(AmazonHttpClient.java:1801) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.AmazonHttpClient$RequestExecutor.handleServiceErrorResponse(AmazonHttpClient.java:1403) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.AmazonHttpClient$RequestExecutor.executeOneRequest(AmazonHttpClient.java:1372) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.AmazonHttpClient$RequestExecutor.executeHelper(AmazonHttpClient.java:1145) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.AmazonHttpClient$RequestExecutor.doExecute(AmazonHttpClient.java:802) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.AmazonHttpClient$RequestExecutor.executeWithTimer(AmazonHttpClient.java:770) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.AmazonHttpClient$RequestExecutor.execute(AmazonHttpClient.java:744) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.AmazonHttpClient$RequestExecutor.access$500(AmazonHttpClient.java:704) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.AmazonHttpClient$RequestExecutionBuilderImpl.execute(AmazonHttpClient.java:686) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.AmazonHttpClient.execute(AmazonHttpClient.java:550) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.http.AmazonHttpClient.execute(AmazonHttpClient.java:530) ~[aws-java-sdk-core-1.11.997.jar!/:na]
at com.amazonaws.services.kms.AWSKMSClient.doInvoke(AWSKMSClient.java:7223) ~[aws-java-sdk-kms-1.11.997.jar!/:na]
at com.amazonaws.services.kms.AWSKMSClient.invoke(AWSKMSClient.java:7190) ~[aws-java-sdk-kms-1.11.997.jar!/:na]
at com.amazonaws.services.kms.AWSKMSClient.invoke(AWSKMSClient.java:7179) ~[aws-java-sdk-kms-1.11.997.jar!/:na]
at com.amazonaws.services.kms.AWSKMSClient.executeDecrypt(AWSKMSClient.java:1775) ~[aws-java-sdk-kms-1.11.997.jar!/:na]
at com.amazonaws.services.kms.AWSKMSClient.decrypt(AWSKMSClient.java:1744) ~[aws-java-sdk-kms-1.11.997.jar!/:na]

@cowtowncoder
Copy link
Member

@gsinghlulu I would need a fully reproduction to know what happens; stack trace alone is not enoigh.

@gsinghlulu
Copy link

This exception is thrown we try to decrypt a text that was encrypted using a key which application do not have access. Root cause of this exception is
Service: AWSKMS; Status Code: 400; Error Code: AccessDeniedException;

@cowtowncoder
Copy link
Member

Looking at the original report, the issue is wrt attempts to deserialize an Exception, during which setCause() needs to be accessed; and for that to work, access must be forced.

I am not quite sure how to proceed with this: my first instinct is to try to suppress the failure (well, very first thing is JDK17-specific test case but after that) for this specific setter.
But that in turn will probably expose the problem of not being able to chain root cause exceptions on JDK 17 and later...

@gsinghlulu
Copy link

gsinghlulu commented May 24, 2022

The exception is thrown here
Class: com.fasterxml.jackson.databind.deser.impl.MethodProperty
image

Seems like jackson is getting mixed up w.r.t modifier for Throwable.cause field, thinking it to be setCause() when it should be initCause()

@gsinghlulu
Copy link

gsinghlulu commented May 24, 2022

I found something interesting, there are 2 cause properties showing up for AWSKMSException one with capitalized C having setCause as modifier and lowercase c having initCause() as modifier

{Cause=[property 'Cause'], StackTrace=[property 'StackTrace'], RequestId=[property 'RequestId'], ErrorCode=[property 'ErrorCode'], ErrorType=[property 'ErrorType'], ErrorMessage=[property 'ErrorMessage'], StatusCode=[property 'StatusCode'], ServiceName=[property 'ServiceName'], HttpHeaders=[property 'HttpHeaders'], RawResponse=[property 'RawResponse'], ProxyHost=[property 'ProxyHost'], RawResponseContent=[property 'RawResponseContent'], cause=[property 'cause']}

gsinghlulu added a commit to gsinghlulu/jackson-databind that referenced this issue May 24, 2022
@gsinghlulu
Copy link

gsinghlulu commented May 24, 2022

I raise a PR to workaround the issue. It's working for Java 17. Please review
#3494

@cowtowncoder
Copy link
Member

@gsinghlulu This is not really related to the issue reported here is it? Could you please file a separate issue if not related to the original issue -- PR can be attached to that one.

@gsinghlulu
Copy link

Though my issue is with AWS SDK it is because they are using PropertyNamingStrategies.UPPER_CAMEL_CASE setting for ObjectMapper. Hence the fix should work for this issue as well. The problem is the capitalization of C in Cause property name because of which when builder.addOrReplaceProperty(prop, true) is called for initCause handling in BeanDeserializerFactor, the cause property is not getting overridden.

        // But then let's decorate things a bit
        // Need to add "initCause" as setter for exceptions (sub-classes of Throwable).
        AnnotatedMethod am = beanDesc.findMethod("initCause", INIT_CAUSE_PARAMS);
        if (am != null) { // should never be null
            //This is a workaround for camelcasing the property names in Java 16 and above
            SimpleBeanPropertyDefinition propDef = SimpleBeanPropertyDefinition.construct(ctxt.getConfig(), am,
                    new PropertyName(getThrowableCausePropertyName(builder)));
            SettableBeanProperty prop = constructSettableProperty(ctxt, beanDesc, propDef,
                    am.getParameterType(0));
            if (prop != null) {
                // 21-Aug-2011, tatus: We may actually have found 'cause' property
                //   to set... but let's replace it just in case, otherwise can end up with odd errors.
                builder.addOrReplaceProperty(prop, true); //Line 449
            }
        }

Refer com.amazonaws.transform.JsonErrorUnmarshaller

@cowtowncoder
Copy link
Member

@gsinghlulu ah! Ok, I should paid closer attention there. Ok I can see how things might go there... hmmh.
This does seem not like the optimal way to resolve the problem, but I can see how it would actually fix the issue here. The only (?) thing that is confusing is where the upper-case Cause comes from -- I could see it being result of NamingStrategy but not sure if that could yet have been applied.

... but perhaps it is, as you mentioned, from Field that gets renamed (initCause() won't because we are just adding it).

I think I will have another look here: thank you for providing the potential fix, as well as explaining it -- and apologies for misunderstanding it first.

@cowtowncoder
Copy link
Member

Hmmh. The most confusing aspect is the alleged method setCause() in Throwable. I can't find any references to it via JDK Javadocs... but it seems to exist somehow?

@yawkat
Copy link
Member

yawkat commented May 25, 2022

@gsinghlulu
Copy link

@cowtowncoder Yes I agree it's not an optimal fix, but rather a workaround. The uppercase Cause is coming from the Throwable.cause field, when PropertyNamingStrategies.UPPER_CAMEL_CASE strategy is used.

@cowtowncoder
Copy link
Member

@yawkat Ok. Just didn't see it in JDK javadocs; I guess only public (and maybe protected methods are included).

@gsinghlulu yes, correct on both accounts. I think fix is along the right lines; will see if we could first remove setCause() if it exists (or replace it). I am only hesitant about looking at Cause in case other translations might miss it. But it might be good enough too, either way thank you for providing it!

@cowtowncoder cowtowncoder changed the title JDK 16 Illegal reflective access for Throwable.setCause with PropertyNamingStrategy UPPER_CAMEL_CASE JDK 16 Illegal reflective access for Throwable.setCause() with PropertyNamingStrategy.UPPER_CAMEL_CASE May 26, 2022
@cowtowncoder cowtowncoder added this to the 2.13.4 milestone May 26, 2022
@cowtowncoder
Copy link
Member

Ok, yes, JDK 12 added Throwable.setCause() apparently.

This specific issue now fixed in 2.13 (for 2.13.4, then 2.14.0).
But I think there is the follow-up issue of PropertyNamingStrategy not working for Throwable, will file separate issue for that.

millems added a commit to millems/jackson-databind that referenced this issue May 1, 2024
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

4 participants