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

Fix #1190 by marking RequestOptions transient #1197

Merged
merged 1 commit into from
Apr 22, 2021

Conversation

ramon-stripe
Copy link
Contributor

@ramon-stripe ramon-stripe commented Apr 19, 2021

cc @stripe/api-libraries
The problem was that Gson was attempting to make certain java.net classes constructors accessible through reflection but this is not longer allowed in JDK16 with the default module setup.

The reason that Gson was attempting to instantiate these classes was that we forgot to mark the field StripeCollection.requestOptions transient (it isn't part of the JSON response). The type RequestOptions in turn contains a java.net.Proxy-type field, which was the source of the problem.

I also added ReflectionCheckingTypeAdapterFactory which is a fake TypeAdapterFactory that just checks that we aren't using reflection to access anything outside of com.stripe packages.

In trying to replicate this problem I tried to get this project to build on JDK 16 but that required an upgrade to Gradle 7 which turned out to be too much to put into this PR. The WIP branch for that is here.

Fixes #1190

@CLAassistant
Copy link

CLAassistant commented Apr 19, 2021

CLA assistant check
All committers have signed the CLA.

Also try to prevent similar problems in the future by checking that
we only use the reflection-based type adapter for classes in `com.stripe.`.
@ramon-stripe ramon-stripe force-pushed the ramon/20210419-jdk16err branch from 8121871 to 148a5ac Compare April 19, 2021 21:15
@richardm-stripe
Copy link
Contributor

r? @ob-stripe would you take a look at this?
Thank you so much @ramon-stripe!!
Ignore the github actions, those have been flaking and I need to take a look. The travis CI is the real deal.

Copy link
Contributor

@ob-stripe ob-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@rt-bar
Copy link

rt-bar commented Feb 14, 2022

Hello,

For information :

In the "ReflectionCheckingTypeAdapterFactory" added by this fix, there is the import of "com.google.gson.internal.bind.ReflectiveTypeAdapterFactory" that is "internal" and not exported in the gson Manifest (Export-Package)

So in OSGI context, this generate an "Unresolved constraint" error when wiring.

@Marcono1234
Copy link

Gson 2.9.1 added a feature which should allow implementing this without having to rely on internal Gson classes.

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

Successfully merging this pull request may close these issues.

java.lang.reflect.InaccessibleObjectException
6 participants