Skip to content

Patch OkHttp's Platform class to avoid eager call to Security.getProviders()#12

Merged
mcculls merged 1 commit intojava7from
mcculls/avoid-okhttp-conscrypt-check
Mar 3, 2026
Merged

Patch OkHttp's Platform class to avoid eager call to Security.getProviders()#12
mcculls merged 1 commit intojava7from
mcculls/avoid-okhttp-conscrypt-check

Conversation

@mcculls
Copy link

@mcculls mcculls commented Mar 3, 2026

We want to avoid this call because it can lead to unwanted side-effects, such as initialization of java-util-logging on IBM JDKs that bundle the IBMSASL provider. The only reason for this call is to check for the Conscrypt module, which is only available on Android and is not applicable to how the tracer uses OkHttp.

Patched class is based on https://raw.githubusercontent.com/DataDog/okhttp/tags/parent-3.12.15/okhttp/src/main/java/okhttp3/internal/platform/Platform.java with the following sections of code removed:

  public static boolean isConscryptPreferred() {
    // mainly to allow tests to run cleanly
    if ("conscrypt".equals(System.getProperty("okhttp.platform"))) {
      return true;
    }

    // check if Provider manually installed
    String preferredProvider = Security.getProviders()[0].getName();
    return "Conscrypt".equals(preferredProvider);
  }
    if (isConscryptPreferred()) {
      Platform conscrypt = ConscryptPlatform.buildIfSupported();

      if (conscrypt != null) {
        return conscrypt;
      }
    }

…iders()

We want to avoid this call because it can lead to unwanted side-effects, such as
initialization of java-util-logging on IBM JDKs that bundle the IBMSASL provider.
The only reason for this call is to check for the Conscrypt module, which is only
available on Android and is not applicable to how the tracer uses OkHttp.

Patched class is based on https://raw.githubusercontent.com/DataDog/okhttp/tags/parent-3.12.15/okhttp/src/main/java/okhttp3/internal/platform/Platform.java with the following sections of code removed:

```
  public static boolean isConscryptPreferred() {
    // mainly to allow tests to run cleanly
    if ("conscrypt".equals(System.getProperty("okhttp.platform"))) {
      return true;
    }

    // check if Provider manually installed
    String preferredProvider = Security.getProviders()[0].getName();
    return "Conscrypt".equals(preferredProvider);
  }
```

```
    if (isConscryptPreferred()) {
      Platform conscrypt = ConscryptPlatform.buildIfSupported();

      if (conscrypt != null) {
        return conscrypt;
      }
    }
```
@mcculls mcculls requested a review from bric3 March 3, 2026 10:16
@bric3
Copy link

bric3 commented Mar 3, 2026

xref: DataDog/dd-trace-java#10714

@mcculls mcculls requested a review from PerfectSlayer March 3, 2026 10:19
@mcculls
Copy link
Author

mcculls commented Mar 3, 2026

Adding Bruce as second reviewer, since I need review from someone with write access

Copy link
Collaborator

@PerfectSlayer PerfectSlayer left a comment

Choose a reason for hiding this comment

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

Looking good.
To be clear, we are patching this upstrean (in our fork) and do no longer use the relocate from the dd-trace-java communication module?

@mcculls
Copy link
Author

mcculls commented Mar 3, 2026

To be clear, we are patching this upstrean (in our fork) and do no longer use the relocate from the dd-trace-java communication module?

For the short-term we are keeping the patch in dd-trace-java.

Once we are comfortable about doing a new release of our OkHttp fork (or need to do a release for a different reason) we can replace the patch with a proper release.

That's why I want to have the patched code in this fork, even if we're not doing a release yet - otherwise we risk forgetting to apply it when we actually do a new release...

@mcculls mcculls merged commit 7e95d5f into java7 Mar 3, 2026
1 check passed
@mcculls mcculls deleted the mcculls/avoid-okhttp-conscrypt-check branch March 3, 2026 10:31
@bric3
Copy link

bric3 commented Mar 3, 2026

I'd like to add that the CI not anymore running on this project.
And that this project has not been update to support be able to publish again to the Central Portal.

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.

3 participants