Support both okhttp 4.x and 5.x while staying on 4.x for now (#3170)

Support both okhttp 4.x and 5.x while staying on 4.x for now

Author: aldexis

changelog/@unreleased/pr-3170.v2.yml

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Support both okhttp 4.x and 5.x while staying on 4.x for now
+  links:
+  - https://github.com/palantir/conjure-java-runtime/pull/3170

okhttp-clients/src/main/java/com/palantir/conjure/java/okhttp/ConcurrencyLimitingInterceptor.java

@@ -85,7 +85,9 @@ public Response intercept(Chain chain) throws IOException {
 
     private static Response wrapResponse(Limiter.Listener listener, Response response) throws IOException {
         // OkHttp guarantees not-null to execute() and callbacks, but not at this level.
-        if (response.body() == null) {
+        // In OkHttp 5.x, response.body() is guaranteed to be non-null, but in 4.x it can be null.
+        // If it is empty however, the content type is null
+        if (response.body() == null || response.body().contentType() == null) {
             listener.onIgnore();
             return response;
         } else if (response.body().source().exhausted()) {

okhttp-clients/src/main/java/com/palantir/conjure/java/okhttp/OkHttpClients.java

@@ -53,7 +53,6 @@
 import okhttp3.OkHttpClient;
 import okhttp3.Protocol;
 import okhttp3.TlsVersion;
-import okhttp3.internal.Util;
 
 public final class OkHttpClients {
     private static final SafeLogger log = SafeLoggerFactory.get(OkHttpClients.class);
@@ -119,7 +118,7 @@ public final class OkHttpClients {
     /** The {@link ScheduledExecutorService} used for recovering leaked limits. */
     private static final Supplier<ScheduledExecutorService> limitReviver =
             Suppliers.memoize(() -> Tracers.wrap(Executors.newSingleThreadScheduledExecutor(instrument(
-                    Util.threadFactory("conjure-java-runtime/leaked limit reviver", true),
+                    threadFactory("conjure-java-runtime/leaked limit reviver", true),
                     "conjure-java-runtime/leaked limit reviver"))));
 
     /**
@@ -134,7 +133,7 @@ public final class OkHttpClients {
             Suppliers.memoize(() -> Tracers.wrap(Executors.newScheduledThreadPool(
                     NUM_SCHEDULING_THREADS,
                     instrument(
-                            Util.threadFactory("conjure-java-runtime/OkHttp Scheduler", true),
+                            threadFactory("conjure-java-runtime/OkHttp Scheduler", true),
                             "conjure-java-runtime/OkHttp Scheduler"))));
 
     private OkHttpClients() {}
@@ -340,4 +339,14 @@ private static ImmutableList<ConnectionSpec> createConnectionSpecs() {
     private static ThreadFactory instrument(ThreadFactory threadFactory, String name) {
         return MetricRegistries.instrument(SharedTaggedMetricRegistries.getSingleton(), threadFactory, name);
     }
+
+    private static ThreadFactory threadFactory(String name, boolean daemon) {
+        // Inspired by OkHttp's internal thread factory
+        // Extracted to be compatible with OkHttp 4.x and 5.x
+        return runnable -> {
+            Thread thread = new Thread(runnable, name);
+            thread.setDaemon(daemon);
+            return thread;
+        };
+    }
 }

okhttp-clients/src/main/java/com/palantir/conjure/java/okhttp/Okhttp39HostnameVerifier.java

@@ -40,11 +40,11 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
+import java.util.regex.Pattern;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLException;
 import javax.net.ssl.SSLSession;
 import javax.security.auth.x500.X500Principal;
-import okhttp3.internal.Util;
 
 /**
  * A HostnameVerifier consistent with <a href="http://www.ietf.org/rfc/rfc2818.txt">RFC 2818</a>.
@@ -60,6 +60,21 @@
  * </ol>
  */
 final class Okhttp39HostnameVerifier implements HostnameVerifier {
+    /**
+     * Regex extracted from Okhttp 4.x okhttp3.internal.Util to be compatible with Okhttp 5.x.
+     * Comment below copied over from Okhttp 4.x.
+     *
+     * Quick and dirty pattern to differentiate IP addresses from hostnames. This is an approximation
+     * of Android's private InetAddress#isNumeric API.
+     *
+     * This matches IPv6 addresses as a hex string containing at least one colon, and possibly
+     * including dots after the first colon. It matches IPv4 addresses as strings containing only
+     * decimal digits and dots. This pattern matches strings like "a:.23" and "54" that are neither IP
+     * addresses nor hostnames; they will be verified as IP addresses (which is a more strict
+     * verification).
+     */
+    private static final Pattern VERIFY_AS_IP_ADDRESS = Pattern.compile("([0-9a-fA-F]*:[0-9a-fA-F:.]*)|([\\d.]+)");
+
     public static final Okhttp39HostnameVerifier INSTANCE = new Okhttp39HostnameVerifier();
 
     private static final int ALT_DNS_NAME = 2;
@@ -78,7 +93,9 @@ public boolean verify(String host, SSLSession session) {
     }
 
     public boolean verify(String host, X509Certificate certificate) {
-        return Util.canParseAsIpAddress(host) ? verifyIpAddress(host, certificate) : verifyHostname(host, certificate);
+        return VERIFY_AS_IP_ADDRESS.matcher(host).matches()
+                ? verifyIpAddress(host, certificate)
+                : verifyHostname(host, certificate);
     }
 
     /** Returns true if {@code certificate} matches {@code ipAddress}. */

versions.props

@@ -13,7 +13,13 @@ com.palantir.safe-logging:* = 3.9.0
 com.palantir.tokens:* = 3.18.0
 com.palantir.tracing:* = 6.24.0
 com.palantir.tritium:* = 0.101.0
+# dependency-upgrader:OFF
+# We don't want to prematurely force upgrades to oktthp 5.x for consumers on 4.x.
+# There are some ABI breaks in 5.x that would require consumers to fix their code, which would be disruptive to
+# conjure upgrades.
+# Instead, conjure-java-runtime was made to be compatible with both 4.x and 5.x
 com.squareup.okhttp3:* = 4.12.0
+# dependency-upgrader:ON
 io.dropwizard.metrics:* = 4.2.33
 io.reactivex.rxjava2:rxjava = 2.2.21
 org.apache.commons:commons-lang3 = 3.17.0