Skip to content

Fix #1190 by marking RequestOptions transient - #1197

Merged
richardm-stripe merged 1 commit into
stripe:masterfrom
ramon-stripe:ramon/20210419-jdk16err
Apr 22, 2021
Merged

Fix #1190 by marking RequestOptions transient#1197
richardm-stripe merged 1 commit into
stripe:masterfrom
ramon-stripe:ramon/20210419-jdk16err

Conversation

@ramon-stripe

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

Copy link
Copy Markdown
Contributor

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

CLAassistant commented Apr 19, 2021

Copy link
Copy Markdown

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
Copy Markdown
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.

@ob-stripe ob-stripe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm!

@rt-bar

rt-bar commented Feb 14, 2022

Copy link
Copy Markdown

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
Copy Markdown

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