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

NetHttpTransport.Builder trustCertificates() clobbers any changes made through setSslSocketFactory() #1663

Open
frankyn opened this issue May 13, 2022 · 0 comments
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: docs Improvement to the documentation for an API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@frankyn
Copy link
Member

frankyn commented May 13, 2022

Environment details

  1. OS type and version: any
  2. Java version: any
  3. version(s): google-http-client-gson:1.40.1

Steps to reproduce

The issue came up for Dataproc Hadoop Connector (GoogleCloudDataproc/hadoop-connectors#782) which uses a custom SslSocketFactory to enable socket level options, e.g. keep-alive. When the builder was used this way it would lose context on trustCertificates or vice-versa depending on which method was called first.

  1. Create a builder calling trustCertificates, followed by setSslSocketFactory(); then build.
  2. TrustCertificates mutation is lost

Addressing the builder implementation to be more of an intent rather than calling setSslSocketFactory() in trustCertificates would help make the builder easier to use. In addition documentation for these builder methods is non-descriptive beyond what the value means and it would help to call out quirks in implementation.

Code example

// I used defaultSslSocketFactory() but expect that it can be any SslSocketFactory
NetHttpTransport transport = new NetHttpTransport.Builder()
            .trustCertificates(GoogleUtils.getCertificateTrustStore());
            .setSslSocketFactory(HttpsURLConnection.getDefaultSSLSocketFactory()).build();

External references such as API reference guides

  • This builder is has no documented guidance which would have helped understand the conflict that could occur.
@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label May 14, 2022
@eaball35 eaball35 added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. type: docs Improvement to the documentation for an API. priority: p3 Desirable enhancement or fix. May not be included in next release. priority: p2 Moderately-important priority. Fix may not be included in next release. and removed triage me I really want to be triaged. priority: p3 Desirable enhancement or fix. May not be included in next release. labels May 16, 2022
@meltsufin meltsufin added priority: p3 Desirable enhancement or fix. May not be included in next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Sep 20, 2022
gcf-owl-bot bot added a commit that referenced this issue Oct 24, 2022
chore: remove check for flatten plugin

We have had the check for the flatten-maven-plugin in each
Cloud Java client repository. However, the behavior of the plugin
has been stable and its not each repository's responsibility to
assert the plugin's behavior. A new check is going to be added at
the googleapis/java-shared-config repository to assert the
plugin's behavior when we upgrade its version.
Source-Link: googleapis/synthtool@9266ddc
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:ae72564aa9c368b9ccd96f4af21f87889fd83b9e60635b80844deb5a2ccd08aa
gcf-merge-on-green bot pushed a commit that referenced this issue Oct 24, 2022
chore: remove check for flatten plugin

We have had the check for the flatten-maven-plugin in each
Cloud Java client repository. However, the behavior of the plugin
has been stable and its not each repository's responsibility to
assert the plugin's behavior. A new check is going to be added at
the googleapis/java-shared-config repository to assert the
plugin's behavior when we upgrade its version.
Source-Link: https://togithub.com/googleapis/synthtool/commit/9266ddc3b17fc15f34d2fb88ce8c5f1a4bfe64b0
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:ae72564aa9c368b9ccd96f4af21f87889fd83b9e60635b80844deb5a2ccd08aa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: docs Improvement to the documentation for an API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

4 participants