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

okhttp-gson: fix SSL settings with okhttp3 #4226

Merged
merged 1 commit into from
Oct 24, 2019

Conversation

fabiokung
Copy link
Contributor

The old code boostrapping TLS settings used to work with older okhttp (< 3), but will throw NullPointerExceptions with okhttp3, which I noticed while working on kubernetes-client/java#709.

cc @wing328 @yue9944882 @brendandburns

fabiokung added a commit to fabiokung/java that referenced this pull request Oct 22, 2019
The old code used to work with older okhttp (< 3), but will throw
NullPointerExceptions with okhttp3.
@fabiokung fabiokung force-pushed the fix-okhttp3-gson-ssl branch from f11d2a6 to bfe40c8 Compare October 22, 2019 21:37
fabiokung added a commit to fabiokung/java that referenced this pull request Oct 22, 2019
fabiokung added a commit to fabiokung/java that referenced this pull request Oct 22, 2019
fabiokung added a commit to fabiokung/java that referenced this pull request Oct 22, 2019
fabiokung added a commit to fabiokung/java that referenced this pull request Oct 22, 2019
@fabiokung
Copy link
Contributor Author

CI failures seem unrelated.

@yue9944882
Copy link
Member

@fabiokung can you paste a re-pro for the NPE issue? thanks

@fabiokung
Copy link
Contributor Author

fabiokung commented Oct 23, 2019

@yue9944882 any usage of ApiClient that leads to applySslSettings() falling into the default case (no custom CA cert, veryfyingSsl=true) will throw the NPE.

For example, without changes in here, the snippet below throws a NPE:

ApiClient api = new ApiClient();
api.setVerifyingSsl(true);

@yue9944882
Copy link
Member

ApiClient api = new ApiClient();

the default construction of ApiClient is targeting an http endpoint on localhost. so the second line doesn't make sense, isn't it?

@wing328
Copy link
Member

wing328 commented Oct 24, 2019

CircleCI failure has been addressed in the master (it's a bad change in CircleCI image that they've rolled back the change)

@wing328
Copy link
Member

wing328 commented Oct 24, 2019

I'm able to repeat the NPE issue with api.setVerifyingSsl(true); and confirm your PR fixes it.

@wing328
Copy link
Member

wing328 commented Oct 24, 2019

the default construction of ApiClient is targeting an http endpoint on localhost. so the second line doesn't make sense, isn't it?

ApiClient can be treated as a wrapper of the underlying HTTP library for any endpoints/hosts specified in the OpenAPI doc/spec

@wing328 wing328 merged commit c3666e9 into OpenAPITools:master Oct 24, 2019
@wing328 wing328 mentioned this pull request Oct 24, 2019
5 tasks
@wing328
Copy link
Member

wing328 commented Oct 24, 2019

Filed #4252 to add a test covering the issue moving forward.

fabiokung added a commit to fabiokung/java that referenced this pull request Oct 24, 2019
fabiokung added a commit to fabiokung/java that referenced this pull request Oct 24, 2019
yue9944882 pushed a commit to yue9944882/java that referenced this pull request Nov 6, 2019
yue9944882 pushed a commit to yue9944882/java that referenced this pull request Nov 8, 2019
depends on yue9944882/gen#1

okhttp3 and models generated by openapi-generator

Changes required to make everything compile with models generated by
openapi-generator (instead of swagger-codegen).

These changes break backwards compatibility, it will require a new major
version release.

pin sundrio to 0.19.2

it includes sundrio/sundrio#156

reformat code (./mvnw fmt)

depend on javax.annotation:javax.annotation-api

to make javax.annotation.Generated work with Java 9

prevent Travis builds failing because of no output

... when tests take more than 10min to complete.

javadoc generation for http.response.details

drop the TODO, not needed anymore

remove reference to swagger-codegen

no gson internals for date parsing/formatting

Reapply changes from kubernetes-client#366 to the code generated by openapi-generator,
and keep the JSON file pinned (ignored by the generator) until those
changes make it into the generator.

regenerate code with SSL bootstrapping fix

Using a generator with OpenAPITools/openapi-generator#4226 applied.

re-generate from v4.2.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants