From cd92e95e110ea47722c428d6a6f6cf029a27e609 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Tue, 12 Aug 2025 21:43:19 +0100 Subject: [PATCH 01/12] Add Call.Factory --- android-test/build.gradle.kts | 1 + okhttp/api/android/okhttp.api | 12 ++++++ okhttp/api/jvm/okhttp.api | 12 ++++++ .../commonJvmAndroid/kotlin/okhttp3/Call.kt | 29 ++++++++++++++ .../kotlin/okhttp3/OkHttpClient.kt | 38 ++++++++++++++++++- 5 files changed, 91 insertions(+), 1 deletion(-) diff --git a/android-test/build.gradle.kts b/android-test/build.gradle.kts index 8b97915e931b..8d45a2d4087b 100644 --- a/android-test/build.gradle.kts +++ b/android-test/build.gradle.kts @@ -65,6 +65,7 @@ dependencies { "friendsImplementation"(projects.okhttpDnsoverhttps) testImplementation(projects.okhttp) + testImplementation(projects.okhttpCoroutines) testImplementation(libs.junit) testImplementation(libs.junit.ktx) testImplementation(libs.assertk) diff --git a/okhttp/api/android/okhttp.api b/okhttp/api/android/okhttp.api index 32d88d388d8e..0e73b572894f 100644 --- a/okhttp/api/android/okhttp.api +++ b/okhttp/api/android/okhttp.api @@ -129,6 +129,16 @@ public abstract interface class okhttp3/Call : java/lang/Cloneable { public abstract fun timeout ()Lokio/Timeout; } +public abstract interface class okhttp3/Call$Chain { + public abstract fun getClient ()Lokhttp3/OkHttpClient; + public abstract fun getRequest ()Lokhttp3/Request; + public abstract fun proceed (Lokhttp3/Request;)Lokhttp3/Call; +} + +public abstract interface class okhttp3/Call$Decorator { + public abstract fun newCall (Lokhttp3/Call$Chain;)Lokhttp3/Call; +} + public abstract interface class okhttp3/Call$Factory { public abstract fun newCall (Lokhttp3/Request;)Lokhttp3/Call; } @@ -902,6 +912,7 @@ public class okhttp3/OkHttpClient : okhttp3/Call$Factory, okhttp3/WebSocket$Fact public final fun fastFallback ()Z public final fun followRedirects ()Z public final fun followSslRedirects ()Z + public final fun getCallDecorators ()Ljava/util/List; public final fun hostnameVerifier ()Ljavax/net/ssl/HostnameVerifier; public final fun interceptors ()Ljava/util/List; public final fun minWebSocketMessageToCompress ()J @@ -927,6 +938,7 @@ public final class okhttp3/OkHttpClient$Builder { public final fun -addInterceptor (Lkotlin/jvm/functions/Function1;)Lokhttp3/OkHttpClient$Builder; public final fun -addNetworkInterceptor (Lkotlin/jvm/functions/Function1;)Lokhttp3/OkHttpClient$Builder; public fun ()V + public final fun addCallDecorator (Lokhttp3/Call$Decorator;)Lokhttp3/OkHttpClient$Builder; public final fun addInterceptor (Lokhttp3/Interceptor;)Lokhttp3/OkHttpClient$Builder; public final fun addNetworkInterceptor (Lokhttp3/Interceptor;)Lokhttp3/OkHttpClient$Builder; public final fun authenticator (Lokhttp3/Authenticator;)Lokhttp3/OkHttpClient$Builder; diff --git a/okhttp/api/jvm/okhttp.api b/okhttp/api/jvm/okhttp.api index 0133737243fc..040ea91cc844 100644 --- a/okhttp/api/jvm/okhttp.api +++ b/okhttp/api/jvm/okhttp.api @@ -129,6 +129,16 @@ public abstract interface class okhttp3/Call : java/lang/Cloneable { public abstract fun timeout ()Lokio/Timeout; } +public abstract interface class okhttp3/Call$Chain { + public abstract fun getClient ()Lokhttp3/OkHttpClient; + public abstract fun getRequest ()Lokhttp3/Request; + public abstract fun proceed (Lokhttp3/Request;)Lokhttp3/Call; +} + +public abstract interface class okhttp3/Call$Decorator { + public abstract fun newCall (Lokhttp3/Call$Chain;)Lokhttp3/Call; +} + public abstract interface class okhttp3/Call$Factory { public abstract fun newCall (Lokhttp3/Request;)Lokhttp3/Call; } @@ -901,6 +911,7 @@ public class okhttp3/OkHttpClient : okhttp3/Call$Factory, okhttp3/WebSocket$Fact public final fun fastFallback ()Z public final fun followRedirects ()Z public final fun followSslRedirects ()Z + public final fun getCallDecorators ()Ljava/util/List; public final fun hostnameVerifier ()Ljavax/net/ssl/HostnameVerifier; public final fun interceptors ()Ljava/util/List; public final fun minWebSocketMessageToCompress ()J @@ -926,6 +937,7 @@ public final class okhttp3/OkHttpClient$Builder { public final fun -addInterceptor (Lkotlin/jvm/functions/Function1;)Lokhttp3/OkHttpClient$Builder; public final fun -addNetworkInterceptor (Lkotlin/jvm/functions/Function1;)Lokhttp3/OkHttpClient$Builder; public fun ()V + public final fun addCallDecorator (Lokhttp3/Call$Decorator;)Lokhttp3/OkHttpClient$Builder; public final fun addInterceptor (Lokhttp3/Interceptor;)Lokhttp3/OkHttpClient$Builder; public final fun addNetworkInterceptor (Lokhttp3/Interceptor;)Lokhttp3/OkHttpClient$Builder; public final fun authenticator (Lokhttp3/Authenticator;)Lokhttp3/OkHttpClient$Builder; diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Call.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Call.kt index fdd3d3da294e..371bd4c715e2 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Call.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Call.kt @@ -96,4 +96,33 @@ interface Call : Cloneable { fun interface Factory { fun newCall(request: Request): Call } + + /** + * The equivalent of an Interceptor for [Call.Factory], but supported directly within [OkHttpClient] newCall. + * + * An [Interceptor] forms a chain as part of execution of a Call. Instead, Call.Decorator intercepts + * [Call.Factory.newCall] with similar flexibility to Application [OkHttpClient.interceptors]. + * + * That is, it may do any of + * - Modify the request such as adding Tracing Context + * - Wrap the [Call] returned + * - Return some [Call] implementation that will immediately fail avoiding network calls based on network or + * authentication state. + * - Redirect the [Call], such as using an alternative [Call.Factory]. + * - Defer execution, something not safe in an Interceptor. + * + * It should not throw an exception, instead it should return a Call that will fail on [Call.execute]. + * + * A Decorator that changes the OkHttpClient should typically retain later decorators in the new client. + */ + fun interface Decorator { + fun newCall(chain: Chain): Call + } + + interface Chain { + val client: OkHttpClient + val request: Request + + fun proceed(request: Request): Call + } } diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/OkHttpClient.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/OkHttpClient.kt index de3e75d5e701..39739f422061 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/OkHttpClient.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/OkHttpClient.kt @@ -145,6 +145,14 @@ open class OkHttpClient internal constructor( val interceptors: List = builder.interceptors.toImmutableList() + /** + * Returns an immutable list of Call decorators that have a chance to return a different, likely + * decorating, implementation of Call. This allows functionality such as fail fast without normal Call + * execution based on network conditions, or setting Tracing context on the calling thread. + */ + val callDecorators: List = + builder.callDecorators.toImmutableList() + /** * Returns an immutable list of interceptors that observe a single network request and response. * These interceptors must call [Interceptor.Chain.proceed] exactly once: it is an error for @@ -265,6 +273,27 @@ open class OkHttpClient internal constructor( internal val routeDatabase: RouteDatabase = builder.routeDatabase ?: RouteDatabase() internal val taskRunner: TaskRunner = builder.taskRunner ?: TaskRunner.INSTANCE + private val decoratedCallFactory = + callDecorators.foldRight( + Call.Factory { request -> + RealCall(client = this, originalRequest = request, forWebSocket = false) + }, + ) { callDecorator, next -> + Call.Factory { request -> + callDecorator.newCall( + object : Call.Chain { + override val client: OkHttpClient + get() = this@OkHttpClient + + override val request: Request + get() = request + + override fun proceed(request: Request): Call = next.newCall(request) + }, + ) + } + } + @get:JvmName("connectionPool") val connectionPool: ConnectionPool = builder.connectionPool ?: ConnectionPool( @@ -359,7 +388,7 @@ open class OkHttpClient internal constructor( } /** Prepares the [request] to be executed at some point in the future. */ - override fun newCall(request: Request): Call = RealCall(this, request, forWebSocket = false) + override fun newCall(request: Request): Call = decoratedCallFactory.newCall(request) /** Uses [request] to connect a new web socket. */ override fun newWebSocket( @@ -596,6 +625,7 @@ open class OkHttpClient internal constructor( internal var dispatcher: Dispatcher = Dispatcher() internal var connectionPool: ConnectionPool? = null internal val interceptors: MutableList = mutableListOf() + internal val callDecorators: MutableList = mutableListOf() internal val networkInterceptors: MutableList = mutableListOf() internal var eventListenerFactory: EventListener.Factory = EventListener.NONE.asFactory() internal var retryOnConnectionFailure = true @@ -631,6 +661,7 @@ open class OkHttpClient internal constructor( this.dispatcher = okHttpClient.dispatcher this.connectionPool = okHttpClient.connectionPool this.interceptors += okHttpClient.interceptors + this.callDecorators += okHttpClient.callDecorators this.networkInterceptors += okHttpClient.networkInterceptors this.eventListenerFactory = okHttpClient.eventListenerFactory this.retryOnConnectionFailure = okHttpClient.retryOnConnectionFailure @@ -735,6 +766,11 @@ open class OkHttpClient internal constructor( this.eventListenerFactory = eventListenerFactory } + fun addCallDecorator(decorator: Call.Decorator) = + apply { + callDecorators += decorator + } + /** * Configure this client to retry or not when a connectivity problem is encountered. By default, * this client silently recovers from the following problems: From c9adb47534ade0f197cc66931c87ccdf968a1dce Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Tue, 12 Aug 2025 21:50:53 +0100 Subject: [PATCH 02/12] copy code across --- .../android/httpengine/CronetCallFactory.java | 326 +++++++++++++++++ .../android/httpengine/CronetInterceptor.java | 166 +++++++++ .../httpengine/CronetTimeoutException.java | 21 ++ .../CronetTransportResponseBody.java | 55 +++ .../OkHttpBridgeRequestCallback.java | 312 ++++++++++++++++ .../android/httpengine/RedirectStrategy.java | 89 +++++ .../httpengine/RequestBodyConverter.java | 28 ++ .../httpengine/RequestBodyConverterImpl.java | 332 ++++++++++++++++++ .../httpengine/RequestResponseConverter.java | 164 +++++++++ .../RequestResponseConverterBasedBuilder.java | 90 +++++ .../android/httpengine/ResponseConverter.java | 260 ++++++++++++++ .../httpengine/UploadBodyDataBroker.java | 178 ++++++++++ 12 files changed, 2021 insertions(+) create mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetCallFactory.java create mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetInterceptor.java create mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetTimeoutException.java create mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetTransportResponseBody.java create mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.java create mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.java create mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.java create mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.java create mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.java create mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.java create mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.java create mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.java diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetCallFactory.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetCallFactory.java new file mode 100644 index 000000000000..e4f4a120d574 --- /dev/null +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetCallFactory.java @@ -0,0 +1,326 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package okhttp3.android.httpengine; + +import android.util.Log; +import com.google.net.cronet.okhttptransport.RequestResponseConverter.CronetRequestAndOkHttpResponse; +import okhttp3.Call; +import okhttp3.Callback; +import okhttp3.Request; +import okhttp3.Response; +import okio.AsyncTimeout; +import okio.Timeout; +import android.net.http.HttpEngine; + +import java.io.IOException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import static com.google.common.base.Preconditions.*; +import static java.util.concurrent.TimeUnit.MILLISECONDS; + +/** A {@link Call.Factory} implementation using Cronet as the transport layer. */ +public final class CronetCallFactory implements Call.Factory { + + private static final String TAG = "CronetCallFactory"; + + private final RequestResponseConverter converter; + private final ExecutorService responseCallbackExecutor; + private final int readTimeoutMillis; + private final int writeTimeoutMillis; + private final int callTimeoutMillis; + + private CronetCallFactory( + RequestResponseConverter converter, + ExecutorService responseCallbackExecutor, + int readTimeoutMillis, + int writeTimeoutMillis, + int callTimeoutMillis) { + checkArgument(readTimeoutMillis >= 0, "Read timeout mustn't be negative!"); + checkArgument(writeTimeoutMillis >= 0, "Write timeout mustn't be negative!"); + checkArgument(callTimeoutMillis >= 0, "Call timeout mustn't be negative!"); + + this.converter = converter; + this.responseCallbackExecutor = responseCallbackExecutor; + this.readTimeoutMillis = readTimeoutMillis; + this.writeTimeoutMillis = writeTimeoutMillis; + this.callTimeoutMillis = callTimeoutMillis; + } + + public static Builder newBuilder(HttpEngine HttpEngine) { + return new Builder(HttpEngine); + } + + @Override + public Call newCall(Request request) { + return new CronetCall(request, this, converter, responseCallbackExecutor); + } + + private static class CronetCall implements Call { + + private final Request okHttpRequest; + private final CronetCallFactory motherFactory; + private final RequestResponseConverter converter; + private final ExecutorService responseCallbackExecutor; + + private final AtomicBoolean executed = new AtomicBoolean(); + private final AtomicBoolean canceled = new AtomicBoolean(); + private final AtomicReference convertedRequestAndResponse = + new AtomicReference<>(); + private final AsyncTimeout timeout; + + private CronetCall( + Request okHttpRequest, + CronetCallFactory motherFactory, + RequestResponseConverter converter, + ExecutorService responseCallbackExecutor) { + this.okHttpRequest = okHttpRequest; + this.motherFactory = motherFactory; + this.converter = converter; + this.responseCallbackExecutor = responseCallbackExecutor; + + this.timeout = + new AsyncTimeout() { + @Override + protected void timedOut() { + CronetCall.this.cancel(); // Timeout has its own method named cancel + } + }; + timeout.timeout(motherFactory.callTimeoutMillis, MILLISECONDS); + } + + @Override + public Request request() { + return okHttpRequest; + } + + @Override + public Response execute() throws IOException { + evaluateExecutionPreconditions(); + try { + timeout.enter(); + CronetRequestAndOkHttpResponse requestAndOkHttpResponse = + converter.convert( + request(), motherFactory.readTimeoutMillis, motherFactory.writeTimeoutMillis); + convertedRequestAndResponse.set(requestAndOkHttpResponse); + + startRequestIfNotCanceled(); + + return toCronetCallFactoryResponse(this, requestAndOkHttpResponse.getResponse()); + } catch (RuntimeException | IOException e) { + // If the request finished successfully don't exit the timeout yet. Reading the body also + // needs to be considered and the body object will take care of exiting it. See + // toCronetCallFactoryResponse() for details. + timeout.exit(); + throw e; + } + } + + @Override + public void enqueue(Callback responseCallback) { + try { + timeout.enter(); + evaluateExecutionPreconditions(); + CronetRequestAndOkHttpResponse requestAndOkHttpResponse = + converter.convert( + request(), motherFactory.readTimeoutMillis, motherFactory.writeTimeoutMillis); + convertedRequestAndResponse.set(requestAndOkHttpResponse); + CronetCall call = this; + + Futures.addCallback( + requestAndOkHttpResponse.getResponseAsync(), + new FutureCallback() { + @Override + public void onSuccess(Response result) { + try { + responseCallback.onResponse(call, toCronetCallFactoryResponse(call, result)); + } catch (IOException e) { + // The call factory doesn't really mind this - the application code + // threw an exception while handling the response, they should have taken care + // of it. Just logging the error is consistent with plain OkHttp implementation. + Log.i(TAG, "Callback failure for " + toLoggableString(), e); + } + } + + @Override + public void onFailure(Throwable t) { + if (t instanceof IOException) { + responseCallback.onFailure(call, (IOException) t); + } else { + responseCallback.onFailure(call, new IOException(t)); + } + } + }, + responseCallbackExecutor); + + startRequestIfNotCanceled(); + } catch (IOException e) { + // If the request finished successfully don't exit the timeout yet. Reading the body also + // needs to be considered and the body object will take care of exiting it. See + // toCronetCallFactoryResponse() for details. + timeout.exit(); + responseCallback.onFailure(this, e); + } + } + + @Override + public Call clone() { + return motherFactory.newCall(request()); + } + + @Override + public void cancel() { + if (canceled.getAndSet(true)) { + // already canceled + return; + } + CronetRequestAndOkHttpResponse localConverted = convertedRequestAndResponse.get(); + if (localConverted != null) { + localConverted.getRequest().cancel(); + } // else the cancel signal will be picked up by the execute() / enqueue() methods. + } + + @Override + public boolean isExecuted() { + return executed.get(); + } + + @Override + public boolean isCanceled() { + return canceled.get(); + } + + @Override + public Timeout timeout() { + return timeout; + } + + private String toLoggableString() { + return "call to " + request().url().redact(); + } + + /** + * Verifies that the call can be executed and sets the state of the call to "being executed". + * + * @throws IllegalStateException if the request has already been executed. + * @throws IOException if the request was canceled + */ + private void evaluateExecutionPreconditions() throws IOException { + if (canceled.get()) { + throw new IOException("Can't execute canceled requests"); + } + checkState(!executed.getAndSet(true), "Already Executed"); + } + + private void startRequestIfNotCanceled() { + CronetRequestAndOkHttpResponse requestAndOkHttpResponse = convertedRequestAndResponse.get(); + checkState(requestAndOkHttpResponse != null, "convertedRequestAndResponse must be set!"); + + // There might be a race between the execution and cancellation + // evaluateExecutionPreconditions check didn't capture and cancel() might have missed that + // as well. Check once again that the request isn't canceled. + // This way, no matter how the instructions of the two threads are interleaved, we always end + // up with a serialized-like outcome (either cancel() was fully run before execute(), or vice + // versa). + + // Thread 1 (cancel() call) | Thread 2 (execute() call) + // ------------------------------------------------------------------------------- + // canceled = true | if (canceled) throw; + // convertedRequest?.cancel() | convertedRequest = convert(request) + // | if (canceled) convertedRequest.cancel() + if (canceled.get()) { + requestAndOkHttpResponse.getRequest().cancel(); + } else { + requestAndOkHttpResponse.getRequest().start(); + } + } + } + + private static Response toCronetCallFactoryResponse(CronetCall call, Response response) { + checkNotNull(response.body()); + + return response + .newBuilder() + .body( + new CronetTransportResponseBody(response.body()) { + @Override + void customCloseHook() { + call.timeout.exit(); + } + }) + .build(); + } + + public static final class Builder + extends RequestResponseConverterBasedBuilder { + private static final int DEFAULT_READ_WRITE_TIMEOUT_MILLIS = 10000; + + private int readTimeoutMillis = DEFAULT_READ_WRITE_TIMEOUT_MILLIS; + private int writeTimeoutMillis = DEFAULT_READ_WRITE_TIMEOUT_MILLIS; + private int callTimeoutMillis = 0; // No timeout + private ExecutorService callbackExecutorService = null; + + Builder(HttpEngine HttpEngine) { + super(HttpEngine, Builder.class); + } + + public Builder setReadTimeoutMillis(int readTimeoutMillis) { + checkArgument(readTimeoutMillis >= 0, "Read timeout mustn't be negative!"); + this.readTimeoutMillis = readTimeoutMillis; + return this; + } + + public Builder setWriteTimeoutMillis(int writeTimeoutMillis) { + checkArgument(writeTimeoutMillis >= 0, "Write timeout mustn't be negative!"); + this.writeTimeoutMillis = writeTimeoutMillis; + return this; + } + + public Builder setCallbackExecutorService(ExecutorService callbackExecutorService) { + checkNotNull(callbackExecutorService); + this.callbackExecutorService = callbackExecutorService; + return this; + } + + public Builder setCallTimeoutMillis(int callTimeoutMillis) { + checkArgument(callTimeoutMillis >= 0, "Call timeout mustn't be negative!"); + this.callTimeoutMillis = callTimeoutMillis; + + return this; + } + + @Override + CronetCallFactory build(RequestResponseConverter converter) { + ExecutorService localCallbackExecutorService; + if (callbackExecutorService == null) { + // Consistent with OkHttp impl + localCallbackExecutorService = Executors.newCachedThreadPool(); + } else { + localCallbackExecutorService = callbackExecutorService; + } + + return new CronetCallFactory( + converter, + localCallbackExecutorService, + readTimeoutMillis, + writeTimeoutMillis, + callTimeoutMillis); + } + } +} diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetInterceptor.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetInterceptor.java new file mode 100644 index 000000000000..4ebc06c8f9d8 --- /dev/null +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetInterceptor.java @@ -0,0 +1,166 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package okhttp3.android.httpengine; + +import android.util.Log; +import com.google.net.cronet.okhttptransport.RequestResponseConverter.CronetRequestAndOkHttpResponse; +import okhttp3.*; +import android.net.http.HttpEngine; +import android.net.http.UrlRequest; + +import java.io.IOException; +import java.util.Iterator; +import java.util.Map; +import java.util.Map.Entry; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ScheduledThreadPoolExecutor; + +import static com.google.common.base.Preconditions.checkNotNull; +import static java.util.concurrent.TimeUnit.MILLISECONDS; + +/** + * An OkHttp interceptor that redirects HTTP traffic to use Cronet instead of using the OkHttp + * network stack. + * + *

The interceptor should be used as the last application interceptor to ensure that all other + * interceptors are visited before sending the request on wire and after a response is returned. + * + *

The interceptor is a plug-and-play replacement for the OkHttp stack for the most part, + * however, there are some caveats to keep in mind: + * + *

    + *
  1. The entirety of OkHttp core is bypassed. This includes caching configuration and network + * interceptors. + *
  2. Some response fields are not being populated due to mismatches between Cronet's and + * OkHttp's architecture. TODO(danstahr): add a concrete list). + *
+ */ +public final class CronetInterceptor implements Interceptor, AutoCloseable { + private static final String TAG = "CronetInterceptor"; + + private static final int CANCELLATION_CHECK_INTERVAL_MILLIS = 500; + + private final RequestResponseConverter converter; + private final Map activeCalls = new ConcurrentHashMap<>(); + private final ScheduledExecutorService scheduledExecutor = new ScheduledThreadPoolExecutor(1); + + private CronetInterceptor(RequestResponseConverter converter) { + this.converter = checkNotNull(converter); + + // TODO(danstahr): There's no other way to know if the call is canceled but polling + // (https://github.com/square/okhttp/issues/7164). + ScheduledFuture unusedFuture = + scheduledExecutor.scheduleAtFixedRate( + () -> { + Iterator> activeCallsIterator = + activeCalls.entrySet().iterator(); + + while (activeCallsIterator.hasNext()) { + try { + Entry activeCall = activeCallsIterator.next(); + if (activeCall.getKey().isCanceled()) { + activeCallsIterator.remove(); + activeCall.getValue().cancel(); + } + } catch (RuntimeException e) { + Log.w(TAG, "Unable to propagate cancellation status", e); + } + } + }, + CANCELLATION_CHECK_INTERVAL_MILLIS, + CANCELLATION_CHECK_INTERVAL_MILLIS, + MILLISECONDS); + } + + @Override + public Response intercept(Chain chain) throws IOException { + if (chain.call().isCanceled()) { + throw new IOException("Canceled"); + } + + Request request = chain.request(); + + CronetRequestAndOkHttpResponse requestAndOkHttpResponse = + converter.convert(request, chain.readTimeoutMillis(), chain.writeTimeoutMillis()); + + activeCalls.put(chain.call(), requestAndOkHttpResponse.getRequest()); + + try { + requestAndOkHttpResponse.getRequest().start(); + return toInterceptorResponse(requestAndOkHttpResponse.getResponse(), chain.call()); + } catch (RuntimeException | IOException e) { + // If the response is retrieved successfully the caller is responsible for closing + // the response, which will remove it from the active calls map. + activeCalls.remove(chain.call()); + throw e; + } + } + + /** Creates a {@link CronetInterceptor} builder. */ + public static Builder newBuilder(HttpEngine HttpEngine) { + return new Builder(HttpEngine); + } + + @Override + public void close() { + scheduledExecutor.shutdown(); + } + + /** A builder for {@link CronetInterceptor}. */ + public static final class Builder + extends RequestResponseConverterBasedBuilder { + + Builder(HttpEngine HttpEngine) { + super(HttpEngine, Builder.class); + } + + /** Builds the interceptor. The same builder can be used to build multiple interceptors. */ + @Override + public CronetInterceptor build(RequestResponseConverter converter) { + return new CronetInterceptor(converter); + } + } + + private Response toInterceptorResponse(Response response, Call call) { + checkNotNull(response.body()); + + if (response.body() instanceof CronetInterceptorResponseBody) { + return response; + } + + return response + .newBuilder() + .body(new CronetInterceptorResponseBody(response.body(), call)) + .build(); + } + + private class CronetInterceptorResponseBody extends CronetTransportResponseBody { + private final Call call; + + private CronetInterceptorResponseBody(ResponseBody delegate, Call call) { + super(delegate); + this.call = call; + } + + @Override + void customCloseHook() { + activeCalls.remove(call); + } + } +} diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetTimeoutException.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetTimeoutException.java new file mode 100644 index 000000000000..c512cd2bce70 --- /dev/null +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetTimeoutException.java @@ -0,0 +1,21 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package okhttp3.android.httpengine; + +import java.io.IOException; + +public class CronetTimeoutException extends IOException {} diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetTransportResponseBody.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetTransportResponseBody.java new file mode 100644 index 000000000000..8e233ddef644 --- /dev/null +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetTransportResponseBody.java @@ -0,0 +1,55 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package okhttp3.android.httpengine; + +import androidx.annotation.Nullable; +import okhttp3.MediaType; +import okhttp3.ResponseBody; +import okio.BufferedSource; + +abstract class CronetTransportResponseBody extends ResponseBody { + + private final ResponseBody delegate; + + protected CronetTransportResponseBody(ResponseBody delegate) { + this.delegate = delegate; + } + + @Nullable + @Override + public final MediaType contentType() { + return delegate.contentType(); + } + + @Override + public final long contentLength() { + return delegate.contentLength(); + } + + @Override + public final BufferedSource source() { + return delegate.source(); + } + + @Override + public final void close() { + delegate.close(); + customCloseHook(); + } + + abstract void customCloseHook(); +} diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.java new file mode 100644 index 000000000000..9e705bf9ed85 --- /dev/null +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.java @@ -0,0 +1,312 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package okhttp3.android.httpengine; + +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.SettableFuture; +import okio.Buffer; +import okio.Source; +import okio.Timeout; +import android.net.http.CronetException; +import android.net.http.UrlRequest; +import android.net.http.UrlResponseInfo; + +import javax.annotation.Nullable; +import java.io.IOException; +import java.net.ProtocolException; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.atomic.AtomicBoolean; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; +import static java.util.concurrent.TimeUnit.MILLISECONDS; + +/** + * An implementation of Cronet's callback. This is the heart of the bridge and deals with most of + * the async-sync paradigm translation. + * + *

Translating the UrlResponseInfo is relatively straightforward as the entire object is + * available immediately and is relatively small, so it can easily fit in memory. + * + *

Translating the body is a bit more tricky because of the mismatch between OkHttp and Cronet + * designs. We invoke Cronet's read and wait for the result using synchronization primitives (see + * BodySource implementation). The implementation is assuming that there's always at most one read() + * request in flight (which is safe to assume), and relies on reasonable fairness of thread + * scheduling, especially when handling cancellations. + */ +class OkHttpBridgeRequestCallback extends UrlRequest.Callback { + + /** + * The byte buffer capacity for reading Cronet response bodies. Each response callback will + * allocate its own buffer of this size once the response starts being processed. + */ + private static final int CRONET_BYTE_BUFFER_CAPACITY = 32 * 1024; + + /** A bridge between Cronet's asynchronous callbacks and OkHttp's blocking stream-like reads. */ + private final SettableFuture bodySourceFuture = SettableFuture.create(); + + /** Signal whether the request is finished and the response has been fully read. */ + private final AtomicBoolean finished = new AtomicBoolean(false); + + /** Signal whether the request was canceled. */ + private final AtomicBoolean canceled = new AtomicBoolean(false); + + /** + * An internal, blocking, thread safe way of passing data between the callback methods and {@link + * #bodySourceFuture}. + * + *

Has a capacity of 2 - at most one slot for a read result and at most 1 slot for cancellation + * signal, this guarantees that all inserts are non blocking. + */ + private final BlockingQueue callbackResults = new ArrayBlockingQueue<>(2); + + /** The response headers. */ + private final SettableFuture headersFuture = SettableFuture.create(); + + /** The read timeout as specified by OkHttp. * */ + private final long readTimeoutMillis; + + /** The previous responses as reported to {@link #onRedirectReceived}, from oldest to newest. * */ + private final List urlResponseInfoChain = new ArrayList<>(); + + private final RedirectStrategy redirectStrategy; + + /** The request being processed. Set when the request is first seen by the callback. */ + private volatile UrlRequest request; + + OkHttpBridgeRequestCallback(long readTimeoutMillis, RedirectStrategy redirectStrategy) { + checkArgument(readTimeoutMillis >= 0); + + // So that we don't have to special case infinity. Int.MAX_VALUE is ~infinity for all practical + // use cases. + if (readTimeoutMillis == 0) { + this.readTimeoutMillis = Integer.MAX_VALUE; + } else { + this.readTimeoutMillis = readTimeoutMillis; + } + this.redirectStrategy = redirectStrategy; + } + + /** Returns the {@link UrlResponseInfo} for the request associated with this callback. */ + ListenableFuture getUrlResponseInfo() { + return headersFuture; + } + + /** + * Returns the OkHttp {@link Source} for the request associated with this callback. + * + *

Note that retrieving data from the {@code Source} instance might block further as the + * response body is streamed. + */ + ListenableFuture getBodySource() { + return bodySourceFuture; + } + + List getUrlResponseInfoChain() { + return Collections.unmodifiableList(urlResponseInfoChain); + } + + @Override + public void onRedirectReceived( + UrlRequest urlRequest, UrlResponseInfo urlResponseInfo, String nextUrl) { + // We shouldn't follow redirects - pass the given UrlResponseInfo as the ultimate result + if (!redirectStrategy.followRedirects()) { + checkState(headersFuture.set(urlResponseInfo)); + // Note: This might not match the content length headers but we have no way of accessing + // the actual body with current Cronet's APIs (see RedirectStrategy). + checkState(bodySourceFuture.set(new Buffer())); + urlRequest.cancel(); + return; + } + + // We should follow redirects and we haven't hit the cap yet + urlResponseInfoChain.add(urlResponseInfo); + if (urlResponseInfo.getUrlChain().size() <= redirectStrategy.numberOfRedirectsToFollow()) { + urlRequest.followRedirect(); + return; + } + + // Cap reached - cancel the request and fail. Exception crafted to match OkHttp. + urlRequest.cancel(); + + IOException e = + new ProtocolException( + "Too many follow-up requests: " + (redirectStrategy.numberOfRedirectsToFollow() + 1)); + headersFuture.setException(e); + bodySourceFuture.setException(e); + } + + @Override + public void onResponseStarted(UrlRequest urlRequest, UrlResponseInfo urlResponseInfo) { + request = urlRequest; + + checkState(headersFuture.set(urlResponseInfo)); + checkState(bodySourceFuture.set(new CronetBodySource())); + } + + @Override + public void onReadCompleted( + UrlRequest urlRequest, UrlResponseInfo urlResponseInfo, ByteBuffer byteBuffer) { + callbackResults.add(new CallbackResult(CallbackStep.ON_READ_COMPLETED, byteBuffer, null)); + } + + @Override + public void onSucceeded(UrlRequest urlRequest, UrlResponseInfo urlResponseInfo) { + callbackResults.add(new CallbackResult(CallbackStep.ON_SUCCESS, null, null)); + } + + @Override + public void onFailed(UrlRequest urlRequest, UrlResponseInfo urlResponseInfo, CronetException e) { + // If this was called before we start reading the body, the exception will + // propagate in the future providing headers and the body wrapper. + if (headersFuture.setException(e) && bodySourceFuture.setException(e)) { + return; + } + + // If this was called as a reaction to a read() call, the read result will propagate + // the exception. + callbackResults.add(new CallbackResult(CallbackStep.ON_FAILED, null, e)); + } + + @Override + public void onCanceled(UrlRequest urlRequest, UrlResponseInfo responseInfo) { + canceled.set(true); + callbackResults.add(new CallbackResult(CallbackStep.ON_CANCELED, null, null)); + + // If there's nobody listening it's possible that the cancellation happened before we even + // received anything from the server. In that case inform the thread that's awaiting server + // response about the cancellation as well. This becomes a no-op if the futures + // were already set. + IOException e = new IOException("The request was canceled!"); + headersFuture.setException(e); + bodySourceFuture.setException(e); + } + + private class CronetBodySource implements Source { + + private ByteBuffer buffer = ByteBuffer.allocateDirect(CRONET_BYTE_BUFFER_CAPACITY); + + /** Whether the close() method has been called. */ + private volatile boolean closed = false; + + @Override + public long read(Buffer sink, long byteCount) throws IOException { + if (canceled.get()) { + throw new IOException("The request was canceled!"); + } + + // Using IAE instead of NPE (checkNotNull) for okio.RealBufferedSource consistency + checkArgument(sink != null, "sink == null"); + checkArgument(byteCount >= 0, "byteCount < 0: %s", byteCount); + checkState(!closed, "closed"); + + if (finished.get()) { + return -1; + } + + if (byteCount < buffer.limit()) { + buffer.limit((int) byteCount); + } + + request.read(buffer); + + CallbackResult result; + try { + result = callbackResults.poll(readTimeoutMillis, MILLISECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + result = null; + } + + if (result == null) { + // Either readResult.poll() was interrupted or it timed out. + request.cancel(); + throw new CronetTimeoutException(); + } + + switch (result.callbackStep) { + // We null the buffer in final statuses to allow fast GC of the buffer even if the callback + // is still in use. + case ON_FAILED: + finished.set(true); + buffer = null; + throw new IOException(result.exception); + case ON_SUCCESS: + finished.set(true); + buffer = null; + return -1; + case ON_CANCELED: + // The canceled flag is already set by the onCanceled method + // so not setting it here. + + buffer = null; + throw new IOException("The request was canceled!"); + case ON_READ_COMPLETED: + result.buffer.flip(); + int bytesWritten = sink.write(result.buffer); + result.buffer.clear(); + return bytesWritten; + } + + throw new AssertionError("The switch block above is exhaustive!"); + } + + @Override + public Timeout timeout() { + // TODO(danstahr): This should likely respect the OkHttp timeout somehow + return Timeout.NONE; + } + + @Override + public void close() { + if (closed) { + return; + } + closed = true; + if (!finished.get()) { + request.cancel(); + } + } + } + + private static class CallbackResult { + private final CallbackStep callbackStep; + @Nullable private final ByteBuffer buffer; + @Nullable private final CronetException exception; + + private CallbackResult( + CallbackStep callbackStep, + @Nullable ByteBuffer buffer, + @Nullable CronetException exception) { + this.callbackStep = callbackStep; + this.buffer = buffer; + this.exception = exception; + } + } + + private enum CallbackStep { + ON_READ_COMPLETED, + ON_SUCCESS, + ON_FAILED, + ON_CANCELED + } +} diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.java new file mode 100644 index 000000000000..b78995fa33f5 --- /dev/null +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.java @@ -0,0 +1,89 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package okhttp3.android.httpengine; + +/** Defines a redirect strategy for the Cronet OkHttp transport layer. */ +public abstract class RedirectStrategy { + + /** The default number of redirects to follow. Should be less than the Chromium wide safeguard. */ + private static final int DEFAULT_REDIRECTS = 16; + + /** + * Returns whether redirects should be followed at all. If set to false, the redirect response + * will be returned. + */ + abstract boolean followRedirects(); + + /** + * Returns the maximum number of redirects to follow. If more redirects are attempted an exception + * should be thrown by the component handling the request. Shouldn't be called at all if {@link + * #followRedirects()} return false. + */ + abstract int numberOfRedirectsToFollow(); + + /** + * Returns a strategy which will not follow redirects. + * + *

Note that because of Cronet's limitations + * (https://developer.android.com/guide/topics/connectivity/cronet/lifecycle#overview) it is + * impossible to retrieve the body of a redirect response. As a result, a dummy empty body will + * always be provided. + */ + public static RedirectStrategy withoutRedirects() { + return WithoutRedirectsHolder.INSTANCE; + } + + /** + * Returns a strategy which will follow redirects up to {@link #DEFAULT_REDIRECTS} times. If more + * redirects are attempted an exception is thrown. + */ + public static RedirectStrategy defaultStrategy() { + return DefaultRedirectsHolder.INSTANCE; + } + + private static class WithoutRedirectsHolder { + private static final RedirectStrategy INSTANCE = + new RedirectStrategy() { + @Override + boolean followRedirects() { + return false; + } + + @Override + int numberOfRedirectsToFollow() { + throw new UnsupportedOperationException(); + } + }; + } + + private static class DefaultRedirectsHolder { + private static final RedirectStrategy INSTANCE = + new RedirectStrategy() { + @Override + boolean followRedirects() { + return true; + } + + @Override + int numberOfRedirectsToFollow() { + return DEFAULT_REDIRECTS; + } + }; + } + + private RedirectStrategy() {} +} diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.java new file mode 100644 index 000000000000..26fd8439b983 --- /dev/null +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.java @@ -0,0 +1,28 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package okhttp3.android.httpengine; + +import okhttp3.RequestBody; +import android.net.http.UploadDataProvider; + +import java.io.IOException; + +/** An interface for classes converting from OkHttp to Cronet request bodies. */ +interface RequestBodyConverter { + UploadDataProvider convertRequestBody(RequestBody requestBody, int writeTimeoutMillis) + throws IOException; +} diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.java new file mode 100644 index 000000000000..7f71f3adc59d --- /dev/null +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.java @@ -0,0 +1,332 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package okhttp3.android.httpengine; + +import androidx.annotation.VisibleForTesting; +import com.google.common.base.Verify; +import com.google.common.util.concurrent.*; +import com.google.net.cronet.okhttptransport.UploadBodyDataBroker.ReadResult; +import okhttp3.RequestBody; +import okio.Buffer; +import okio.BufferedSink; +import okio.Okio; +import android.net.http.UploadDataProvider; +import android.net.http.UploadDataSink; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.TimeoutException; + +import static java.util.concurrent.TimeUnit.MILLISECONDS; + +final class RequestBodyConverterImpl implements RequestBodyConverter { + + private static final long IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES = 1024 * 1024; + + private final InMemoryRequestBodyConverter inMemoryRequestBodyConverter; + private final StreamingRequestBodyConverter streamingRequestBodyConverter; + + RequestBodyConverterImpl( + InMemoryRequestBodyConverter inMemoryConverter, + StreamingRequestBodyConverter streamingConverter) { + this.inMemoryRequestBodyConverter = inMemoryConverter; + this.streamingRequestBodyConverter = streamingConverter; + } + + static RequestBodyConverterImpl create(ExecutorService bodyReaderExecutor) { + return new RequestBodyConverterImpl( + new InMemoryRequestBodyConverter(), new StreamingRequestBodyConverter(bodyReaderExecutor)); + } + + @Override + public UploadDataProvider convertRequestBody(RequestBody requestBody, int writeTimeoutMillis) + throws IOException { + long contentLength = requestBody.contentLength(); + if (contentLength == -1 || contentLength > IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES) { + return streamingRequestBodyConverter.convertRequestBody(requestBody, writeTimeoutMillis); + } else { + return inMemoryRequestBodyConverter.convertRequestBody(requestBody, writeTimeoutMillis); + } + } + + /** + * Implementation of {@link RequestBodyConverter} that doesn't need to hold the entire request + * body in memory. + * + *

+ * + *

    + *
  1. {@link RequestBody#writeTo(BufferedSink)} is invoked on the body, but the sink doesn't + * accept any data + *
  2. A call to {@link UploadDataProvider#read(UploadDataSink, ByteBuffer)} unblocks the sink, + * which accepts a part of the body (size depends on the buffer's capacity), then blocks + * again. Buffer is sent to Cronet. + *
+ * + * This is repeated until the entire body has been read. + */ + @VisibleForTesting + static final class StreamingRequestBodyConverter implements RequestBodyConverter { + + private final ExecutorService readerExecutor; + + StreamingRequestBodyConverter(ExecutorService readerExecutor) { + this.readerExecutor = readerExecutor; + } + + @Override + public UploadDataProvider convertRequestBody(RequestBody requestBody, int writeTimeoutMillis) { + return new StreamingUploadDataProvider( + requestBody, new UploadBodyDataBroker(), readerExecutor, writeTimeoutMillis); + } + + private static class StreamingUploadDataProvider extends UploadDataProvider { + private final RequestBody okHttpRequestBody; + private final UploadBodyDataBroker broker; + private final ListeningExecutorService readTaskExecutor; + private final long writeTimeoutMillis; + + /** The future for the task that reads the OkHttp request body in the background. */ + private ListenableFuture readTaskFuture; + + /** The number of bytes we read from the OkHttp body thus far. */ + private long totalBytesReadFromOkHttp; + + private StreamingUploadDataProvider( + RequestBody okHttpRequestBody, + UploadBodyDataBroker broker, + ExecutorService readTaskExecutor, + long writeTimeoutMillis) { + this.okHttpRequestBody = okHttpRequestBody; + this.broker = broker; + if (readTaskExecutor instanceof ListeningExecutorService) { + this.readTaskExecutor = (ListeningExecutorService) readTaskExecutor; + } else { + this.readTaskExecutor = MoreExecutors.listeningDecorator(readTaskExecutor); + } + + // So that we don't have to special case infinity. Int.MAX_VALUE is ~infinity for all + // practical use cases. + this.writeTimeoutMillis = writeTimeoutMillis == 0 ? Integer.MAX_VALUE : writeTimeoutMillis; + } + + @Override + public long getLength() throws IOException { + return okHttpRequestBody.contentLength(); + } + + @Override + public void read(UploadDataSink uploadDataSink, ByteBuffer byteBuffer) throws IOException { + ensureReadTaskStarted(); + + if (getLength() == -1) { + readUnknownBodyLength(uploadDataSink, byteBuffer); + } else { + readKnownBodyLength(uploadDataSink, byteBuffer); + } + } + + private void readKnownBodyLength(UploadDataSink uploadDataSink, ByteBuffer byteBuffer) + throws IOException { + try { + UploadBodyDataBroker.ReadResult readResult = readFromOkHttp(byteBuffer); + + if (totalBytesReadFromOkHttp > getLength()) { + throw prepareBodyTooLongException(getLength(), totalBytesReadFromOkHttp); + } + + if (totalBytesReadFromOkHttp < getLength()) { + switch (readResult) { + case SUCCESS: + uploadDataSink.onReadSucceeded(false); + break; + case END_OF_BODY: + throw new IOException("The source has been exhausted but we expected more data!"); + } + return; + } + // Else we're handling what's supposed to be the last chunk + handleLastBodyRead(uploadDataSink, byteBuffer); + + } catch (TimeoutException | ExecutionException e) { + readTaskFuture.cancel(true); + uploadDataSink.onReadError(new IOException(e)); + } + } + + /** + * The last body read is special for fixed length bodies - if Cronet receives exactly the + * right amount of data it won't ask for more, even if there is more data in the stream. As a + * result, when we read the advertised number of bytes, we need to make sure that the stream + * is indeed finished. + */ + private void handleLastBodyRead(UploadDataSink uploadDataSink, ByteBuffer filledByteBuffer) + throws IOException, TimeoutException, ExecutionException { + // We reuse the same buffer for the END_OF_DATA read (it should be non-destructive and if + // it overwrites what's in there we don't mind as that's an error anyway). We just need + // to make sure we restore the original position afterwards. We don't use mark() / reset() + // as the mark position can be invalidated by limit manipulation. + int bufferPosition = filledByteBuffer.position(); + filledByteBuffer.position(0); + + UploadBodyDataBroker.ReadResult readResult = readFromOkHttp(filledByteBuffer); + + if (!readResult.equals(ReadResult.END_OF_BODY)) { + throw prepareBodyTooLongException(getLength(), totalBytesReadFromOkHttp); + } + + Verify.verify( + filledByteBuffer.position() == 0, + "END_OF_BODY reads shouldn't write anything to the buffer"); + + // revert the position change + filledByteBuffer.position(bufferPosition); + + uploadDataSink.onReadSucceeded(false); + } + + private void readUnknownBodyLength(UploadDataSink uploadDataSink, ByteBuffer byteBuffer) { + try { + UploadBodyDataBroker.ReadResult readResult = readFromOkHttp(byteBuffer); + uploadDataSink.onReadSucceeded(readResult.equals(ReadResult.END_OF_BODY)); + } catch (TimeoutException | ExecutionException e) { + readTaskFuture.cancel(true); + uploadDataSink.onReadError(new IOException(e)); + } + } + + private void ensureReadTaskStarted() { + // We don't expect concurrent calls so a simple flag is sufficient + if (readTaskFuture == null) { + readTaskFuture = + readTaskExecutor.submit( + (Callable) + () -> { + BufferedSink bufferedSink = Okio.buffer(broker); + okHttpRequestBody.writeTo(bufferedSink); + bufferedSink.flush(); + broker.handleEndOfStreamSignal(); + return null; + }); + + Futures.addCallback( + readTaskFuture, + new FutureCallback() { + @Override + public void onSuccess(Object result) {} + + @Override + public void onFailure(Throwable t) { + broker.setBackgroundReadError(t); + } + }, + MoreExecutors.directExecutor()); + } + } + + private ReadResult readFromOkHttp(ByteBuffer byteBuffer) + throws TimeoutException, ExecutionException { + int positionBeforeRead = byteBuffer.position(); + UploadBodyDataBroker.ReadResult readResult = + Uninterruptibles.getUninterruptibly( + broker.enqueueBodyRead(byteBuffer), writeTimeoutMillis, MILLISECONDS); + int bytesRead = byteBuffer.position() - positionBeforeRead; + totalBytesReadFromOkHttp += bytesRead; + return readResult; + } + + private static IOException prepareBodyTooLongException( + long expectedLength, long minActualLength) { + return new IOException( + "Expected " + expectedLength + " bytes but got at least " + minActualLength); + } + + @Override + public void rewind(UploadDataSink uploadDataSink) { + // TODO(danstahr): OkHttp 4 can use isOneShot flag here and rewind safely. + uploadDataSink.onRewindError(new UnsupportedOperationException("Rewind is not supported!")); + } + } + } + + /** + * Converts OkHttp's {@link RequestBody} to Cronet's {@link UploadDataProvider} by materializing + * the body in memory first. + * + *

This strategy shouldn't be used for large requests (and for requests with uncapped length) + * to avoid OOM issues. + */ + @VisibleForTesting + static final class InMemoryRequestBodyConverter implements RequestBodyConverter { + + @Override + public UploadDataProvider convertRequestBody(RequestBody requestBody, int writeTimeoutMillis) + throws IOException { + + // content length is immutable by contract + long length = requestBody.contentLength(); + if (length < 0 || length > IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES) { + throw new IOException( + "Expected definite length less than " + + IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES + + "but got " + + length); + } + + return new UploadDataProvider() { + private volatile boolean isMaterialized = false; + private final Buffer materializedBody = new Buffer(); + + @Override + public long getLength() { + return length; + } + + @Override + public void read(UploadDataSink uploadDataSink, ByteBuffer byteBuffer) throws IOException { + // We're not expecting any concurrent calls here so a simple flag should be sufficient. + if (!isMaterialized) { + requestBody.writeTo(materializedBody); + materializedBody.flush(); + isMaterialized = true; + long reportedLength = getLength(); + long actualLength = materializedBody.size(); + if (actualLength != reportedLength) { + throw new IOException( + "Expected " + reportedLength + " bytes but got " + actualLength); + } + } + if (materializedBody.read(byteBuffer) == -1) { + // This should never happen - for known body length we shouldn't be called at all + // if there's no more data to read. + throw new IllegalStateException("The source has been exhausted but we expected more!"); + } + uploadDataSink.onReadSucceeded(false); + } + + @Override + public void rewind(UploadDataSink uploadDataSink) { + // TODO(danstahr): OkHttp 4 can use isOneShot flag here and rewind safely. + uploadDataSink.onRewindError(new UnsupportedOperationException()); + } + }; + } + } +} diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.java new file mode 100644 index 000000000000..b873e45c4144 --- /dev/null +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.java @@ -0,0 +1,164 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package okhttp3.android.httpengine; + +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; +import okhttp3.Request; +import okhttp3.RequestBody; +import okhttp3.Response; +import android.net.http.HttpEngine; +import android.net.http.UrlRequest; + +import java.io.IOException; +import java.util.concurrent.Executor; +import java.util.concurrent.Future; + +/** Converts OkHttp requests to Cronet requests. */ +final class RequestResponseConverter { + private static final String CONTENT_LENGTH_HEADER_NAME = "Content-Length"; + private static final String CONTENT_TYPE_HEADER_NAME = "Content-Type"; + private static final String CONTENT_TYPE_HEADER_DEFAULT_VALUE = "application/octet-stream"; + + private final HttpEngine HttpEngine; + private final Executor uploadDataProviderExecutor; + private final ResponseConverter responseConverter; + private final RequestBodyConverter requestBodyConverter; + private final RedirectStrategy redirectStrategy; + + RequestResponseConverter( + HttpEngine HttpEngine, + Executor uploadDataProviderExecutor, + RequestBodyConverter requestBodyConverter, + ResponseConverter responseConverter, + RedirectStrategy redirectStrategy) { + this.HttpEngine = HttpEngine; + this.uploadDataProviderExecutor = uploadDataProviderExecutor; + this.requestBodyConverter = requestBodyConverter; + this.responseConverter = responseConverter; + this.redirectStrategy = redirectStrategy; + } + + /** + * Converts OkHttp's {@link Request} to a corresponding Cronet's {@link UrlRequest}. + * + *

Since Cronet doesn't have a notion of a Response, which is handled entirely from the + * callbacks, this method also returns a {@link Future} like object the + * caller should use to obtain the matching {@link Response} for the given request. For example: + * + * 

+   *   RequestResponseConverter converter = ...
+   *   CronetRequestAndOkHttpResponse reqResp = converter.convert(okHttpRequest);
+   *   reqResp.getRequest.start();
+   *
+   *   // Will block until status code, headers... are available
+   *   Response okHttpResponse = reqResp.getResponse();
+   *
+   *   // use OkHttp Response as usual
+   *
+ */ + CronetRequestAndOkHttpResponse convert( + Request okHttpRequest, int readTimeoutMillis, int writeTimeoutMillis) throws IOException { + + OkHttpBridgeRequestCallback callback = + new OkHttpBridgeRequestCallback(readTimeoutMillis, redirectStrategy); + + // The OkHttp request callback methods are lightweight, the heavy lifting is done by OkHttp / + // app owned threads. Use a direct executor to avoid extra thread hops. + UrlRequest.Builder builder = + HttpEngine + .newUrlRequestBuilder( + okHttpRequest.url().toString(), callback, MoreExecutors.directExecutor()) + .allowDirectExecutor(); + + builder.setHttpMethod(okHttpRequest.method()); + + for (int i = 0; i < okHttpRequest.headers().size(); i++) { + builder.addHeader(okHttpRequest.headers().name(i), okHttpRequest.headers().value(i)); + } + + RequestBody body = okHttpRequest.body(); + + if (body != null) { + if (okHttpRequest.header(CONTENT_LENGTH_HEADER_NAME) == null && body.contentLength() != -1) { + builder.addHeader(CONTENT_LENGTH_HEADER_NAME, String.valueOf(body.contentLength())); + } + + if (body.contentLength() != 0) { + if (body.contentType() != null) { + builder.addHeader(CONTENT_TYPE_HEADER_NAME, body.contentType().toString()); + } else if (okHttpRequest.header(CONTENT_TYPE_HEADER_NAME) == null) { + // Cronet always requires content-type to be present when a body is present. Use a generic + // value if one isn't provided. + builder.addHeader(CONTENT_TYPE_HEADER_NAME, CONTENT_TYPE_HEADER_DEFAULT_VALUE); + } // else use the header + + builder.setUploadDataProvider( + requestBodyConverter.convertRequestBody(body, writeTimeoutMillis), + uploadDataProviderExecutor); + } + } + + return new CronetRequestAndOkHttpResponse( + builder.build(), createResponseSupplier(okHttpRequest, callback)); + } + + private ResponseSupplier createResponseSupplier( + Request request, OkHttpBridgeRequestCallback callback) { + return new ResponseSupplier() { + @Override + public Response getResponse() throws IOException { + return responseConverter.toResponse(request, callback); + } + + @Override + public ListenableFuture getResponseFuture() { + return responseConverter.toResponseAsync(request, callback); + } + }; + } + + /** A {@link Future} like holder for OkHttp's {@link Response}. */ + private interface ResponseSupplier { + Response getResponse() throws IOException; + + ListenableFuture getResponseFuture(); + } + + /** A simple data class for bundling Cronet request and OkHttp response. */ + static final class CronetRequestAndOkHttpResponse { + private final UrlRequest request; + private final ResponseSupplier responseSupplier; + + CronetRequestAndOkHttpResponse(UrlRequest request, ResponseSupplier responseSupplier) { + this.request = request; + this.responseSupplier = responseSupplier; + } + + public UrlRequest getRequest() { + return request; + } + + public Response getResponse() throws IOException { + return responseSupplier.getResponse(); + } + + public ListenableFuture getResponseAsync() { + return responseSupplier.getResponseFuture(); + } + } +} diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.java new file mode 100644 index 000000000000..06c0e728fd23 --- /dev/null +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.java @@ -0,0 +1,90 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package okhttp3.android.httpengine; + +import android.net.http.HttpEngine; +import android.net.http.UploadDataProvider; + +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; + +abstract class RequestResponseConverterBasedBuilder< + SubBuilderT extends RequestResponseConverterBasedBuilder, + ObjectBeingBuiltT> { + private static final int DEFAULT_THREAD_POOL_SIZE = 4; + + private final HttpEngine HttpEngine; + private int uploadDataProviderExecutorSize = DEFAULT_THREAD_POOL_SIZE; + // Not setting the default straight away to lazy initialize the object if it ends up not being + // used. + private RedirectStrategy redirectStrategy = null; + private final SubBuilderT castedThis; + + @SuppressWarnings("unchecked") // checked as a precondition + RequestResponseConverterBasedBuilder(HttpEngine HttpEngine, Class clazz) { + this.HttpEngine = checkNotNull(HttpEngine); + checkArgument(this.getClass().equals(clazz)); + castedThis = (SubBuilderT) this; + } + + /** + * Sets the size of upload data provider executor. The same executor is used for all upload data + * providers within the interceptor. + * + * @see android.net.http.UrlRequest.Builder#setUploadDataProvider(UploadDataProvider, Executor) + */ + public final SubBuilderT setUploadDataProviderExecutorSize(int size) { + checkArgument(size > 0, "The number of threads must be positive!"); + uploadDataProviderExecutorSize = size; + return castedThis; + } + + /** + * Sets the strategy for following redirects. + * + *

Note that the Cronet (i.e. Chromium) wide safeguards will still apply if one attempts to + * follow redirects too many times. + */ + public final SubBuilderT setRedirectStrategy(RedirectStrategy redirectStrategy) { + checkNotNull(redirectStrategy); + this.redirectStrategy = redirectStrategy; + return castedThis; + } + + abstract ObjectBeingBuiltT build(RequestResponseConverter converter); + + public final ObjectBeingBuiltT build() { + if (redirectStrategy == null) { + redirectStrategy = RedirectStrategy.defaultStrategy(); + } + + RequestResponseConverter converter = + new RequestResponseConverter( + HttpEngine, + Executors.newFixedThreadPool(uploadDataProviderExecutorSize), + // There must always be enough executors to blocking-read the OkHttp request bodies + // otherwise deadlocks can occur. + RequestBodyConverterImpl.create(Executors.newCachedThreadPool()), + new ResponseConverter(), + redirectStrategy); + + return build(converter); + } +} diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.java new file mode 100644 index 000000000000..8478d8e4b97c --- /dev/null +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.java @@ -0,0 +1,260 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package okhttp3.android.httpengine; + +import androidx.annotation.NonNull; +import com.google.common.base.Ascii; +import com.google.common.base.Splitter; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; +import com.google.common.util.concurrent.Uninterruptibles; +import okhttp3.*; +import okio.Okio; +import okio.Source; +import android.net.http.UrlResponseInfo; + +import javax.annotation.Nullable; +import java.io.IOException; +import java.net.ProtocolException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; + +/** + * Converts Cronet's responses (or, more precisely, its chunks as they come from Cronet's {@link + * android.net.http.UrlRequest.Callback}), to OkHttp's {@link Response}. + */ +final class ResponseConverter { + private static final String CONTENT_LENGTH_HEADER_NAME = "Content-Length"; + private static final String CONTENT_TYPE_HEADER_NAME = "Content-Type"; + private static final String CONTENT_ENCODING_HEADER_NAME = "Content-Encoding"; + + // https://source.chromium.org/search?q=symbol:FilterSourceStream::ParseEncodingType%20f:cc + private static final ImmutableSet ENCODINGS_HANDLED_BY_CRONET = + ImmutableSet.of("br", "deflate", "gzip", "x-gzip"); + + private static final Splitter COMMA_SPLITTER = Splitter.on(',').trimResults().omitEmptyStrings(); + + /** + * Creates an OkHttp's Response from the OkHttp-Cronet bridging callback. + * + *

As long as the callback's {@code UrlResponseInfo} is available this method is non-blocking. + * However, this method doesn't fetch the entire body response. As a result, subsequent calls to + * the result's {@link Response#body()} methods might block further. + */ + Response toResponse(Request request, OkHttpBridgeRequestCallback callback) throws IOException { + UrlResponseInfo cronetResponseInfo = getFutureValue(callback.getUrlResponseInfo()); + Response.Builder responseBuilder = + createResponse(request, cronetResponseInfo, getFutureValue(callback.getBodySource())); + + List redirectResponseInfos = callback.getUrlResponseInfoChain(); + List urlChain = cronetResponseInfo.getUrlChain(); + + if (!redirectResponseInfos.isEmpty()) { + checkArgument( + urlChain.size() == redirectResponseInfos.size() + 1, + "The number of redirects should be consistent across URLs and headers!"); + + Response priorResponse = null; + for (int i = 0; i < redirectResponseInfos.size(); i++) { + Request redirectedRequest = request.newBuilder().url(urlChain.get(i)).build(); + priorResponse = + createResponse(redirectedRequest, redirectResponseInfos.get(i), null) + .priorResponse(priorResponse) + .build(); + } + + responseBuilder + .request(request.newBuilder().url(Iterables.getLast(urlChain)).build()) + .priorResponse(priorResponse); + } + + return responseBuilder.build(); + } + + ListenableFuture toResponseAsync( + Request request, OkHttpBridgeRequestCallback callback) { + return Futures.whenAllComplete(callback.getUrlResponseInfo(), callback.getBodySource()) + .call(() -> toResponse(request, callback), MoreExecutors.directExecutor()); + } + + private static Response.Builder createResponse( + Request request, UrlResponseInfo cronetResponseInfo, @Nullable Source bodySource) + throws IOException { + + Response.Builder responseBuilder = new Response.Builder(); + + @Nullable String contentType = getLastHeaderValue(CONTENT_TYPE_HEADER_NAME, cronetResponseInfo); + + // If all content encodings are those known to Cronet natively, Cronet decodes the body stream. + // Otherwise, it's sent to the callbacks verbatim. For consistency with OkHttp, we only leave + // the Content-Encoding headers if Cronet didn't decode the request. Similarly, for consistency, + // we strip the Content-Length header of decoded responses. + + @Nullable String contentLengthString = null; + + // Theoretically, the content encodings can be scattered across multiple comma separated + // Content-Encoding headers. This list contains individual encodings. + List contentEncodingItems = new ArrayList<>(); + + for (String contentEncodingHeaderValue : + getOrDefault( + cronetResponseInfo.getAllHeaders(), + CONTENT_ENCODING_HEADER_NAME, + Collections.emptyList())) { + Iterables.addAll(contentEncodingItems, COMMA_SPLITTER.split(contentEncodingHeaderValue)); + } + + boolean keepEncodingAffectedHeaders = + contentEncodingItems.isEmpty() + || !ENCODINGS_HANDLED_BY_CRONET.containsAll(contentEncodingItems); + + if (keepEncodingAffectedHeaders) { + contentLengthString = getLastHeaderValue(CONTENT_LENGTH_HEADER_NAME, cronetResponseInfo); + } + + ResponseBody responseBody = null; + if (bodySource != null) { + responseBody = + createResponseBody( + request, + cronetResponseInfo.getHttpStatusCode(), + contentType, + contentLengthString, + bodySource); + } + + responseBuilder + .request(request) + .code(cronetResponseInfo.getHttpStatusCode()) + .message(cronetResponseInfo.getHttpStatusText()) + .protocol(convertProtocol(cronetResponseInfo.getNegotiatedProtocol())) + .body(responseBody); + + for (Map.Entry header : cronetResponseInfo.getAllHeadersAsList()) { + boolean copyHeader = true; + if (!keepEncodingAffectedHeaders) { + if (Ascii.equalsIgnoreCase(header.getKey(), CONTENT_LENGTH_HEADER_NAME) + || Ascii.equalsIgnoreCase(header.getKey(), CONTENT_ENCODING_HEADER_NAME)) { + copyHeader = false; + } + } + if (copyHeader) { + responseBuilder.addHeader(header.getKey(), header.getValue()); + } + } + + return responseBuilder; + } + + /** + * Creates an OkHttp's ResponseBody from the OkHttp-Cronet bridging callback. + * + *

As long as the callback's {@code UrlResponseInfo} is available this method is non-blocking. + * However, this method doesn't fetch the entire body response. As a result, subsequent calls to + * {@link ResponseBody} methods might block further to fetch parts of the body. + */ + private static ResponseBody createResponseBody( + Request request, + int httpStatusCode, + @Nullable String contentType, + @Nullable String contentLengthString, + Source bodySource) + throws IOException { + + long contentLength; + + // Ignore content-length header for HEAD requests (consistency with OkHttp) + if (request.method().equals("HEAD")) { + contentLength = 0; + } else { + try { + contentLength = contentLengthString != null ? Long.parseLong(contentLengthString) : -1; + } catch (NumberFormatException e) { + // TODO(danstahr): add logging + contentLength = -1; + } + } + + // Check for absence of body in No Content / Reset Content responses (OkHttp consistency) + if ((httpStatusCode == 204 || httpStatusCode == 205) && contentLength > 0) { + throw new ProtocolException( + "HTTP " + httpStatusCode + " had non-zero Content-Length: " + contentLengthString); + } + + return ResponseBody.create( + contentType != null ? MediaType.parse(contentType) : null, + contentLength, + Okio.buffer(bodySource)); + } + + /** Converts Cronet's negotiated protocol string to OkHttp's {@link Protocol}. */ + private static Protocol convertProtocol(String negotiatedProtocol) { + // See + // https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids + if (negotiatedProtocol.contains("quic")) { + return Protocol.QUIC; + } else if (negotiatedProtocol.contains("h3")) { + // TODO(danstahr): Should be h3 for newer OkHttp + return Protocol.QUIC; + } else if (negotiatedProtocol.contains("spdy")) { + return Protocol.HTTP_2; + } else if (negotiatedProtocol.contains("h2")) { + return Protocol.HTTP_2; + } else if (negotiatedProtocol.contains("http/1.1")) { + return Protocol.HTTP_1_1; + } + + return Protocol.HTTP_1_0; + } + + /** Returns the last header value for the given name, or null if the header isn't present. */ + @Nullable + private static String getLastHeaderValue(String name, UrlResponseInfo responseInfo) { + List headers = responseInfo.getAllHeaders().get(name); + if (headers == null || headers.isEmpty()) { + return null; + } + return Iterables.getLast(headers); + } + + private static T getFutureValue(Future future) throws IOException { + try { + return Uninterruptibles.getUninterruptibly(future); + } catch (ExecutionException e) { + throw new IOException(e); + } + } + + private static V getOrDefault(Map map, K key, @NonNull V defaultValue) { + V value = map.get(key); + if (value == null) { + return checkNotNull(defaultValue); + } else { + return value; + } + } +} diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.java new file mode 100644 index 000000000000..0b31abcd4d26 --- /dev/null +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.java @@ -0,0 +1,178 @@ +/* + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package okhttp3.android.httpengine; + +import android.util.Pair; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.SettableFuture; +import okio.Buffer; +import okio.Sink; +import okio.Timeout; +import android.net.http.UploadDataSink; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.Future; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import static com.google.common.base.Preconditions.checkState; + +final class UploadBodyDataBroker implements Sink { + + /** + * The read request calls to {@link android.net.http.UploadDataProvider#read(UploadDataSink, + * ByteBuffer)} associated with this broker that we haven't started handling. + * + *

We don't expect more than one parallel read call for a single request body provider. + */ + private final BlockingQueue>> pendingRead = + new ArrayBlockingQueue<>(1); + + /** + * Whether the sink has been closed. + * + *

Calling close() has no practical use but we check that nobody tries to write to the sink + * after closing it, which is an indication of misuse. + */ + private final AtomicBoolean isClosed = new AtomicBoolean(); + + /** + * The exception thrown by the body reading background thread, if any. The exception will be + * rethrown every time someone attempts to continue reading the body. + */ + private final AtomicReference backgroundReadThrowable = new AtomicReference<>(); + + /** + * Indicates that Cronet is ready to receive another body part. + * + *

This method is executed by Cronet's upload data provider. + */ + Future enqueueBodyRead(ByteBuffer readBuffer) { + Throwable backgroundThrowable = backgroundReadThrowable.get(); + if (backgroundThrowable != null) { + return Futures.immediateFailedFuture(backgroundThrowable); + } + SettableFuture future = SettableFuture.create(); + pendingRead.add(Pair.create(readBuffer, future)); + + // Properly handle interleaving handleBackgroundReadError / enqueueBodyRead calls. + if ((backgroundThrowable = backgroundReadThrowable.get()) != null) { + future.setException(backgroundThrowable); + } + return future; + } + + /** + * Signals that reading the OkHttp body failed with the given throwable. + * + *

This method is executed by the background OkHttp body reading thread. + */ + void setBackgroundReadError(Throwable t) { + backgroundReadThrowable.set(t); + Pair> read = pendingRead.poll(); + if (read != null) { + read.second.setException(t); + } + } + + /** + * Signals that reading the body has ended and no future bytes will be sent. + * + *

This method is executed by the background OkHttp body reading thread. + */ + void handleEndOfStreamSignal() throws IOException { + if (isClosed.getAndSet(true)) { + throw new IllegalStateException("Already closed"); + } + + getPendingCronetRead().second.set(ReadResult.END_OF_BODY); + } + + /** + * {@inheritDoc} + * + *

This method is executed by the background OkHttp body reading thread. + */ + @Override + public void write(Buffer source, long byteCount) throws IOException { + // This is just a safeguard, close() is a no-op if the body length contract is honored. + checkState(!isClosed.get()); + + long bytesRemaining = byteCount; + + while (bytesRemaining != 0) { + Pair> payload = getPendingCronetRead(); + + ByteBuffer readBuffer = payload.first; + SettableFuture future = payload.second; + + int originalBufferLimit = readBuffer.limit(); + int bytesToDrain = (int) Math.min(originalBufferLimit, bytesRemaining); + + readBuffer.limit(bytesToDrain); + + try { + long bytesRead = source.read(readBuffer); + if (bytesRead == -1) { + IOException e = new IOException("The source has been exhausted but we expected more!"); + future.setException(e); + throw e; + } + bytesRemaining -= bytesRead; + readBuffer.limit(originalBufferLimit); + future.set(ReadResult.SUCCESS); + } catch (IOException e) { + future.setException(e); + throw e; + } + } + } + + private Pair> getPendingCronetRead() throws IOException { + try { + return pendingRead.take(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException("Interrupted while waiting for a read to finish!"); + } + } + + @Override + public void close() { + isClosed.set(true); + } + + @Override + public void flush() { + // Not necessary, we "flush" by sending the data to Cronet straight away when write() is called. + // Note that this class is wrapped with a okio buffer so writes to the outer layer won't be + // seen by this class immediately. + } + + @Override + public Timeout timeout() { + return Timeout.NONE; + } + + enum ReadResult { + SUCCESS, + END_OF_BODY + } +} From 95aede50ac6343232bde0b6495edb941dc22520e Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Tue, 12 Aug 2025 23:28:02 +0100 Subject: [PATCH 03/12] Rename .java to .kt --- .../{CronetCallFactory.java => HttpEngineCallFactory.kt} | 0 .../{CronetInterceptor.java => HttpEngineInterceptor.kt} | 0 ...{CronetTimeoutException.java => HttpEngineTimeoutException.kt} | 0 ...nsportResponseBody.java => HttpEngineTransportResponseBody.kt} | 0 ...pBridgeRequestCallback.java => OkHttpBridgeRequestCallback.kt} | 0 .../httpengine/{RedirectStrategy.java => RedirectStrategy.kt} | 0 .../{RequestBodyConverter.java => RequestBodyConverter.kt} | 0 ...{RequestBodyConverterImpl.java => RequestBodyConverterImpl.kt} | 0 ...{RequestResponseConverter.java => RequestResponseConverter.kt} | 0 ...rBasedBuilder.java => RequestResponseConverterBasedBuilder.kt} | 0 .../httpengine/{ResponseConverter.java => ResponseConverter.kt} | 0 .../{UploadBodyDataBroker.java => UploadBodyDataBroker.kt} | 0 12 files changed, 0 insertions(+), 0 deletions(-) rename okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/{CronetCallFactory.java => HttpEngineCallFactory.kt} (100%) rename okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/{CronetInterceptor.java => HttpEngineInterceptor.kt} (100%) rename okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/{CronetTimeoutException.java => HttpEngineTimeoutException.kt} (100%) rename okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/{CronetTransportResponseBody.java => HttpEngineTransportResponseBody.kt} (100%) rename okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/{OkHttpBridgeRequestCallback.java => OkHttpBridgeRequestCallback.kt} (100%) rename okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/{RedirectStrategy.java => RedirectStrategy.kt} (100%) rename okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/{RequestBodyConverter.java => RequestBodyConverter.kt} (100%) rename okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/{RequestBodyConverterImpl.java => RequestBodyConverterImpl.kt} (100%) rename okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/{RequestResponseConverter.java => RequestResponseConverter.kt} (100%) rename okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/{RequestResponseConverterBasedBuilder.java => RequestResponseConverterBasedBuilder.kt} (100%) rename okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/{ResponseConverter.java => ResponseConverter.kt} (100%) rename okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/{UploadBodyDataBroker.java => UploadBodyDataBroker.kt} (100%) diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetCallFactory.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallFactory.kt similarity index 100% rename from okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetCallFactory.java rename to okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallFactory.kt diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetInterceptor.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt similarity index 100% rename from okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetInterceptor.java rename to okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetTimeoutException.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTimeoutException.kt similarity index 100% rename from okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetTimeoutException.java rename to okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTimeoutException.kt diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetTransportResponseBody.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTransportResponseBody.kt similarity index 100% rename from okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/CronetTransportResponseBody.java rename to okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTransportResponseBody.kt diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.kt similarity index 100% rename from okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.java rename to okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.kt diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.kt similarity index 100% rename from okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.java rename to okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.kt diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.kt similarity index 100% rename from okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.java rename to okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.kt diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt similarity index 100% rename from okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.java rename to okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt similarity index 100% rename from okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.java rename to okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.kt similarity index 100% rename from okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.java rename to okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.kt diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.kt similarity index 100% rename from okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.java rename to okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.kt diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.java b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.kt similarity index 100% rename from okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.java rename to okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.kt From 331a47bb97d8449993bd497fe3775b4a9629d97b Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Tue, 12 Aug 2025 23:28:02 +0100 Subject: [PATCH 04/12] Testing with HttpEngine --- android-test/build.gradle.kts | 2 + .../android/test/HttpEngineBridgeTest.kt | 88 ++++ gradle/libs.versions.toml | 2 + okhttp/build.gradle.kts | 1 + .../httpengine/HttpEngineCallFactory.kt | 419 +++++++++-------- .../httpengine/HttpEngineInterceptor.kt | 201 ++++----- .../httpengine/HttpEngineTimeoutException.kt | 7 +- .../HttpEngineTransportResponseBody.kt | 52 +-- .../httpengine/OkHttpBridgeRequestCallback.kt | 361 ++++++++------- .../android/httpengine/RedirectStrategy.kt | 98 ++-- .../httpengine/RequestBodyConverter.kt | 16 +- .../httpengine/RequestBodyConverterImpl.kt | 420 +++++++++--------- .../httpengine/RequestResponseConverter.kt | 203 ++++----- .../RequestResponseConverterBasedBuilder.kt | 88 ++-- .../android/httpengine/ResponseConverter.kt | 394 ++++++++-------- .../httpengine/UploadBodyDataBroker.kt | 179 ++++---- 16 files changed, 1249 insertions(+), 1282 deletions(-) create mode 100644 android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt diff --git a/android-test/build.gradle.kts b/android-test/build.gradle.kts index 8d45a2d4087b..e703eb13f73c 100644 --- a/android-test/build.gradle.kts +++ b/android-test/build.gradle.kts @@ -105,6 +105,8 @@ dependencies { androidTestImplementation(libs.squareup.moshi) androidTestImplementation(libs.squareup.moshi.kotlin) androidTestImplementation(libs.squareup.okio.fakefilesystem) + androidTestImplementation(libs.kotlinx.coroutines.test) + androidTestImplementation(projects.okhttpCoroutines) androidTestImplementation(libs.androidx.test.runner) androidTestImplementation(libs.junit.jupiter.api) diff --git a/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt b/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt new file mode 100644 index 000000000000..31e6a999e4fa --- /dev/null +++ b/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt @@ -0,0 +1,88 @@ +/* + * Copyright (C) 2025 Block, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package okhttp.android.test + +import android.content.Context +import android.net.http.ConnectionMigrationOptions +import android.net.http.ConnectionMigrationOptions.MIGRATION_OPTION_ENABLED +import android.net.http.DnsOptions +import android.net.http.DnsOptions.DNS_OPTION_ENABLED +import android.net.http.HttpEngine +import android.net.http.QuicOptions +import androidx.test.core.app.ApplicationProvider +import androidx.test.filters.SdkSuppress +import kotlinx.coroutines.test.runTest +import okhttp3.HttpUrl.Companion.toHttpUrl +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.android.httpengine.HttpEngineInterceptor +import okhttp3.coroutines.executeAsync +import org.junit.Test + +@SdkSuppress(minSdkVersion = 34) +class HttpEngineBridgeTest { + @Test + fun testNewCall() = runTest { + val context = ApplicationProvider.getApplicationContext() + + val httpEngine = HttpEngine.Builder(context) + .setStoragePath(context.filesDir.resolve("httpEngine").apply { + mkdirs() + }.path) + .setConnectionMigrationOptions(ConnectionMigrationOptions.Builder() + .setAllowNonDefaultNetworkUsage(MIGRATION_OPTION_ENABLED) + .setDefaultNetworkMigration(MIGRATION_OPTION_ENABLED) + .setPathDegradationMigration(MIGRATION_OPTION_ENABLED) + .build()) + .addQuicHint("www.google.com", 443, 443) + .addQuicHint("google.com", 443, 443) + .setDnsOptions(DnsOptions.Builder() + .setPersistHostCache(DNS_OPTION_ENABLED) + .setPreestablishConnectionsToStaleDnsResults(DNS_OPTION_ENABLED) + .setUseHttpStackDnsResolver(DNS_OPTION_ENABLED) + .setStaleDnsOptions(DnsOptions.StaleDnsOptions.Builder() + .setUseStaleOnNameNotResolved(DNS_OPTION_ENABLED) + .build()) + .build()) + .setEnableQuic(true) + .setQuicOptions(QuicOptions.Builder() + .addAllowedQuicHost("www.google.com") + .addAllowedQuicHost("google.com") + .build()) + .build() + + val httpEngineInterceptor = HttpEngineInterceptor + .newBuilder(httpEngine = httpEngine) + .build() + + val client = OkHttpClient.Builder() + .addInterceptor(httpEngineInterceptor) + .build() + + val call = client.newCall(Request("https://google.com/robots.txt".toHttpUrl())) + + val response = call.executeAsync() + + println(response.body.string().take(40)) + + val call2 = client.newCall(Request("https://google.com/robots.txt".toHttpUrl())) + + val response2 = call2.executeAsync() + + println(response2.body.string().take(40)) + println(response2.protocol) + } +} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index ff52726d8069..ef91d286ab4f 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -6,6 +6,7 @@ com-squareup-moshi = "1.15.2" com-squareup-okio = "3.16.0" de-mannodermaus-junit5 = "1.8.0" graalvm = "24.2.2" +guava = "33.4.8-android" junit-platform = "1.13.4" kotlinx-serialization = "1.9.0" ksp = "2.2.0-2.0.2" @@ -59,6 +60,7 @@ gradlePlugin-mavenPublish = "com.vanniktech:gradle-maven-publish-plugin:0.34.0" gradlePlugin-mavenSympathy = "io.github.usefulness.maven-sympathy:io.github.usefulness.maven-sympathy.gradle.plugin:0.3.0" gradlePlugin-shadow = "com.gradleup.shadow:shadow-gradle-plugin:9.0.1" gradlePlugin-spotless = "com.diffplug.spotless:spotless-plugin-gradle:7.2.1" +guava = { module = "com.google.guava:guava", version.ref = "guava" } hamcrestLibrary = "org.hamcrest:hamcrest-library:3.0" httpClient5 = "org.apache.httpcomponents.client5:httpclient5:5.5" jettyClient = "org.eclipse.jetty:jetty-client:9.4.57.v20241219" diff --git a/okhttp/build.gradle.kts b/okhttp/build.gradle.kts index 3198289c19f4..e9c0903cae63 100644 --- a/okhttp/build.gradle.kts +++ b/okhttp/build.gradle.kts @@ -95,6 +95,7 @@ kotlin { compileOnly(libs.conscrypt.openjdk) implementation(libs.androidx.annotation) implementation(libs.androidx.startup.runtime) + implementation(libs.guava) } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallFactory.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallFactory.kt index e4f4a120d574..df1cf245db05 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallFactory.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallFactory.kt @@ -13,206 +13,184 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -package okhttp3.android.httpengine; - -import android.util.Log; -import com.google.net.cronet.okhttptransport.RequestResponseConverter.CronetRequestAndOkHttpResponse; -import okhttp3.Call; -import okhttp3.Callback; -import okhttp3.Request; -import okhttp3.Response; -import okio.AsyncTimeout; -import okio.Timeout; -import android.net.http.HttpEngine; - -import java.io.IOException; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; - -import static com.google.common.base.Preconditions.*; -import static java.util.concurrent.TimeUnit.MILLISECONDS; - -/** A {@link Call.Factory} implementation using Cronet as the transport layer. */ -public final class CronetCallFactory implements Call.Factory { - - private static final String TAG = "CronetCallFactory"; - - private final RequestResponseConverter converter; - private final ExecutorService responseCallbackExecutor; - private final int readTimeoutMillis; - private final int writeTimeoutMillis; - private final int callTimeoutMillis; - - private CronetCallFactory( - RequestResponseConverter converter, - ExecutorService responseCallbackExecutor, - int readTimeoutMillis, - int writeTimeoutMillis, - int callTimeoutMillis) { - checkArgument(readTimeoutMillis >= 0, "Read timeout mustn't be negative!"); - checkArgument(writeTimeoutMillis >= 0, "Write timeout mustn't be negative!"); - checkArgument(callTimeoutMillis >= 0, "Call timeout mustn't be negative!"); - - this.converter = converter; - this.responseCallbackExecutor = responseCallbackExecutor; - this.readTimeoutMillis = readTimeoutMillis; - this.writeTimeoutMillis = writeTimeoutMillis; - this.callTimeoutMillis = callTimeoutMillis; - } - - public static Builder newBuilder(HttpEngine HttpEngine) { - return new Builder(HttpEngine); +package okhttp3.android.httpengine + +import android.net.http.HttpEngine +import android.os.Build +import android.util.Log +import androidx.annotation.RequiresExtension +import com.google.common.util.concurrent.FutureCallback +import com.google.common.util.concurrent.Futures +import java.io.IOException +import java.util.concurrent.ExecutorService +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicReference +import okhttp3.Call +import okhttp3.Callback +import okhttp3.Request +import okhttp3.Response +import okhttp3.android.httpengine.RequestResponseConverter.CronetRequestAndOkHttpResponse +import okio.AsyncTimeout +import okio.Timeout + +/** A [Call.Factory] implementation using Cronet as the transport layer. */ +@RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) +class HttpEngineCallFactory private constructor( + converter: RequestResponseConverter, + responseCallbackExecutor: ExecutorService, + readTimeoutMillis: Int, + writeTimeoutMillis: Int, + callTimeoutMillis: Int +) : Call.Factory { + private val converter: RequestResponseConverter + private val responseCallbackExecutor: ExecutorService + private val readTimeoutMillis: Int + private val writeTimeoutMillis: Int + private val callTimeoutMillis: Int + + init { + check(readTimeoutMillis >= 0) { "Read timeout mustn't be negative!" } + check(writeTimeoutMillis >= 0) { "Write timeout mustn't be negative!" } + check(callTimeoutMillis >= 0) { "Call timeout mustn't be negative!" } + + this.converter = converter + this.responseCallbackExecutor = responseCallbackExecutor + this.readTimeoutMillis = readTimeoutMillis + this.writeTimeoutMillis = writeTimeoutMillis + this.callTimeoutMillis = callTimeoutMillis } - @Override - public Call newCall(Request request) { - return new CronetCall(request, this, converter, responseCallbackExecutor); + override fun newCall(request: Request): Call { + return CronetCall(request, this, converter, responseCallbackExecutor) } - private static class CronetCall implements Call { - - private final Request okHttpRequest; - private final CronetCallFactory motherFactory; - private final RequestResponseConverter converter; - private final ExecutorService responseCallbackExecutor; - - private final AtomicBoolean executed = new AtomicBoolean(); - private final AtomicBoolean canceled = new AtomicBoolean(); - private final AtomicReference convertedRequestAndResponse = - new AtomicReference<>(); - private final AsyncTimeout timeout; - - private CronetCall( - Request okHttpRequest, - CronetCallFactory motherFactory, - RequestResponseConverter converter, - ExecutorService responseCallbackExecutor) { - this.okHttpRequest = okHttpRequest; - this.motherFactory = motherFactory; - this.converter = converter; - this.responseCallbackExecutor = responseCallbackExecutor; - + private class CronetCall( + private val okHttpRequest: Request, + private val motherFactory: HttpEngineCallFactory, + private val converter: RequestResponseConverter, + private val responseCallbackExecutor: ExecutorService + ) : Call { + private val executed = AtomicBoolean() + private val canceled = AtomicBoolean() + private val convertedRequestAndResponse: AtomicReference = + AtomicReference() + internal val timeout: AsyncTimeout + + init { this.timeout = - new AsyncTimeout() { - @Override - protected void timedOut() { - CronetCall.this.cancel(); // Timeout has its own method named cancel - } - }; - timeout.timeout(motherFactory.callTimeoutMillis, MILLISECONDS); + object : AsyncTimeout() { + override fun timedOut() { + this@CronetCall.cancel() // Timeout has its own method named cancel + } + } + timeout.timeout(motherFactory.callTimeoutMillis.toLong(), TimeUnit.MILLISECONDS) } - @Override - public Request request() { - return okHttpRequest; + override fun request(): Request { + return okHttpRequest } - @Override - public Response execute() throws IOException { - evaluateExecutionPreconditions(); + @Throws(IOException::class) + override fun execute(): Response { + evaluateExecutionPreconditions() try { - timeout.enter(); - CronetRequestAndOkHttpResponse requestAndOkHttpResponse = - converter.convert( - request(), motherFactory.readTimeoutMillis, motherFactory.writeTimeoutMillis); - convertedRequestAndResponse.set(requestAndOkHttpResponse); + timeout.enter() + val requestAndOkHttpResponse: CronetRequestAndOkHttpResponse = + converter.convert( + request(), motherFactory.readTimeoutMillis, motherFactory.writeTimeoutMillis + ) + convertedRequestAndResponse.set(requestAndOkHttpResponse) - startRequestIfNotCanceled(); + startRequestIfNotCanceled() - return toCronetCallFactoryResponse(this, requestAndOkHttpResponse.getResponse()); - } catch (RuntimeException | IOException e) { + return toCronetCallFactoryResponse(this, requestAndOkHttpResponse.response) + } catch (e: RuntimeException) { // If the request finished successfully don't exit the timeout yet. Reading the body also // needs to be considered and the body object will take care of exiting it. See // toCronetCallFactoryResponse() for details. - timeout.exit(); - throw e; + timeout.exit() + throw e + } catch (e: IOException) { + timeout.exit() + throw e } } - @Override - public void enqueue(Callback responseCallback) { + override fun enqueue(responseCallback: Callback) { try { - timeout.enter(); - evaluateExecutionPreconditions(); - CronetRequestAndOkHttpResponse requestAndOkHttpResponse = - converter.convert( - request(), motherFactory.readTimeoutMillis, motherFactory.writeTimeoutMillis); - convertedRequestAndResponse.set(requestAndOkHttpResponse); - CronetCall call = this; + timeout.enter() + evaluateExecutionPreconditions() + val requestAndOkHttpResponse: CronetRequestAndOkHttpResponse = + converter.convert( + request(), motherFactory.readTimeoutMillis, motherFactory.writeTimeoutMillis + ) + convertedRequestAndResponse.set(requestAndOkHttpResponse) + val call = this Futures.addCallback( - requestAndOkHttpResponse.getResponseAsync(), - new FutureCallback() { - @Override - public void onSuccess(Response result) { - try { - responseCallback.onResponse(call, toCronetCallFactoryResponse(call, result)); - } catch (IOException e) { - // The call factory doesn't really mind this - the application code - // threw an exception while handling the response, they should have taken care - // of it. Just logging the error is consistent with plain OkHttp implementation. - Log.i(TAG, "Callback failure for " + toLoggableString(), e); - } + requestAndOkHttpResponse.responseAsync, + object : FutureCallback { + public override fun onSuccess(result: Response) { + try { + responseCallback.onResponse(call, toCronetCallFactoryResponse(call, result)) + } catch (e: IOException) { + // The call factory doesn't really mind this - the application code + // threw an exception while handling the response, they should have taken care + // of it. Just logging the error is consistent with plain OkHttp implementation. + Log.i(TAG, "Callback failure for " + toLoggableString(), e) } + } - @Override - public void onFailure(Throwable t) { - if (t instanceof IOException) { - responseCallback.onFailure(call, (IOException) t); - } else { - responseCallback.onFailure(call, new IOException(t)); - } + public override fun onFailure(t: Throwable) { + if (t is IOException) { + responseCallback.onFailure(call, t) + } else { + responseCallback.onFailure(call, IOException(t)) } - }, - responseCallbackExecutor); + } + }, + responseCallbackExecutor + ) - startRequestIfNotCanceled(); - } catch (IOException e) { + startRequestIfNotCanceled() + } catch (e: IOException) { // If the request finished successfully don't exit the timeout yet. Reading the body also // needs to be considered and the body object will take care of exiting it. See // toCronetCallFactoryResponse() for details. - timeout.exit(); - responseCallback.onFailure(this, e); + timeout.exit() + responseCallback.onFailure(this, e) } } - @Override - public Call clone() { - return motherFactory.newCall(request()); + override fun clone(): Call { + return motherFactory.newCall(request()) } - @Override - public void cancel() { + override fun cancel() { if (canceled.getAndSet(true)) { // already canceled - return; + return } - CronetRequestAndOkHttpResponse localConverted = convertedRequestAndResponse.get(); - if (localConverted != null) { - localConverted.getRequest().cancel(); - } // else the cancel signal will be picked up by the execute() / enqueue() methods. + val localConverted: CronetRequestAndOkHttpResponse? = convertedRequestAndResponse.get() + localConverted?.request?.cancel() // else the cancel signal will be picked up by the execute() / enqueue() methods. } - @Override - public boolean isExecuted() { - return executed.get(); + override fun isExecuted(): Boolean { + return executed.get() } - @Override - public boolean isCanceled() { - return canceled.get(); + override fun isCanceled(): Boolean { + return canceled.get() } - @Override - public Timeout timeout() { - return timeout; + override fun timeout(): Timeout { + return timeout } - private String toLoggableString() { - return "call to " + request().url().redact(); + fun toLoggableString(): String { + return "call to " + request().url.redact() } /** @@ -221,16 +199,17 @@ public final class CronetCallFactory implements Call.Factory { * @throws IllegalStateException if the request has already been executed. * @throws IOException if the request was canceled */ - private void evaluateExecutionPreconditions() throws IOException { + @Throws(IOException::class) + fun evaluateExecutionPreconditions() { if (canceled.get()) { - throw new IOException("Can't execute canceled requests"); + throw IOException("Can't execute canceled requests") } - checkState(!executed.getAndSet(true), "Already Executed"); + check(!executed.getAndSet(true)) { "Already Executed" } } - private void startRequestIfNotCanceled() { - CronetRequestAndOkHttpResponse requestAndOkHttpResponse = convertedRequestAndResponse.get(); - checkState(requestAndOkHttpResponse != null, "convertedRequestAndResponse must be set!"); + fun startRequestIfNotCanceled() { + val requestAndOkHttpResponse: CronetRequestAndOkHttpResponse? = convertedRequestAndResponse.get() + check(requestAndOkHttpResponse != null) { "convertedRequestAndResponse must be set!" } // There might be a race between the execution and cancellation // evaluateExecutionPreconditions check didn't capture and cancel() might have missed that @@ -245,82 +224,84 @@ public final class CronetCallFactory implements Call.Factory { // convertedRequest?.cancel() | convertedRequest = convert(request) // | if (canceled) convertedRequest.cancel() if (canceled.get()) { - requestAndOkHttpResponse.getRequest().cancel(); + requestAndOkHttpResponse.request.cancel() } else { - requestAndOkHttpResponse.getRequest().start(); + requestAndOkHttpResponse.request.start() } } } - private static Response toCronetCallFactoryResponse(CronetCall call, Response response) { - checkNotNull(response.body()); - - return response - .newBuilder() - .body( - new CronetTransportResponseBody(response.body()) { - @Override - void customCloseHook() { - call.timeout.exit(); - } - }) - .build(); - } - - public static final class Builder - extends RequestResponseConverterBasedBuilder { - private static final int DEFAULT_READ_WRITE_TIMEOUT_MILLIS = 10000; + class Builder + internal constructor(httpEngine: HttpEngine) : + RequestResponseConverterBasedBuilder(httpEngine) { + private var readTimeoutMillis: Int = DEFAULT_READ_WRITE_TIMEOUT_MILLIS + private var writeTimeoutMillis: Int = DEFAULT_READ_WRITE_TIMEOUT_MILLIS + private var callTimeoutMillis = 0 // No timeout + private var callbackExecutorService: ExecutorService? = null + + fun setReadTimeoutMillis(readTimeoutMillis: Int): Builder { + check(readTimeoutMillis >= 0) { "Read timeout mustn't be negative!" } + this.readTimeoutMillis = readTimeoutMillis + return this + } - private int readTimeoutMillis = DEFAULT_READ_WRITE_TIMEOUT_MILLIS; - private int writeTimeoutMillis = DEFAULT_READ_WRITE_TIMEOUT_MILLIS; - private int callTimeoutMillis = 0; // No timeout - private ExecutorService callbackExecutorService = null; + fun setWriteTimeoutMillis(writeTimeoutMillis: Int): Builder { + check(writeTimeoutMillis >= 0) { "Write timeout mustn't be negative!" } + this.writeTimeoutMillis = writeTimeoutMillis + return this + } - Builder(HttpEngine HttpEngine) { - super(HttpEngine, Builder.class); + fun setCallbackExecutorService(callbackExecutorService: ExecutorService?): Builder { + checkNotNull(callbackExecutorService) + this.callbackExecutorService = callbackExecutorService + return this } - public Builder setReadTimeoutMillis(int readTimeoutMillis) { - checkArgument(readTimeoutMillis >= 0, "Read timeout mustn't be negative!"); - this.readTimeoutMillis = readTimeoutMillis; - return this; + fun setCallTimeoutMillis(callTimeoutMillis: Int): Builder { + check(callTimeoutMillis >= 0) { "Call timeout mustn't be negative!" } + this.callTimeoutMillis = callTimeoutMillis + + return this } - public Builder setWriteTimeoutMillis(int writeTimeoutMillis) { - checkArgument(writeTimeoutMillis >= 0, "Write timeout mustn't be negative!"); - this.writeTimeoutMillis = writeTimeoutMillis; - return this; + override fun build(converter: RequestResponseConverter): HttpEngineCallFactory { + val localCallbackExecutorService = callbackExecutorService ?: + // Consistent with OkHttp impl + Executors.newCachedThreadPool() + + return HttpEngineCallFactory( + converter, + localCallbackExecutorService, + readTimeoutMillis, + writeTimeoutMillis, + callTimeoutMillis + ) } - public Builder setCallbackExecutorService(ExecutorService callbackExecutorService) { - checkNotNull(callbackExecutorService); - this.callbackExecutorService = callbackExecutorService; - return this; + companion object { + private const val DEFAULT_READ_WRITE_TIMEOUT_MILLIS = 10000 } + } - public Builder setCallTimeoutMillis(int callTimeoutMillis) { - checkArgument(callTimeoutMillis >= 0, "Call timeout mustn't be negative!"); - this.callTimeoutMillis = callTimeoutMillis; + companion object { + private const val TAG = "CronetCallFactory" - return this; + fun newBuilder(httpEngine: HttpEngine): Builder { + return Builder(httpEngine) } - @Override - CronetCallFactory build(RequestResponseConverter converter) { - ExecutorService localCallbackExecutorService; - if (callbackExecutorService == null) { - // Consistent with OkHttp impl - localCallbackExecutorService = Executors.newCachedThreadPool(); - } else { - localCallbackExecutorService = callbackExecutorService; - } + private fun toCronetCallFactoryResponse(call: CronetCall, response: Response): Response { + checkNotNull(response.body) - return new CronetCallFactory( - converter, - localCallbackExecutorService, - readTimeoutMillis, - writeTimeoutMillis, - callTimeoutMillis); + return response + .newBuilder() + .body( + object : HttpEngineTransportResponseBody(response.body) { + override fun customCloseHook() { + call.timeout.exit() + } + }) + .build() } } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt index 4ebc06c8f9d8..87505037a6e9 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt @@ -13,154 +13,145 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -package okhttp3.android.httpengine; - -import android.util.Log; -import com.google.net.cronet.okhttptransport.RequestResponseConverter.CronetRequestAndOkHttpResponse; -import okhttp3.*; -import android.net.http.HttpEngine; -import android.net.http.UrlRequest; - -import java.io.IOException; -import java.util.Iterator; -import java.util.Map; -import java.util.Map.Entry; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.ScheduledThreadPoolExecutor; - -import static com.google.common.base.Preconditions.checkNotNull; -import static java.util.concurrent.TimeUnit.MILLISECONDS; +package okhttp3.android.httpengine + +import android.net.http.HttpEngine +import android.net.http.UrlRequest +import android.os.Build +import android.util.Log +import androidx.annotation.RequiresExtension +import java.io.IOException +import java.lang.AutoCloseable +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.ScheduledThreadPoolExecutor +import java.util.concurrent.TimeUnit +import okhttp3.Call +import okhttp3.Interceptor +import okhttp3.Response +import okhttp3.ResponseBody /** * An OkHttp interceptor that redirects HTTP traffic to use Cronet instead of using the OkHttp * network stack. * - *

The interceptor should be used as the last application interceptor to ensure that all other + * + * The interceptor should be used as the last application interceptor to ensure that all other * interceptors are visited before sending the request on wire and after a response is returned. * - *

The interceptor is a plug-and-play replacement for the OkHttp stack for the most part, + * + * The interceptor is a plug-and-play replacement for the OkHttp stack for the most part, * however, there are some caveats to keep in mind: * - *

    - *
  1. The entirety of OkHttp core is bypassed. This includes caching configuration and network - * interceptors. - *
  2. Some response fields are not being populated due to mismatches between Cronet's and - * OkHttp's architecture. TODO(danstahr): add a concrete list). - *
+ * + * 1. The entirety of OkHttp core is bypassed. This includes caching configuration and network + * interceptors. + * 1. Some response fields are not being populated due to mismatches between Cronet's and + * OkHttp's architecture. TODO(danstahr): add a concrete list). + * */ -public final class CronetInterceptor implements Interceptor, AutoCloseable { - private static final String TAG = "CronetInterceptor"; - - private static final int CANCELLATION_CHECK_INTERVAL_MILLIS = 500; +@RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) +class HttpEngineInterceptor private constructor(converter: RequestResponseConverter?) : Interceptor, AutoCloseable { + private val converter: RequestResponseConverter = checkNotNull(converter) + private val activeCalls: MutableMap = ConcurrentHashMap() + private val scheduledExecutor: ScheduledExecutorService = ScheduledThreadPoolExecutor(1) - private final RequestResponseConverter converter; - private final Map activeCalls = new ConcurrentHashMap<>(); - private final ScheduledExecutorService scheduledExecutor = new ScheduledThreadPoolExecutor(1); - - private CronetInterceptor(RequestResponseConverter converter) { - this.converter = checkNotNull(converter); + init { // TODO(danstahr): There's no other way to know if the call is canceled but polling // (https://github.com/square/okhttp/issues/7164). - ScheduledFuture unusedFuture = - scheduledExecutor.scheduleAtFixedRate( - () -> { - Iterator> activeCallsIterator = - activeCalls.entrySet().iterator(); - - while (activeCallsIterator.hasNext()) { - try { - Entry activeCall = activeCallsIterator.next(); - if (activeCall.getKey().isCanceled()) { - activeCallsIterator.remove(); - activeCall.getValue().cancel(); - } - } catch (RuntimeException e) { - Log.w(TAG, "Unable to propagate cancellation status", e); - } + val unusedFuture = + scheduledExecutor.scheduleAtFixedRate( + Runnable { + val activeCallsIterator = + activeCalls.entries.iterator() + while (activeCallsIterator.hasNext()) { + try { + val activeCall = activeCallsIterator.next() + if (activeCall.key!!.isCanceled()) { + activeCallsIterator.remove() + activeCall.value!!.cancel() } - }, - CANCELLATION_CHECK_INTERVAL_MILLIS, - CANCELLATION_CHECK_INTERVAL_MILLIS, - MILLISECONDS); + } catch (e: RuntimeException) { + Log.w(TAG, "Unable to propagate cancellation status", e) + } + } + }, + CANCELLATION_CHECK_INTERVAL_MILLIS.toLong(), + CANCELLATION_CHECK_INTERVAL_MILLIS.toLong(), + TimeUnit.MILLISECONDS + ) } - @Override - public Response intercept(Chain chain) throws IOException { + @Throws(IOException::class) + override fun intercept(chain: Interceptor.Chain): Response { if (chain.call().isCanceled()) { - throw new IOException("Canceled"); + throw IOException("Canceled") } - Request request = chain.request(); + val request = chain.request() - CronetRequestAndOkHttpResponse requestAndOkHttpResponse = - converter.convert(request, chain.readTimeoutMillis(), chain.writeTimeoutMillis()); + val requestAndOkHttpResponse: RequestResponseConverter.CronetRequestAndOkHttpResponse = + converter.convert(request, chain.readTimeoutMillis(), chain.writeTimeoutMillis()) - activeCalls.put(chain.call(), requestAndOkHttpResponse.getRequest()); + activeCalls[chain.call()] = requestAndOkHttpResponse.request try { - requestAndOkHttpResponse.getRequest().start(); - return toInterceptorResponse(requestAndOkHttpResponse.getResponse(), chain.call()); - } catch (RuntimeException | IOException e) { + requestAndOkHttpResponse.request.start() + return toInterceptorResponse(requestAndOkHttpResponse.response, chain.call()) + } catch (e: RuntimeException) { // If the response is retrieved successfully the caller is responsible for closing // the response, which will remove it from the active calls map. - activeCalls.remove(chain.call()); - throw e; + activeCalls.remove(chain.call()) + throw e + } catch (e: IOException) { + activeCalls.remove(chain.call()) + throw e } } - /** Creates a {@link CronetInterceptor} builder. */ - public static Builder newBuilder(HttpEngine HttpEngine) { - return new Builder(HttpEngine); + override fun close() { + scheduledExecutor.shutdown() } - @Override - public void close() { - scheduledExecutor.shutdown(); + /** A builder for [HttpEngineInterceptor]. */ + class Builder + internal constructor(httpEngine: HttpEngine) : + RequestResponseConverterBasedBuilder(httpEngine) { + /** Builds the interceptor. The same builder can be used to build multiple interceptors. */ + override fun build(converter: RequestResponseConverter): HttpEngineInterceptor { + return HttpEngineInterceptor(converter) + } } - /** A builder for {@link CronetInterceptor}. */ - public static final class Builder - extends RequestResponseConverterBasedBuilder { + private fun toInterceptorResponse(response: Response, call: Call): Response { + checkNotNull(response.body) - Builder(HttpEngine HttpEngine) { - super(HttpEngine, Builder.class); + if (response.body is HttpEngineInterceptorResponseBody) { + return response } - /** Builds the interceptor. The same builder can be used to build multiple interceptors. */ - @Override - public CronetInterceptor build(RequestResponseConverter converter) { - return new CronetInterceptor(converter); - } + return response + .newBuilder() + .body(HttpEngineInterceptorResponseBody(response.body, call)) + .build() } - private Response toInterceptorResponse(Response response, Call call) { - checkNotNull(response.body()); - - if (response.body() instanceof CronetInterceptorResponseBody) { - return response; + private inner class HttpEngineInterceptorResponseBody(delegate: ResponseBody, private val call: Call) : + HttpEngineTransportResponseBody(delegate) { + override fun customCloseHook() { + activeCalls.remove(call) } - - return response - .newBuilder() - .body(new CronetInterceptorResponseBody(response.body(), call)) - .build(); } - private class CronetInterceptorResponseBody extends CronetTransportResponseBody { - private final Call call; + companion object { + private const val TAG = "CronetInterceptor" - private CronetInterceptorResponseBody(ResponseBody delegate, Call call) { - super(delegate); - this.call = call; - } + private const val CANCELLATION_CHECK_INTERVAL_MILLIS = 500 - @Override - void customCloseHook() { - activeCalls.remove(call); + /** Creates a [HttpEngineInterceptor] builder. */ + fun newBuilder(httpEngine: HttpEngine): Builder { + return Builder(httpEngine) } } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTimeoutException.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTimeoutException.kt index c512cd2bce70..3d2326d839ff 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTimeoutException.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTimeoutException.kt @@ -13,9 +13,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +package okhttp3.android.httpengine -package okhttp3.android.httpengine; +import java.io.IOException -import java.io.IOException; - -public class CronetTimeoutException extends IOException {} +class HttpEngineTimeoutException : IOException() diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTransportResponseBody.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTransportResponseBody.kt index 8e233ddef644..461ed8a9c0f1 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTransportResponseBody.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTransportResponseBody.kt @@ -13,43 +13,33 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -package okhttp3.android.httpengine; - -import androidx.annotation.Nullable; -import okhttp3.MediaType; -import okhttp3.ResponseBody; -import okio.BufferedSource; - -abstract class CronetTransportResponseBody extends ResponseBody { - - private final ResponseBody delegate; - - protected CronetTransportResponseBody(ResponseBody delegate) { - this.delegate = delegate; - } - - @Nullable - @Override - public final MediaType contentType() { - return delegate.contentType(); +package okhttp3.android.httpengine + +import android.os.Build +import androidx.annotation.RequiresExtension +import okhttp3.MediaType +import okhttp3.ResponseBody +import okio.BufferedSource + +@RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) +internal abstract class HttpEngineTransportResponseBody protected constructor(private val delegate: ResponseBody) : + ResponseBody() { + override fun contentType(): MediaType? { + return delegate.contentType() } - @Override - public final long contentLength() { - return delegate.contentLength(); + override fun contentLength(): Long { + return delegate.contentLength() } - @Override - public final BufferedSource source() { - return delegate.source(); + override fun source(): BufferedSource { + return delegate.source() } - @Override - public final void close() { - delegate.close(); - customCloseHook(); + override fun close() { + delegate.close() + customCloseHook() } - abstract void customCloseHook(); + abstract fun customCloseHook() } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.kt index 9e705bf9ed85..c8b8714caa39 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.kt @@ -13,300 +13,287 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -package okhttp3.android.httpengine; - -import com.google.common.util.concurrent.ListenableFuture; -import com.google.common.util.concurrent.SettableFuture; -import okio.Buffer; -import okio.Source; -import okio.Timeout; -import android.net.http.CronetException; -import android.net.http.UrlRequest; -import android.net.http.UrlResponseInfo; - -import javax.annotation.Nullable; -import java.io.IOException; -import java.net.ProtocolException; -import java.nio.ByteBuffer; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.concurrent.ArrayBlockingQueue; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.atomic.AtomicBoolean; - -import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkState; -import static java.util.concurrent.TimeUnit.MILLISECONDS; +package okhttp3.android.httpengine + +import android.net.http.HttpException +import android.net.http.UrlRequest +import android.net.http.UrlResponseInfo +import android.os.Build +import androidx.annotation.RequiresExtension +import com.google.common.util.concurrent.ListenableFuture +import com.google.common.util.concurrent.SettableFuture +import java.io.IOException +import java.net.ProtocolException +import java.nio.ByteBuffer +import java.util.* +import java.util.concurrent.ArrayBlockingQueue +import java.util.concurrent.BlockingQueue +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean +import kotlin.concurrent.Volatile +import okio.Buffer +import okio.Source +import okio.Timeout /** * An implementation of Cronet's callback. This is the heart of the bridge and deals with most of * the async-sync paradigm translation. * - *

Translating the UrlResponseInfo is relatively straightforward as the entire object is + * + * Translating the UrlResponseInfo is relatively straightforward as the entire object is * available immediately and is relatively small, so it can easily fit in memory. * - *

Translating the body is a bit more tricky because of the mismatch between OkHttp and Cronet + * + * Translating the body is a bit more tricky because of the mismatch between OkHttp and Cronet * designs. We invoke Cronet's read and wait for the result using synchronization primitives (see * BodySource implementation). The implementation is assuming that there's always at most one read() * request in flight (which is safe to assume), and relies on reasonable fairness of thread * scheduling, especially when handling cancellations. */ -class OkHttpBridgeRequestCallback extends UrlRequest.Callback { - - /** - * The byte buffer capacity for reading Cronet response bodies. Each response callback will - * allocate its own buffer of this size once the response starts being processed. - */ - private static final int CRONET_BYTE_BUFFER_CAPACITY = 32 * 1024; - - /** A bridge between Cronet's asynchronous callbacks and OkHttp's blocking stream-like reads. */ - private final SettableFuture bodySourceFuture = SettableFuture.create(); +@RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) +internal class OkHttpBridgeRequestCallback(readTimeoutMillis: Long, redirectStrategy: RedirectStrategy) : + UrlRequest.Callback { + /** A bridge between Cronet's asynchronous callbacks and OkHttp's blocking stream-like reads. */ + private val bodySourceFuture: SettableFuture = SettableFuture.create() - /** Signal whether the request is finished and the response has been fully read. */ - private final AtomicBoolean finished = new AtomicBoolean(false); + /** Signal whether the request is finished and the response has been fully read. */ + private val finished = AtomicBoolean(false) - /** Signal whether the request was canceled. */ - private final AtomicBoolean canceled = new AtomicBoolean(false); + /** Signal whether the request was canceled. */ + private val canceled = AtomicBoolean(false) /** - * An internal, blocking, thread safe way of passing data between the callback methods and {@link - * #bodySourceFuture}. + * An internal, blocking, thread safe way of passing data between the callback methods and [ ][.bodySourceFuture]. * - *

Has a capacity of 2 - at most one slot for a read result and at most 1 slot for cancellation - * signal, this guarantees that all inserts are non blocking. + * + * Has a capacity of 2 - at most one slot for a read result and at most 1 slot for cancellation + * signal, this guarantees that all inserts are non-blocking. */ - private final BlockingQueue callbackResults = new ArrayBlockingQueue<>(2); + private val callbackResults: BlockingQueue = ArrayBlockingQueue(2) - /** The response headers. */ - private final SettableFuture headersFuture = SettableFuture.create(); + /** The response headers. */ + private val headersFuture: SettableFuture = SettableFuture.create() - /** The read timeout as specified by OkHttp. * */ - private final long readTimeoutMillis; + /** The read timeout as specified by OkHttp. * */ + private val readTimeoutMillis: Long - /** The previous responses as reported to {@link #onRedirectReceived}, from oldest to newest. * */ - private final List urlResponseInfoChain = new ArrayList<>(); + /** The previous responses as reported to [.onRedirectReceived], from oldest to newest. * */ + private val urlResponseInfoChain: MutableList = ArrayList() - private final RedirectStrategy redirectStrategy; + private val redirectStrategy: RedirectStrategy - /** The request being processed. Set when the request is first seen by the callback. */ - private volatile UrlRequest request; + /** The request being processed. Set when the request is first seen by the callback. */ + @Volatile + private var request: UrlRequest? = null - OkHttpBridgeRequestCallback(long readTimeoutMillis, RedirectStrategy redirectStrategy) { - checkArgument(readTimeoutMillis >= 0); + init { + check(readTimeoutMillis >= 0) // So that we don't have to special case infinity. Int.MAX_VALUE is ~infinity for all practical // use cases. - if (readTimeoutMillis == 0) { - this.readTimeoutMillis = Integer.MAX_VALUE; + if (readTimeoutMillis == 0L) { + this.readTimeoutMillis = Int.MAX_VALUE.toLong() } else { - this.readTimeoutMillis = readTimeoutMillis; + this.readTimeoutMillis = readTimeoutMillis } - this.redirectStrategy = redirectStrategy; - } - - /** Returns the {@link UrlResponseInfo} for the request associated with this callback. */ - ListenableFuture getUrlResponseInfo() { - return headersFuture; + this.redirectStrategy = redirectStrategy } - /** - * Returns the OkHttp {@link Source} for the request associated with this callback. - * - *

Note that retrieving data from the {@code Source} instance might block further as the - * response body is streamed. - */ - ListenableFuture getBodySource() { - return bodySourceFuture; + val urlResponseInfo: ListenableFuture + /** Returns the [UrlResponseInfo] for the request associated with this callback. */ + get() = headersFuture + + val bodySource: ListenableFuture + /** + * Returns the OkHttp [Source] for the request associated with this callback. + * + * Note that retrieving data from the `Source` instance might block further as the + * response body is streamed. + */ + get() = bodySourceFuture + + fun getUrlResponseInfoChain(): MutableList { + return Collections.unmodifiableList(urlResponseInfoChain) } - List getUrlResponseInfoChain() { - return Collections.unmodifiableList(urlResponseInfoChain); - } - - @Override - public void onRedirectReceived( - UrlRequest urlRequest, UrlResponseInfo urlResponseInfo, String nextUrl) { + override fun onRedirectReceived( + urlRequest: UrlRequest, urlResponseInfo: UrlResponseInfo, nextUrl: String + ) { // We shouldn't follow redirects - pass the given UrlResponseInfo as the ultimate result if (!redirectStrategy.followRedirects()) { - checkState(headersFuture.set(urlResponseInfo)); + check(headersFuture.set(urlResponseInfo)) // Note: This might not match the content length headers but we have no way of accessing // the actual body with current Cronet's APIs (see RedirectStrategy). - checkState(bodySourceFuture.set(new Buffer())); - urlRequest.cancel(); - return; + check(bodySourceFuture.set(Buffer())) + urlRequest.cancel() + return } // We should follow redirects and we haven't hit the cap yet - urlResponseInfoChain.add(urlResponseInfo); - if (urlResponseInfo.getUrlChain().size() <= redirectStrategy.numberOfRedirectsToFollow()) { - urlRequest.followRedirect(); - return; + urlResponseInfoChain.add(urlResponseInfo) + if (urlResponseInfo.urlChain.size <= redirectStrategy.numberOfRedirectsToFollow()) { + urlRequest.followRedirect() + return } // Cap reached - cancel the request and fail. Exception crafted to match OkHttp. - urlRequest.cancel(); - - IOException e = - new ProtocolException( - "Too many follow-up requests: " + (redirectStrategy.numberOfRedirectsToFollow() + 1)); - headersFuture.setException(e); - bodySourceFuture.setException(e); + urlRequest.cancel() + + val e: IOException = + ProtocolException( + "Too many follow-up requests: " + (redirectStrategy.numberOfRedirectsToFollow() + 1) + ) + headersFuture.setException(e) + bodySourceFuture.setException(e) } - @Override - public void onResponseStarted(UrlRequest urlRequest, UrlResponseInfo urlResponseInfo) { - request = urlRequest; + override fun onResponseStarted(urlRequest: UrlRequest, urlResponseInfo: UrlResponseInfo) { + request = urlRequest - checkState(headersFuture.set(urlResponseInfo)); - checkState(bodySourceFuture.set(new CronetBodySource())); + check(headersFuture.set(urlResponseInfo)) + check(bodySourceFuture.set(CronetBodySource())) } - @Override - public void onReadCompleted( - UrlRequest urlRequest, UrlResponseInfo urlResponseInfo, ByteBuffer byteBuffer) { - callbackResults.add(new CallbackResult(CallbackStep.ON_READ_COMPLETED, byteBuffer, null)); + override fun onReadCompleted( + urlRequest: UrlRequest, urlResponseInfo: UrlResponseInfo, byteBuffer: ByteBuffer + ) { + callbackResults.add(CallbackResult(CallbackStep.ON_READ_COMPLETED, byteBuffer, null)) } - @Override - public void onSucceeded(UrlRequest urlRequest, UrlResponseInfo urlResponseInfo) { - callbackResults.add(new CallbackResult(CallbackStep.ON_SUCCESS, null, null)); + override fun onSucceeded(urlRequest: UrlRequest, urlResponseInfo: UrlResponseInfo) { + callbackResults.add(CallbackResult(CallbackStep.ON_SUCCESS, null, null)) } - @Override - public void onFailed(UrlRequest urlRequest, UrlResponseInfo urlResponseInfo, CronetException e) { + override fun onFailed(urlRequest: UrlRequest, urlResponseInfo: UrlResponseInfo?, e: HttpException) { // If this was called before we start reading the body, the exception will // propagate in the future providing headers and the body wrapper. if (headersFuture.setException(e) && bodySourceFuture.setException(e)) { - return; + return } // If this was called as a reaction to a read() call, the read result will propagate // the exception. - callbackResults.add(new CallbackResult(CallbackStep.ON_FAILED, null, e)); + callbackResults.add(CallbackResult(CallbackStep.ON_FAILED, null, e)) } - @Override - public void onCanceled(UrlRequest urlRequest, UrlResponseInfo responseInfo) { - canceled.set(true); - callbackResults.add(new CallbackResult(CallbackStep.ON_CANCELED, null, null)); + override fun onCanceled(urlRequest: UrlRequest, responseInfo: UrlResponseInfo?) { + canceled.set(true) + callbackResults.add(CallbackResult(CallbackStep.ON_CANCELED, null, null)) // If there's nobody listening it's possible that the cancellation happened before we even // received anything from the server. In that case inform the thread that's awaiting server // response about the cancellation as well. This becomes a no-op if the futures // were already set. - IOException e = new IOException("The request was canceled!"); - headersFuture.setException(e); - bodySourceFuture.setException(e); + val e = IOException("The request was canceled!") + headersFuture.setException(e) + bodySourceFuture.setException(e) } - private class CronetBodySource implements Source { + private inner class CronetBodySource : Source { + private var buffer: ByteBuffer? = ByteBuffer.allocateDirect(CRONET_BYTE_BUFFER_CAPACITY) - private ByteBuffer buffer = ByteBuffer.allocateDirect(CRONET_BYTE_BUFFER_CAPACITY); + /** Whether the close() method has been called. */ + @Volatile + private var closed = false - /** Whether the close() method has been called. */ - private volatile boolean closed = false; - - @Override - public long read(Buffer sink, long byteCount) throws IOException { + @Throws(IOException::class) + override fun read(sink: Buffer, byteCount: Long): Long { if (canceled.get()) { - throw new IOException("The request was canceled!"); + throw IOException("The request was canceled!") } // Using IAE instead of NPE (checkNotNull) for okio.RealBufferedSource consistency - checkArgument(sink != null, "sink == null"); - checkArgument(byteCount >= 0, "byteCount < 0: %s", byteCount); - checkState(!closed, "closed"); + check(byteCount >= 0) { "byteCount < 0: $byteCount" } + check(!closed) { "closed" } if (finished.get()) { - return -1; + return -1 } - if (byteCount < buffer.limit()) { - buffer.limit((int) byteCount); + if (byteCount < buffer!!.limit()) { + buffer!!.limit(byteCount.toInt()) } - request.read(buffer); + request!!.read(buffer!!) - CallbackResult result; + var result: CallbackResult? try { - result = callbackResults.poll(readTimeoutMillis, MILLISECONDS); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - result = null; + result = callbackResults.poll(readTimeoutMillis, TimeUnit.MILLISECONDS) + } catch (e: InterruptedException) { + Thread.currentThread().interrupt() + result = null } if (result == null) { // Either readResult.poll() was interrupted or it timed out. - request.cancel(); - throw new CronetTimeoutException(); + request!!.cancel() + throw HttpEngineTimeoutException() } - switch (result.callbackStep) { - // We null the buffer in final statuses to allow fast GC of the buffer even if the callback - // is still in use. - case ON_FAILED: - finished.set(true); - buffer = null; - throw new IOException(result.exception); - case ON_SUCCESS: - finished.set(true); - buffer = null; - return -1; - case ON_CANCELED: + when (result.callbackStep) { + CallbackStep.ON_FAILED -> { + finished.set(true) + buffer = null + throw IOException(result.exception) + } + + CallbackStep.ON_SUCCESS -> { + finished.set(true) + buffer = null + return -1 + } + + CallbackStep.ON_CANCELED -> { // The canceled flag is already set by the onCanceled method // so not setting it here. - - buffer = null; - throw new IOException("The request was canceled!"); - case ON_READ_COMPLETED: - result.buffer.flip(); - int bytesWritten = sink.write(result.buffer); - result.buffer.clear(); - return bytesWritten; + buffer = null + throw IOException("The request was canceled!") + } + + CallbackStep.ON_READ_COMPLETED -> { + val resultBuffer = result.buffer!! + resultBuffer.flip() + val bytesWritten = sink.write(result.buffer) + result.buffer.clear() + return bytesWritten.toLong() + } } - - throw new AssertionError("The switch block above is exhaustive!"); } - @Override - public Timeout timeout() { + override fun timeout(): Timeout { // TODO(danstahr): This should likely respect the OkHttp timeout somehow - return Timeout.NONE; + return Timeout.NONE } - @Override - public void close() { + override fun close() { if (closed) { - return; + return } - closed = true; + closed = true if (!finished.get()) { - request.cancel(); + request!!.cancel() } } } - private static class CallbackResult { - private final CallbackStep callbackStep; - @Nullable private final ByteBuffer buffer; - @Nullable private final CronetException exception; - - private CallbackResult( - CallbackStep callbackStep, - @Nullable ByteBuffer buffer, - @Nullable CronetException exception) { - this.callbackStep = callbackStep; - this.buffer = buffer; - this.exception = exception; - } - } + private class CallbackResult( + val callbackStep: CallbackStep, + val buffer: ByteBuffer?, + val exception: HttpException? + ) - private enum CallbackStep { + private enum class CallbackStep { ON_READ_COMPLETED, ON_SUCCESS, ON_FAILED, ON_CANCELED } + + companion object { + /** + * The byte buffer capacity for reading Cronet response bodies. Each response callback will + * allocate its own buffer of this size once the response starts being processed. + */ + private val CRONET_BYTE_BUFFER_CAPACITY = 32 * 1024 + } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.kt index b78995fa33f5..8a7e41ce28b9 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.kt @@ -13,77 +13,65 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +package okhttp3.android.httpengine -package okhttp3.android.httpengine; - -/** Defines a redirect strategy for the Cronet OkHttp transport layer. */ -public abstract class RedirectStrategy { - - /** The default number of redirects to follow. Should be less than the Chromium wide safeguard. */ - private static final int DEFAULT_REDIRECTS = 16; - +/** Defines a redirect strategy for the Cronet OkHttp transport layer. */ +abstract class RedirectStrategy private constructor() { /** * Returns whether redirects should be followed at all. If set to false, the redirect response * will be returned. */ - abstract boolean followRedirects(); + abstract fun followRedirects(): Boolean /** * Returns the maximum number of redirects to follow. If more redirects are attempted an exception - * should be thrown by the component handling the request. Shouldn't be called at all if {@link - * #followRedirects()} return false. + * should be thrown by the component handling the request. Shouldn't be called at all if [ ][.followRedirects] return false. */ - abstract int numberOfRedirectsToFollow(); + abstract fun numberOfRedirectsToFollow(): Int - /** - * Returns a strategy which will not follow redirects. - * - *

Note that because of Cronet's limitations - * (https://developer.android.com/guide/topics/connectivity/cronet/lifecycle#overview) it is - * impossible to retrieve the body of a redirect response. As a result, a dummy empty body will - * always be provided. - */ - public static RedirectStrategy withoutRedirects() { - return WithoutRedirectsHolder.INSTANCE; - } + internal object WithoutRedirectsHolder : RedirectStrategy() { + override fun followRedirects(): Boolean { + return false + } - /** - * Returns a strategy which will follow redirects up to {@link #DEFAULT_REDIRECTS} times. If more - * redirects are attempted an exception is thrown. - */ - public static RedirectStrategy defaultStrategy() { - return DefaultRedirectsHolder.INSTANCE; + override fun numberOfRedirectsToFollow(): Int { + throw UnsupportedOperationException() + } } - private static class WithoutRedirectsHolder { - private static final RedirectStrategy INSTANCE = - new RedirectStrategy() { - @Override - boolean followRedirects() { - return false; - } + internal object DefaultRedirectsHolder : RedirectStrategy() { + override fun followRedirects(): Boolean { + return true + } - @Override - int numberOfRedirectsToFollow() { - throw new UnsupportedOperationException(); - } - }; + override fun numberOfRedirectsToFollow(): Int { + return DEFAULT_REDIRECTS + } } - private static class DefaultRedirectsHolder { - private static final RedirectStrategy INSTANCE = - new RedirectStrategy() { - @Override - boolean followRedirects() { - return true; - } + companion object { + /** The default number of redirects to follow. Should be less than the Chromium wide safeguard. */ + private const val DEFAULT_REDIRECTS = 16 - @Override - int numberOfRedirectsToFollow() { - return DEFAULT_REDIRECTS; - } - }; - } + /** + * Returns a strategy which will not follow redirects. + * + * + * Note that because of Cronet's limitations + * (https://developer.android.com/guide/topics/connectivity/cronet/lifecycle#overview) it is + * impossible to retrieve the body of a redirect response. As a result, a dummy empty body will + * always be provided. + */ + fun withoutRedirects(): RedirectStrategy { + return WithoutRedirectsHolder + } - private RedirectStrategy() {} + /** + * Returns a strategy which will follow redirects up to [.DEFAULT_REDIRECTS] times. If more + * redirects are attempted an exception is thrown. + */ + fun defaultStrategy(): RedirectStrategy { + return DefaultRedirectsHolder + } + } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.kt index 26fd8439b983..5c6ad83e43a3 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.kt @@ -13,16 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +package okhttp3.android.httpengine -package okhttp3.android.httpengine; +import android.net.http.UploadDataProvider +import okhttp3.RequestBody -import okhttp3.RequestBody; -import android.net.http.UploadDataProvider; - -import java.io.IOException; - -/** An interface for classes converting from OkHttp to Cronet request bodies. */ -interface RequestBodyConverter { - UploadDataProvider convertRequestBody(RequestBody requestBody, int writeTimeoutMillis) - throws IOException; +/** An interface for classes converting from OkHttp to Cronet request bodies. */ +internal interface RequestBodyConverter { + fun convertRequestBody(requestBody: RequestBody, writeTimeoutMillis: Int): UploadDataProvider } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt index 7f71f3adc59d..2f2b7ed4b7b4 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt @@ -13,161 +13,127 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -package okhttp3.android.httpengine; - -import androidx.annotation.VisibleForTesting; -import com.google.common.base.Verify; -import com.google.common.util.concurrent.*; -import com.google.net.cronet.okhttptransport.UploadBodyDataBroker.ReadResult; -import okhttp3.RequestBody; -import okio.Buffer; -import okio.BufferedSink; -import okio.Okio; -import android.net.http.UploadDataProvider; -import android.net.http.UploadDataSink; - -import java.io.IOException; -import java.nio.ByteBuffer; -import java.util.concurrent.Callable; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.TimeoutException; - -import static java.util.concurrent.TimeUnit.MILLISECONDS; - -final class RequestBodyConverterImpl implements RequestBodyConverter { - - private static final long IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES = 1024 * 1024; - - private final InMemoryRequestBodyConverter inMemoryRequestBodyConverter; - private final StreamingRequestBodyConverter streamingRequestBodyConverter; - - RequestBodyConverterImpl( - InMemoryRequestBodyConverter inMemoryConverter, - StreamingRequestBodyConverter streamingConverter) { - this.inMemoryRequestBodyConverter = inMemoryConverter; - this.streamingRequestBodyConverter = streamingConverter; - } - - static RequestBodyConverterImpl create(ExecutorService bodyReaderExecutor) { - return new RequestBodyConverterImpl( - new InMemoryRequestBodyConverter(), new StreamingRequestBodyConverter(bodyReaderExecutor)); - } - - @Override - public UploadDataProvider convertRequestBody(RequestBody requestBody, int writeTimeoutMillis) - throws IOException { - long contentLength = requestBody.contentLength(); - if (contentLength == -1 || contentLength > IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES) { - return streamingRequestBodyConverter.convertRequestBody(requestBody, writeTimeoutMillis); +package okhttp3.android.httpengine + +import android.net.http.UploadDataProvider +import android.net.http.UploadDataSink +import android.os.Build +import androidx.annotation.RequiresExtension +import androidx.annotation.VisibleForTesting +import com.google.common.base.Verify +import com.google.common.util.concurrent.* +import java.io.IOException +import java.nio.ByteBuffer +import java.util.concurrent.* +import kotlin.concurrent.Volatile +import okhttp3.RequestBody +import okhttp3.android.httpengine.UploadBodyDataBroker.ReadResult +import okio.Buffer +import okio.BufferedSink +import okio.buffer + +@RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) +internal class RequestBodyConverterImpl( + private val inMemoryRequestBodyConverter: InMemoryRequestBodyConverter, + private val streamingRequestBodyConverter: StreamingRequestBodyConverter +) : RequestBodyConverter { + @Throws(IOException::class) + override fun convertRequestBody(requestBody: RequestBody, writeTimeoutMillis: Int): UploadDataProvider { + val contentLength = requestBody.contentLength() + if (contentLength == -1L || contentLength > IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES) { + return streamingRequestBodyConverter.convertRequestBody(requestBody, writeTimeoutMillis) } else { - return inMemoryRequestBodyConverter.convertRequestBody(requestBody, writeTimeoutMillis); + return inMemoryRequestBodyConverter.convertRequestBody(requestBody, writeTimeoutMillis) } } /** - * Implementation of {@link RequestBodyConverter} that doesn't need to hold the entire request + * Implementation of [RequestBodyConverter] that doesn't need to hold the entire request * body in memory. * - *

* - *

    - *
  1. {@link RequestBody#writeTo(BufferedSink)} is invoked on the body, but the sink doesn't - * accept any data - *
  2. A call to {@link UploadDataProvider#read(UploadDataSink, ByteBuffer)} unblocks the sink, - * which accepts a part of the body (size depends on the buffer's capacity), then blocks - * again. Buffer is sent to Cronet. - *
+ * + * + * + * 1. [RequestBody.writeTo] is invoked on the body, but the sink doesn't + * accept any data + * 1. A call to [UploadDataProvider.read] unblocks the sink, + * which accepts a part of the body (size depends on the buffer's capacity), then blocks + * again. Buffer is sent to Cronet. + * * * This is repeated until the entire body has been read. */ @VisibleForTesting - static final class StreamingRequestBodyConverter implements RequestBodyConverter { - - private final ExecutorService readerExecutor; - - StreamingRequestBodyConverter(ExecutorService readerExecutor) { - this.readerExecutor = readerExecutor; - } - - @Override - public UploadDataProvider convertRequestBody(RequestBody requestBody, int writeTimeoutMillis) { - return new StreamingUploadDataProvider( - requestBody, new UploadBodyDataBroker(), readerExecutor, writeTimeoutMillis); + internal class StreamingRequestBodyConverter(private val readerExecutor: ExecutorService) : RequestBodyConverter { + @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) + override fun convertRequestBody(requestBody: RequestBody, writeTimeoutMillis: Int): UploadDataProvider { + return StreamingUploadDataProvider( + requestBody, UploadBodyDataBroker(), readerExecutor, writeTimeoutMillis.toLong() + ) } - private static class StreamingUploadDataProvider extends UploadDataProvider { - private final RequestBody okHttpRequestBody; - private final UploadBodyDataBroker broker; - private final ListeningExecutorService readTaskExecutor; - private final long writeTimeoutMillis; - - /** The future for the task that reads the OkHttp request body in the background. */ - private ListenableFuture readTaskFuture; - - /** The number of bytes we read from the OkHttp body thus far. */ - private long totalBytesReadFromOkHttp; - - private StreamingUploadDataProvider( - RequestBody okHttpRequestBody, - UploadBodyDataBroker broker, - ExecutorService readTaskExecutor, - long writeTimeoutMillis) { - this.okHttpRequestBody = okHttpRequestBody; - this.broker = broker; - if (readTaskExecutor instanceof ListeningExecutorService) { - this.readTaskExecutor = (ListeningExecutorService) readTaskExecutor; - } else { - this.readTaskExecutor = MoreExecutors.listeningDecorator(readTaskExecutor); - } - - // So that we don't have to special case infinity. Int.MAX_VALUE is ~infinity for all - // practical use cases. - this.writeTimeoutMillis = writeTimeoutMillis == 0 ? Integer.MAX_VALUE : writeTimeoutMillis; - } - - @Override - public long getLength() throws IOException { - return okHttpRequestBody.contentLength(); + @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) + private class StreamingUploadDataProvider( + private val okHttpRequestBody: RequestBody, + private val broker: UploadBodyDataBroker, + readTaskExecutor: ExecutorService, + writeTimeoutMillis: Long + ) : UploadDataProvider() { + private val readTaskExecutor: ListeningExecutorService = + readTaskExecutor as? ListeningExecutorService ?: MoreExecutors.listeningDecorator(readTaskExecutor) + + // So that we don't have to special case infinity. Int.MAX_VALUE is ~infinity for all + // practical use cases. + private val writeTimeoutMillis: Long = + if (writeTimeoutMillis == 0L) Int.MAX_VALUE.toLong() else writeTimeoutMillis + + /** The future for the task that reads the OkHttp request body in the background. */ + private var readTaskFuture: ListenableFuture? = null + + /** The number of bytes we read from the OkHttp body thus far. */ + private var totalBytesReadFromOkHttp: Long = 0 + + @Throws(IOException::class) + override fun getLength(): Long { + return okHttpRequestBody.contentLength() } - @Override - public void read(UploadDataSink uploadDataSink, ByteBuffer byteBuffer) throws IOException { - ensureReadTaskStarted(); + @Throws(IOException::class) + override fun read(uploadDataSink: UploadDataSink, byteBuffer: ByteBuffer) { + ensureReadTaskStarted() - if (getLength() == -1) { - readUnknownBodyLength(uploadDataSink, byteBuffer); + if (length == -1L) { + readUnknownBodyLength(uploadDataSink, byteBuffer) } else { - readKnownBodyLength(uploadDataSink, byteBuffer); + readKnownBodyLength(uploadDataSink, byteBuffer) } } - private void readKnownBodyLength(UploadDataSink uploadDataSink, ByteBuffer byteBuffer) - throws IOException { + @Throws(IOException::class) + fun readKnownBodyLength(uploadDataSink: UploadDataSink, byteBuffer: ByteBuffer) { try { - UploadBodyDataBroker.ReadResult readResult = readFromOkHttp(byteBuffer); + val readResult: ReadResult = readFromOkHttp(byteBuffer) - if (totalBytesReadFromOkHttp > getLength()) { - throw prepareBodyTooLongException(getLength(), totalBytesReadFromOkHttp); + if (totalBytesReadFromOkHttp > length) { + throw prepareBodyTooLongException(length, totalBytesReadFromOkHttp) } - if (totalBytesReadFromOkHttp < getLength()) { - switch (readResult) { - case SUCCESS: - uploadDataSink.onReadSucceeded(false); - break; - case END_OF_BODY: - throw new IOException("The source has been exhausted but we expected more data!"); + if (totalBytesReadFromOkHttp < length) { + when (readResult) { + ReadResult.SUCCESS -> uploadDataSink.onReadSucceeded(false) + ReadResult.END_OF_BODY -> throw IOException("The source has been exhausted but we expected more data!") } - return; + return } // Else we're handling what's supposed to be the last chunk - handleLastBodyRead(uploadDataSink, byteBuffer); - - } catch (TimeoutException | ExecutionException e) { - readTaskFuture.cancel(true); - uploadDataSink.onReadError(new IOException(e)); + handleLastBodyRead(uploadDataSink, byteBuffer) + } catch (e: TimeoutException) { + readTaskFuture!!.cancel(true) + uploadDataSink.onReadError(IOException(e)) + } catch (e: ExecutionException) { + readTaskFuture!!.cancel(true) + uploadDataSink.onReadError(IOException(e)) } } @@ -177,156 +143,168 @@ final class RequestBodyConverterImpl implements RequestBodyConverter { * result, when we read the advertised number of bytes, we need to make sure that the stream * is indeed finished. */ - private void handleLastBodyRead(UploadDataSink uploadDataSink, ByteBuffer filledByteBuffer) - throws IOException, TimeoutException, ExecutionException { + @Throws(IOException::class, TimeoutException::class, ExecutionException::class) + fun handleLastBodyRead(uploadDataSink: UploadDataSink, filledByteBuffer: ByteBuffer) { // We reuse the same buffer for the END_OF_DATA read (it should be non-destructive and if // it overwrites what's in there we don't mind as that's an error anyway). We just need // to make sure we restore the original position afterwards. We don't use mark() / reset() // as the mark position can be invalidated by limit manipulation. - int bufferPosition = filledByteBuffer.position(); - filledByteBuffer.position(0); + val bufferPosition = filledByteBuffer.position() + filledByteBuffer.position(0) - UploadBodyDataBroker.ReadResult readResult = readFromOkHttp(filledByteBuffer); + val readResult: ReadResult = readFromOkHttp(filledByteBuffer) - if (!readResult.equals(ReadResult.END_OF_BODY)) { - throw prepareBodyTooLongException(getLength(), totalBytesReadFromOkHttp); + if (readResult != ReadResult.END_OF_BODY) { + throw prepareBodyTooLongException(length, totalBytesReadFromOkHttp) } Verify.verify( - filledByteBuffer.position() == 0, - "END_OF_BODY reads shouldn't write anything to the buffer"); + filledByteBuffer.position() == 0, + "END_OF_BODY reads shouldn't write anything to the buffer" + ) // revert the position change - filledByteBuffer.position(bufferPosition); + filledByteBuffer.position(bufferPosition) - uploadDataSink.onReadSucceeded(false); + uploadDataSink.onReadSucceeded(false) } - private void readUnknownBodyLength(UploadDataSink uploadDataSink, ByteBuffer byteBuffer) { + fun readUnknownBodyLength(uploadDataSink: UploadDataSink, byteBuffer: ByteBuffer) { try { - UploadBodyDataBroker.ReadResult readResult = readFromOkHttp(byteBuffer); - uploadDataSink.onReadSucceeded(readResult.equals(ReadResult.END_OF_BODY)); - } catch (TimeoutException | ExecutionException e) { - readTaskFuture.cancel(true); - uploadDataSink.onReadError(new IOException(e)); + val readResult: ReadResult = readFromOkHttp(byteBuffer) + uploadDataSink.onReadSucceeded(readResult == ReadResult.END_OF_BODY) + } catch (e: TimeoutException) { + readTaskFuture!!.cancel(true) + uploadDataSink.onReadError(IOException(e)) + } catch (e: ExecutionException) { + readTaskFuture!!.cancel(true) + uploadDataSink.onReadError(IOException(e)) } } - private void ensureReadTaskStarted() { + fun ensureReadTaskStarted() { // We don't expect concurrent calls so a simple flag is sufficient if (readTaskFuture == null) { readTaskFuture = - readTaskExecutor.submit( - (Callable) - () -> { - BufferedSink bufferedSink = Okio.buffer(broker); - okHttpRequestBody.writeTo(bufferedSink); - bufferedSink.flush(); - broker.handleEndOfStreamSignal(); - return null; - }); + readTaskExecutor.submit( + Callable { + val bufferedSink: BufferedSink = broker.buffer() + okHttpRequestBody.writeTo(bufferedSink) + bufferedSink.flush() + broker.handleEndOfStreamSignal() + }) Futures.addCallback( - readTaskFuture, - new FutureCallback() { - @Override - public void onSuccess(Object result) {} - - @Override - public void onFailure(Throwable t) { - broker.setBackgroundReadError(t); - } - }, - MoreExecutors.directExecutor()); + readTaskFuture!!, + object : FutureCallback { + override fun onSuccess(result: Unit) {} + + override fun onFailure(t: Throwable) { + broker.setBackgroundReadError(t) + } + }, + MoreExecutors.directExecutor() + ) } } - private ReadResult readFromOkHttp(ByteBuffer byteBuffer) - throws TimeoutException, ExecutionException { - int positionBeforeRead = byteBuffer.position(); - UploadBodyDataBroker.ReadResult readResult = - Uninterruptibles.getUninterruptibly( - broker.enqueueBodyRead(byteBuffer), writeTimeoutMillis, MILLISECONDS); - int bytesRead = byteBuffer.position() - positionBeforeRead; - totalBytesReadFromOkHttp += bytesRead; - return readResult; + @Throws(TimeoutException::class, ExecutionException::class) + fun readFromOkHttp(byteBuffer: ByteBuffer): ReadResult { + val positionBeforeRead = byteBuffer.position() + val readResult: ReadResult = + Uninterruptibles.getUninterruptibly( + broker.enqueueBodyRead(byteBuffer), writeTimeoutMillis, TimeUnit.MILLISECONDS + ) + val bytesRead = byteBuffer.position() - positionBeforeRead + totalBytesReadFromOkHttp += bytesRead.toLong() + return readResult } - private static IOException prepareBodyTooLongException( - long expectedLength, long minActualLength) { - return new IOException( - "Expected " + expectedLength + " bytes but got at least " + minActualLength); + override fun rewind(uploadDataSink: UploadDataSink) { + // TODO(danstahr): OkHttp 4 can use isOneShot flag here and rewind safely. + uploadDataSink.onRewindError(UnsupportedOperationException("Rewind is not supported!")) } - @Override - public void rewind(UploadDataSink uploadDataSink) { - // TODO(danstahr): OkHttp 4 can use isOneShot flag here and rewind safely. - uploadDataSink.onRewindError(new UnsupportedOperationException("Rewind is not supported!")); + companion object { + private fun prepareBodyTooLongException( + expectedLength: Long, minActualLength: Long + ): IOException { + return IOException( + "Expected " + expectedLength + " bytes but got at least " + minActualLength + ) + } } } } /** - * Converts OkHttp's {@link RequestBody} to Cronet's {@link UploadDataProvider} by materializing + * Converts OkHttp's [RequestBody] to Cronet's [UploadDataProvider] by materializing * the body in memory first. * - *

This strategy shouldn't be used for large requests (and for requests with uncapped length) + * + * This strategy shouldn't be used for large requests (and for requests with uncapped length) * to avoid OOM issues. */ + @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) @VisibleForTesting - static final class InMemoryRequestBodyConverter implements RequestBodyConverter { - - @Override - public UploadDataProvider convertRequestBody(RequestBody requestBody, int writeTimeoutMillis) - throws IOException { - + internal class InMemoryRequestBodyConverter : RequestBodyConverter { + @Throws(IOException::class) + override fun convertRequestBody(requestBody: RequestBody, writeTimeoutMillis: Int): UploadDataProvider { // content length is immutable by contract - long length = requestBody.contentLength(); + + val length = requestBody.contentLength() if (length < 0 || length > IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES) { - throw new IOException( - "Expected definite length less than " - + IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES - + "but got " - + length); + throw IOException( + ("Expected definite length less than " + + IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES + + "but got " + + length) + ) } - return new UploadDataProvider() { - private volatile boolean isMaterialized = false; - private final Buffer materializedBody = new Buffer(); + return object : UploadDataProvider() { + @Volatile + private var isMaterialized = false + private val materializedBody = Buffer() - @Override - public long getLength() { - return length; + override fun getLength(): Long { + return length } - @Override - public void read(UploadDataSink uploadDataSink, ByteBuffer byteBuffer) throws IOException { + @Throws(IOException::class) + override fun read(uploadDataSink: UploadDataSink, byteBuffer: ByteBuffer) { // We're not expecting any concurrent calls here so a simple flag should be sufficient. if (!isMaterialized) { - requestBody.writeTo(materializedBody); - materializedBody.flush(); - isMaterialized = true; - long reportedLength = getLength(); - long actualLength = materializedBody.size(); + requestBody.writeTo(materializedBody) + materializedBody.flush() + isMaterialized = true + val reportedLength = getLength() + val actualLength = materializedBody.size if (actualLength != reportedLength) { - throw new IOException( - "Expected " + reportedLength + " bytes but got " + actualLength); + throw IOException( + "Expected " + reportedLength + " bytes but got " + actualLength + ) } } - if (materializedBody.read(byteBuffer) == -1) { - // This should never happen - for known body length we shouldn't be called at all - // if there's no more data to read. - throw new IllegalStateException("The source has been exhausted but we expected more!"); - } - uploadDataSink.onReadSucceeded(false); + check(materializedBody.read(byteBuffer) != -1) { "The source has been exhausted but we expected more!" } + uploadDataSink.onReadSucceeded(false) } - @Override - public void rewind(UploadDataSink uploadDataSink) { + override fun rewind(uploadDataSink: UploadDataSink) { // TODO(danstahr): OkHttp 4 can use isOneShot flag here and rewind safely. - uploadDataSink.onRewindError(new UnsupportedOperationException()); + uploadDataSink.onRewindError(UnsupportedOperationException()) } - }; + } + } + } + + companion object { + private val IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES = (1024 * 1024).toLong() + + fun create(bodyReaderExecutor: ExecutorService): RequestBodyConverterImpl { + return RequestBodyConverterImpl( + InMemoryRequestBodyConverter(), StreamingRequestBodyConverter(bodyReaderExecutor) + ) } } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt index b873e45c4144..b27209e2bb06 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt @@ -13,152 +13,137 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -package okhttp3.android.httpengine; - -import com.google.common.util.concurrent.ListenableFuture; -import com.google.common.util.concurrent.MoreExecutors; -import okhttp3.Request; -import okhttp3.RequestBody; -import okhttp3.Response; -import android.net.http.HttpEngine; -import android.net.http.UrlRequest; - -import java.io.IOException; -import java.util.concurrent.Executor; -import java.util.concurrent.Future; - -/** Converts OkHttp requests to Cronet requests. */ -final class RequestResponseConverter { - private static final String CONTENT_LENGTH_HEADER_NAME = "Content-Length"; - private static final String CONTENT_TYPE_HEADER_NAME = "Content-Type"; - private static final String CONTENT_TYPE_HEADER_DEFAULT_VALUE = "application/octet-stream"; - - private final HttpEngine HttpEngine; - private final Executor uploadDataProviderExecutor; - private final ResponseConverter responseConverter; - private final RequestBodyConverter requestBodyConverter; - private final RedirectStrategy redirectStrategy; - - RequestResponseConverter( - HttpEngine HttpEngine, - Executor uploadDataProviderExecutor, - RequestBodyConverter requestBodyConverter, - ResponseConverter responseConverter, - RedirectStrategy redirectStrategy) { - this.HttpEngine = HttpEngine; - this.uploadDataProviderExecutor = uploadDataProviderExecutor; - this.requestBodyConverter = requestBodyConverter; - this.responseConverter = responseConverter; - this.redirectStrategy = redirectStrategy; - } - +package okhttp3.android.httpengine + +import android.net.http.HttpEngine +import android.net.http.UrlRequest +import android.os.Build +import androidx.annotation.RequiresExtension +import com.google.common.util.concurrent.ListenableFuture +import com.google.common.util.concurrent.MoreExecutors +import java.io.IOException +import java.util.concurrent.Executor +import okhttp3.Request +import okhttp3.Response + +/** Converts OkHttp requests to Cronet requests. */ +@RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) +internal class RequestResponseConverter( + private val httpEngine: HttpEngine, + private val uploadDataProviderExecutor: Executor, + private val requestBodyConverter: RequestBodyConverter, + private val responseConverter: ResponseConverter, + private val redirectStrategy: RedirectStrategy +) { /** - * Converts OkHttp's {@link Request} to a corresponding Cronet's {@link UrlRequest}. + * Converts OkHttp's [Request] to a corresponding Cronet's [UrlRequest]. + * * - *

Since Cronet doesn't have a notion of a Response, which is handled entirely from the - * callbacks, this method also returns a {@link Future} like object the - * caller should use to obtain the matching {@link Response} for the given request. For example: + * Since Cronet doesn't have a notion of a Response, which is handled entirely from the + * callbacks, this method also returns a [Future] like object the + * caller should use to obtain the matching [Response] for the given request. For example: * * 

-   *   RequestResponseConverter converter = ...
-   *   CronetRequestAndOkHttpResponse reqResp = converter.convert(okHttpRequest);
-   *   reqResp.getRequest.start();
+   * RequestResponseConverter converter = ...
+   * CronetRequestAndOkHttpResponse reqResp = converter.convert(okHttpRequest);
+   * reqResp.getRequest.start();
    *
-   *   // Will block until status code, headers... are available
-   *   Response okHttpResponse = reqResp.getResponse();
+   * // Will block until status code, headers... are available
+   * Response okHttpResponse = reqResp.getResponse();
    *
-   *   // use OkHttp Response as usual
-   *
+ * // use OkHttp Response as usual + * */ - CronetRequestAndOkHttpResponse convert( - Request okHttpRequest, int readTimeoutMillis, int writeTimeoutMillis) throws IOException { - - OkHttpBridgeRequestCallback callback = - new OkHttpBridgeRequestCallback(readTimeoutMillis, redirectStrategy); + @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) + @Throws(IOException::class) + fun convert( + okHttpRequest: Request, readTimeoutMillis: Int, writeTimeoutMillis: Int + ): CronetRequestAndOkHttpResponse { + val callback = + OkHttpBridgeRequestCallback(readTimeoutMillis.toLong(), redirectStrategy) // The OkHttp request callback methods are lightweight, the heavy lifting is done by OkHttp / // app owned threads. Use a direct executor to avoid extra thread hops. - UrlRequest.Builder builder = - HttpEngine - .newUrlRequestBuilder( - okHttpRequest.url().toString(), callback, MoreExecutors.directExecutor()) - .allowDirectExecutor(); + val builder: UrlRequest.Builder = + httpEngine + .newUrlRequestBuilder( + okHttpRequest.url.toString(), MoreExecutors.directExecutor(), callback + ) + .setDirectExecutorAllowed(true) - builder.setHttpMethod(okHttpRequest.method()); + builder.setHttpMethod(okHttpRequest.method) - for (int i = 0; i < okHttpRequest.headers().size(); i++) { - builder.addHeader(okHttpRequest.headers().name(i), okHttpRequest.headers().value(i)); + for (i in 0.. getResponseFuture() { - return responseConverter.toResponseAsync(request, callback); + private fun createResponseSupplier( + request: Request, callback: OkHttpBridgeRequestCallback + ): ResponseSupplier { + return object : ResponseSupplier { + override val response: Response by lazy { responseConverter.toResponse(request, callback) } + + override val responseFuture: ListenableFuture by lazy { + responseConverter.toResponseAsync( + request, + callback + ) } - }; + } } - /** A {@link Future} like holder for OkHttp's {@link Response}. */ - private interface ResponseSupplier { - Response getResponse() throws IOException; + /** A [Future] like holder for OkHttp's [Response]. */ + internal interface ResponseSupplier { + @get:Throws(IOException::class) + val response: Response - ListenableFuture getResponseFuture(); + val responseFuture: ListenableFuture } - /** A simple data class for bundling Cronet request and OkHttp response. */ - static final class CronetRequestAndOkHttpResponse { - private final UrlRequest request; - private final ResponseSupplier responseSupplier; - - CronetRequestAndOkHttpResponse(UrlRequest request, ResponseSupplier responseSupplier) { - this.request = request; - this.responseSupplier = responseSupplier; - } - - public UrlRequest getRequest() { - return request; - } + /** A simple data class for bundling Cronet request and OkHttp response. */ + internal class CronetRequestAndOkHttpResponse( + val request: UrlRequest, + private val responseSupplier: ResponseSupplier + ) { + val response: Response + get() = responseSupplier.response - public Response getResponse() throws IOException { - return responseSupplier.getResponse(); - } + val responseAsync: ListenableFuture + get() = responseSupplier.responseFuture + } - public ListenableFuture getResponseAsync() { - return responseSupplier.getResponseFuture(); - } + companion object { + private const val CONTENT_LENGTH_HEADER_NAME = "Content-Length" + private const val CONTENT_TYPE_HEADER_NAME = "Content-Type" + private const val CONTENT_TYPE_HEADER_DEFAULT_VALUE = "application/octet-stream" } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.kt index 06c0e728fd23..a42a5734b68b 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.kt @@ -13,78 +13,66 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +package okhttp3.android.httpengine -package okhttp3.android.httpengine; +import android.net.http.HttpEngine +import android.os.Build +import androidx.annotation.RequiresExtension +import com.google.common.base.Preconditions.checkArgument +import java.util.concurrent.Executors -import android.net.http.HttpEngine; -import android.net.http.UploadDataProvider; +@RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) +abstract class RequestResponseConverterBasedBuilder, T>( + private val httpEngine: HttpEngine +) { + private var uploadDataProviderExecutorSize: Int = DEFAULT_THREAD_POOL_SIZE -import java.util.concurrent.Executor; -import java.util.concurrent.Executors; - -import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkNotNull; - -abstract class RequestResponseConverterBasedBuilder< - SubBuilderT extends RequestResponseConverterBasedBuilder, - ObjectBeingBuiltT> { - private static final int DEFAULT_THREAD_POOL_SIZE = 4; - - private final HttpEngine HttpEngine; - private int uploadDataProviderExecutorSize = DEFAULT_THREAD_POOL_SIZE; // Not setting the default straight away to lazy initialize the object if it ends up not being // used. - private RedirectStrategy redirectStrategy = null; - private final SubBuilderT castedThis; - - @SuppressWarnings("unchecked") // checked as a precondition - RequestResponseConverterBasedBuilder(HttpEngine HttpEngine, Class clazz) { - this.HttpEngine = checkNotNull(HttpEngine); - checkArgument(this.getClass().equals(clazz)); - castedThis = (SubBuilderT) this; - } + private var redirectStrategy: RedirectStrategy = RedirectStrategy.defaultStrategy() /** * Sets the size of upload data provider executor. The same executor is used for all upload data * providers within the interceptor. * - * @see android.net.http.UrlRequest.Builder#setUploadDataProvider(UploadDataProvider, Executor) + * @see android.net.http.UrlRequest.Builder.setUploadDataProvider */ - public final SubBuilderT setUploadDataProviderExecutorSize(int size) { - checkArgument(size > 0, "The number of threads must be positive!"); - uploadDataProviderExecutorSize = size; - return castedThis; + fun setUploadDataProviderExecutorSize(size: Int): B { + checkArgument(size > 0, "The number of threads must be positive!") + uploadDataProviderExecutorSize = size + return this as B } /** * Sets the strategy for following redirects. * - *

Note that the Cronet (i.e. Chromium) wide safeguards will still apply if one attempts to + * + * Note that the Cronet (i.e. Chromium) wide safeguards will still apply if one attempts to * follow redirects too many times. */ - public final SubBuilderT setRedirectStrategy(RedirectStrategy redirectStrategy) { - checkNotNull(redirectStrategy); - this.redirectStrategy = redirectStrategy; - return castedThis; + fun setRedirectStrategy(redirectStrategy: RedirectStrategy): B { + this.redirectStrategy = redirectStrategy + return this as B } - abstract ObjectBeingBuiltT build(RequestResponseConverter converter); + internal abstract fun build(converter: RequestResponseConverter): T - public final ObjectBeingBuiltT build() { - if (redirectStrategy == null) { - redirectStrategy = RedirectStrategy.defaultStrategy(); - } + fun build(): T { + val converter = + RequestResponseConverter( + httpEngine, + Executors.newFixedThreadPool(uploadDataProviderExecutorSize), + // There must always be enough executors to blocking-read the OkHttp request bodies + // otherwise deadlocks can occur. + RequestBodyConverterImpl.create(Executors.newCachedThreadPool()), + ResponseConverter(), + redirectStrategy + ) - RequestResponseConverter converter = - new RequestResponseConverter( - HttpEngine, - Executors.newFixedThreadPool(uploadDataProviderExecutorSize), - // There must always be enough executors to blocking-read the OkHttp request bodies - // otherwise deadlocks can occur. - RequestBodyConverterImpl.create(Executors.newCachedThreadPool()), - new ResponseConverter(), - redirectStrategy); + return build(converter) + } - return build(converter); + companion object { + private const val DEFAULT_THREAD_POOL_SIZE = 4 } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.kt index 8478d8e4b97c..055242c2d30f 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.kt @@ -13,248 +13,246 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -package okhttp3.android.httpengine; - -import androidx.annotation.NonNull; -import com.google.common.base.Ascii; -import com.google.common.base.Splitter; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; -import com.google.common.util.concurrent.Futures; -import com.google.common.util.concurrent.ListenableFuture; -import com.google.common.util.concurrent.MoreExecutors; -import com.google.common.util.concurrent.Uninterruptibles; -import okhttp3.*; -import okio.Okio; -import okio.Source; -import android.net.http.UrlResponseInfo; - -import javax.annotation.Nullable; -import java.io.IOException; -import java.net.ProtocolException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Future; - -import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkNotNull; +package okhttp3.android.httpengine + +import android.net.http.UrlResponseInfo +import android.os.Build +import androidx.annotation.RequiresExtension +import com.google.common.base.Ascii +import com.google.common.base.Splitter +import com.google.common.collect.ImmutableSet +import com.google.common.collect.Iterables +import com.google.common.util.concurrent.Futures +import com.google.common.util.concurrent.ListenableFuture +import com.google.common.util.concurrent.MoreExecutors +import com.google.common.util.concurrent.Uninterruptibles +import java.io.IOException +import java.net.ProtocolException +import java.util.concurrent.ExecutionException +import java.util.concurrent.Future +import okhttp3.MediaType.Companion.toMediaTypeOrNull +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import okhttp3.ResponseBody +import okhttp3.ResponseBody.Companion.asResponseBody +import okio.Source +import okio.buffer /** - * Converts Cronet's responses (or, more precisely, its chunks as they come from Cronet's {@link - * android.net.http.UrlRequest.Callback}), to OkHttp's {@link Response}. + * Converts Cronet's responses (or, more precisely, its chunks as they come from Cronet's [ ]), to OkHttp's [Response]. */ -final class ResponseConverter { - private static final String CONTENT_LENGTH_HEADER_NAME = "Content-Length"; - private static final String CONTENT_TYPE_HEADER_NAME = "Content-Type"; - private static final String CONTENT_ENCODING_HEADER_NAME = "Content-Encoding"; - - // https://source.chromium.org/search?q=symbol:FilterSourceStream::ParseEncodingType%20f:cc - private static final ImmutableSet ENCODINGS_HANDLED_BY_CRONET = - ImmutableSet.of("br", "deflate", "gzip", "x-gzip"); - - private static final Splitter COMMA_SPLITTER = Splitter.on(',').trimResults().omitEmptyStrings(); - +@RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) +internal class ResponseConverter { /** * Creates an OkHttp's Response from the OkHttp-Cronet bridging callback. * - *

As long as the callback's {@code UrlResponseInfo} is available this method is non-blocking. + * + * As long as the callback's `UrlResponseInfo` is available this method is non-blocking. * However, this method doesn't fetch the entire body response. As a result, subsequent calls to - * the result's {@link Response#body()} methods might block further. + * the result's [Response.body] methods might block further. */ - Response toResponse(Request request, OkHttpBridgeRequestCallback callback) throws IOException { - UrlResponseInfo cronetResponseInfo = getFutureValue(callback.getUrlResponseInfo()); - Response.Builder responseBuilder = - createResponse(request, cronetResponseInfo, getFutureValue(callback.getBodySource())); + @Throws(IOException::class) + fun toResponse(request: Request, callback: OkHttpBridgeRequestCallback): Response { + val cronetResponseInfo: UrlResponseInfo = + getFutureValue(callback.urlResponseInfo) + val responseBuilder: Response.Builder = + createResponse(request, cronetResponseInfo, getFutureValue(callback.bodySource)) - List redirectResponseInfos = callback.getUrlResponseInfoChain(); - List urlChain = cronetResponseInfo.getUrlChain(); + val redirectResponseInfos = callback.getUrlResponseInfoChain() + val urlChain = cronetResponseInfo.urlChain if (!redirectResponseInfos.isEmpty()) { - checkArgument( - urlChain.size() == redirectResponseInfos.size() + 1, - "The number of redirects should be consistent across URLs and headers!"); + check( + urlChain.size == redirectResponseInfos.size + 1 + ) { + "The number of redirects should be consistent across URLs and headers!" + } - Response priorResponse = null; - for (int i = 0; i < redirectResponseInfos.size(); i++) { - Request redirectedRequest = request.newBuilder().url(urlChain.get(i)).build(); + var priorResponse: Response? = null + for (i in redirectResponseInfos.indices) { + val redirectedRequest = request.newBuilder().url(urlChain[i]).build() priorResponse = - createResponse(redirectedRequest, redirectResponseInfos.get(i), null) - .priorResponse(priorResponse) - .build(); + createResponse(redirectedRequest, redirectResponseInfos[i], null) + .priorResponse(priorResponse) + .build() } responseBuilder - .request(request.newBuilder().url(Iterables.getLast(urlChain)).build()) - .priorResponse(priorResponse); + .request(request.newBuilder().url(Iterables.getLast(urlChain)).build()) + .priorResponse(priorResponse) } - return responseBuilder.build(); + return responseBuilder.build() } - ListenableFuture toResponseAsync( - Request request, OkHttpBridgeRequestCallback callback) { - return Futures.whenAllComplete(callback.getUrlResponseInfo(), callback.getBodySource()) - .call(() -> toResponse(request, callback), MoreExecutors.directExecutor()); + fun toResponseAsync( + request: Request, callback: OkHttpBridgeRequestCallback + ): ListenableFuture { + return Futures.whenAllComplete(callback.urlResponseInfo, callback.bodySource) + .call({ toResponse(request, callback) }, MoreExecutors.directExecutor()) } - private static Response.Builder createResponse( - Request request, UrlResponseInfo cronetResponseInfo, @Nullable Source bodySource) - throws IOException { - - Response.Builder responseBuilder = new Response.Builder(); - - @Nullable String contentType = getLastHeaderValue(CONTENT_TYPE_HEADER_NAME, cronetResponseInfo); - - // If all content encodings are those known to Cronet natively, Cronet decodes the body stream. - // Otherwise, it's sent to the callbacks verbatim. For consistency with OkHttp, we only leave - // the Content-Encoding headers if Cronet didn't decode the request. Similarly, for consistency, - // we strip the Content-Length header of decoded responses. - - @Nullable String contentLengthString = null; - - // Theoretically, the content encodings can be scattered across multiple comma separated - // Content-Encoding headers. This list contains individual encodings. - List contentEncodingItems = new ArrayList<>(); - - for (String contentEncodingHeaderValue : - getOrDefault( - cronetResponseInfo.getAllHeaders(), - CONTENT_ENCODING_HEADER_NAME, - Collections.emptyList())) { - Iterables.addAll(contentEncodingItems, COMMA_SPLITTER.split(contentEncodingHeaderValue)); - } + companion object { + private const val CONTENT_LENGTH_HEADER_NAME = "Content-Length" + private const val CONTENT_TYPE_HEADER_NAME = "Content-Type" + private const val CONTENT_ENCODING_HEADER_NAME = "Content-Encoding" + + // https://source.chromium.org/search?q=symbol:FilterSourceStream::ParseEncodingType%20f:cc + private val ENCODINGS_HANDLED_BY_CRONET: ImmutableSet = + ImmutableSet.of("br", "deflate", "gzip", "x-gzip") + + private val COMMA_SPLITTER: Splitter = Splitter.on(',').trimResults().omitEmptyStrings() + + @Throws(IOException::class) + private fun createResponse( + request: Request, cronetResponseInfo: UrlResponseInfo, bodySource: Source? + ): Response.Builder { + val responseBuilder = Response.Builder() + + val contentType: String? = getLastHeaderValue(CONTENT_TYPE_HEADER_NAME, cronetResponseInfo) + + // If all content encodings are those known to Cronet natively, Cronet decodes the body stream. + // Otherwise, it's sent to the callbacks verbatim. For consistency with OkHttp, we only leave + // the Content-Encoding headers if Cronet didn't decode the request. Similarly, for consistency, + // we strip the Content-Length header of decoded responses. + var contentLengthString: String? = null + + // Theoretically, the content encodings can be scattered across multiple comma separated + // Content-Encoding headers. This list contains individual encodings. + val contentEncodingItems: List = buildList { + val headerMap = cronetResponseInfo.headers.asMap + headerMap[CONTENT_ENCODING_HEADER_NAME]?.forEach { + addAll(COMMA_SPLITTER.split(it)) + } + } - boolean keepEncodingAffectedHeaders = + val keepEncodingAffectedHeaders = contentEncodingItems.isEmpty() - || !ENCODINGS_HANDLED_BY_CRONET.containsAll(contentEncodingItems); + || !ENCODINGS_HANDLED_BY_CRONET.containsAll(contentEncodingItems) - if (keepEncodingAffectedHeaders) { - contentLengthString = getLastHeaderValue(CONTENT_LENGTH_HEADER_NAME, cronetResponseInfo); - } + if (keepEncodingAffectedHeaders) { + contentLengthString = getLastHeaderValue(CONTENT_LENGTH_HEADER_NAME, cronetResponseInfo) + } - ResponseBody responseBody = null; - if (bodySource != null) { - responseBody = + var responseBody: ResponseBody? = null + if (bodySource != null) { + responseBody = createResponseBody( - request, - cronetResponseInfo.getHttpStatusCode(), - contentType, - contentLengthString, - bodySource); - } + request, + cronetResponseInfo.httpStatusCode, + contentType, + contentLengthString, + bodySource + ) + } - responseBuilder + responseBuilder .request(request) - .code(cronetResponseInfo.getHttpStatusCode()) - .message(cronetResponseInfo.getHttpStatusText()) - .protocol(convertProtocol(cronetResponseInfo.getNegotiatedProtocol())) - .body(responseBody); + .code(cronetResponseInfo.httpStatusCode) + .message(cronetResponseInfo.httpStatusText) + .protocol(convertProtocol(cronetResponseInfo.negotiatedProtocol)) + .apply { + if (responseBody != null) { + body(responseBody) + } + } - for (Map.Entry header : cronetResponseInfo.getAllHeadersAsList()) { - boolean copyHeader = true; - if (!keepEncodingAffectedHeaders) { - if (Ascii.equalsIgnoreCase(header.getKey(), CONTENT_LENGTH_HEADER_NAME) - || Ascii.equalsIgnoreCase(header.getKey(), CONTENT_ENCODING_HEADER_NAME)) { - copyHeader = false; + for (header in cronetResponseInfo.headers.asList) { + var copyHeader = true + if (!keepEncodingAffectedHeaders) { + if (Ascii.equalsIgnoreCase(header.key, CONTENT_LENGTH_HEADER_NAME) + || Ascii.equalsIgnoreCase(header.key, CONTENT_ENCODING_HEADER_NAME) + ) { + copyHeader = false + } + } + if (copyHeader) { + responseBuilder.addHeader(header.key, header.value) } } - if (copyHeader) { - responseBuilder.addHeader(header.getKey(), header.getValue()); - } - } - - return responseBuilder; - } - - /** - * Creates an OkHttp's ResponseBody from the OkHttp-Cronet bridging callback. - * - *

As long as the callback's {@code UrlResponseInfo} is available this method is non-blocking. - * However, this method doesn't fetch the entire body response. As a result, subsequent calls to - * {@link ResponseBody} methods might block further to fetch parts of the body. - */ - private static ResponseBody createResponseBody( - Request request, - int httpStatusCode, - @Nullable String contentType, - @Nullable String contentLengthString, - Source bodySource) - throws IOException { - long contentLength; - - // Ignore content-length header for HEAD requests (consistency with OkHttp) - if (request.method().equals("HEAD")) { - contentLength = 0; - } else { - try { - contentLength = contentLengthString != null ? Long.parseLong(contentLengthString) : -1; - } catch (NumberFormatException e) { - // TODO(danstahr): add logging - contentLength = -1; - } + return responseBuilder } - // Check for absence of body in No Content / Reset Content responses (OkHttp consistency) - if ((httpStatusCode == 204 || httpStatusCode == 205) && contentLength > 0) { - throw new ProtocolException( - "HTTP " + httpStatusCode + " had non-zero Content-Length: " + contentLengthString); - } + /** + * Creates an OkHttp's ResponseBody from the OkHttp-Cronet bridging callback. + * + * + * As long as the callback's `UrlResponseInfo` is available this method is non-blocking. + * However, this method doesn't fetch the entire body response. As a result, subsequent calls to + * [ResponseBody] methods might block further to fetch parts of the body. + */ + @Throws(IOException::class) + private fun createResponseBody( + request: Request, + httpStatusCode: Int, + contentType: String?, + contentLengthString: String?, + bodySource: Source + ): ResponseBody { + // Ignore content-length header for HEAD requests (consistency with OkHttp) + val contentLength: Long = if (request.method == "HEAD") { + 0 + } else { + try { + contentLengthString?.toLong() ?: -1 + } catch (e: NumberFormatException) { + // TODO(danstahr): add logging + -1 + } + } - return ResponseBody.create( - contentType != null ? MediaType.parse(contentType) : null, - contentLength, - Okio.buffer(bodySource)); - } + // Check for absence of body in No Content / Reset Content responses (OkHttp consistency) + if ((httpStatusCode == 204 || httpStatusCode == 205) && contentLength > 0) { + throw ProtocolException( + "HTTP $httpStatusCode had non-zero Content-Length: $contentLengthString" + ) + } - /** Converts Cronet's negotiated protocol string to OkHttp's {@link Protocol}. */ - private static Protocol convertProtocol(String negotiatedProtocol) { - // See - // https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids - if (negotiatedProtocol.contains("quic")) { - return Protocol.QUIC; - } else if (negotiatedProtocol.contains("h3")) { - // TODO(danstahr): Should be h3 for newer OkHttp - return Protocol.QUIC; - } else if (negotiatedProtocol.contains("spdy")) { - return Protocol.HTTP_2; - } else if (negotiatedProtocol.contains("h2")) { - return Protocol.HTTP_2; - } else if (negotiatedProtocol.contains("http/1.1")) { - return Protocol.HTTP_1_1; + return bodySource.buffer() + .asResponseBody( + contentType?.toMediaTypeOrNull(), + contentLength + ) } - return Protocol.HTTP_1_0; - } + /** Converts Cronet's negotiated protocol string to OkHttp's [Protocol]. */ + private fun convertProtocol(negotiatedProtocol: String): Protocol { + // See + // https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids + if (negotiatedProtocol.contains("quic")) { + return Protocol.QUIC + } else if (negotiatedProtocol.contains("h3")) { + // TODO(danstahr): Should be h3 for newer OkHttp + return Protocol.QUIC + } else if (negotiatedProtocol.contains("spdy")) { + return Protocol.HTTP_2 + } else if (negotiatedProtocol.contains("h2")) { + return Protocol.HTTP_2 + } else if (negotiatedProtocol.contains("http/1.1")) { + return Protocol.HTTP_1_1 + } - /** Returns the last header value for the given name, or null if the header isn't present. */ - @Nullable - private static String getLastHeaderValue(String name, UrlResponseInfo responseInfo) { - List headers = responseInfo.getAllHeaders().get(name); - if (headers == null || headers.isEmpty()) { - return null; + return Protocol.HTTP_1_0 } - return Iterables.getLast(headers); - } - private static T getFutureValue(Future future) throws IOException { - try { - return Uninterruptibles.getUninterruptibly(future); - } catch (ExecutionException e) { - throw new IOException(e); + /** Returns the last header value for the given name, or null if the header isn't present. */ + private fun getLastHeaderValue(name: String?, responseInfo: UrlResponseInfo): String? { + val headers = responseInfo.headers.asMap[name] + if (headers == null || headers.isEmpty()) { + return null + } + return Iterables.getLast(headers) } - } - private static V getOrDefault(Map map, K key, @NonNull V defaultValue) { - V value = map.get(key); - if (value == null) { - return checkNotNull(defaultValue); - } else { - return value; + @Throws(IOException::class) + private fun getFutureValue(future: Future): T { + try { + return Uninterruptibles.getUninterruptibly(future) + } catch (e: ExecutionException) { + throw IOException(e) + } } } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.kt index 0b31abcd4d26..4307ecdb1524 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.kt @@ -13,165 +13,158 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -package okhttp3.android.httpengine; - -import android.util.Pair; -import com.google.common.util.concurrent.Futures; -import com.google.common.util.concurrent.SettableFuture; -import okio.Buffer; -import okio.Sink; -import okio.Timeout; -import android.net.http.UploadDataSink; - -import java.io.IOException; -import java.nio.ByteBuffer; -import java.util.concurrent.ArrayBlockingQueue; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.Future; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; - -import static com.google.common.base.Preconditions.checkState; - -final class UploadBodyDataBroker implements Sink { - +package okhttp3.android.httpengine + +import android.os.Build +import androidx.annotation.RequiresExtension +import com.google.common.util.concurrent.Futures +import com.google.common.util.concurrent.SettableFuture +import java.io.IOException +import java.nio.ByteBuffer +import java.util.concurrent.ArrayBlockingQueue +import java.util.concurrent.BlockingQueue +import java.util.concurrent.Future +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicReference +import kotlin.math.min +import okio.Buffer +import okio.Sink +import okio.Timeout + +@RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) +internal class UploadBodyDataBroker : Sink { /** - * The read request calls to {@link android.net.http.UploadDataProvider#read(UploadDataSink, - * ByteBuffer)} associated with this broker that we haven't started handling. + * The read request calls to [android.net.http.UploadDataProvider.read] associated with this broker that we haven't started handling. * - *

We don't expect more than one parallel read call for a single request body provider. + * + * We don't expect more than one parallel read call for a single request body provider. */ - private final BlockingQueue>> pendingRead = - new ArrayBlockingQueue<>(1); + private val pendingRead: BlockingQueue>> = + ArrayBlockingQueue>>(1) /** * Whether the sink has been closed. * - *

Calling close() has no practical use but we check that nobody tries to write to the sink + * + * Calling close() has no practical use but we check that nobody tries to write to the sink * after closing it, which is an indication of misuse. */ - private final AtomicBoolean isClosed = new AtomicBoolean(); + private val isClosed = AtomicBoolean() /** * The exception thrown by the body reading background thread, if any. The exception will be * rethrown every time someone attempts to continue reading the body. */ - private final AtomicReference backgroundReadThrowable = new AtomicReference<>(); + private val backgroundReadThrowable = AtomicReference() /** * Indicates that Cronet is ready to receive another body part. * - *

This method is executed by Cronet's upload data provider. + * + * This method is executed by Cronet's upload data provider. */ - Future enqueueBodyRead(ByteBuffer readBuffer) { - Throwable backgroundThrowable = backgroundReadThrowable.get(); - if (backgroundThrowable != null) { - return Futures.immediateFailedFuture(backgroundThrowable); + fun enqueueBodyRead(readBuffer: ByteBuffer): Future { + backgroundReadThrowable.get()?.let { + return Futures.immediateFailedFuture(it) } - SettableFuture future = SettableFuture.create(); - pendingRead.add(Pair.create(readBuffer, future)); + val future: SettableFuture = SettableFuture.create() + pendingRead.add(Pair(readBuffer, future)) // Properly handle interleaving handleBackgroundReadError / enqueueBodyRead calls. - if ((backgroundThrowable = backgroundReadThrowable.get()) != null) { - future.setException(backgroundThrowable); + backgroundReadThrowable.get()?.let { + future.setException(it) } - return future; + return future } /** * Signals that reading the OkHttp body failed with the given throwable. * - *

This method is executed by the background OkHttp body reading thread. + * This method is executed by the background OkHttp body reading thread. */ - void setBackgroundReadError(Throwable t) { - backgroundReadThrowable.set(t); - Pair> read = pendingRead.poll(); - if (read != null) { - read.second.setException(t); - } + fun setBackgroundReadError(t: Throwable) { + backgroundReadThrowable.set(t) + pendingRead.poll()?.second?.setException(t) } /** * Signals that reading the body has ended and no future bytes will be sent. * - *

This method is executed by the background OkHttp body reading thread. + * + * This method is executed by the background OkHttp body reading thread. */ - void handleEndOfStreamSignal() throws IOException { - if (isClosed.getAndSet(true)) { - throw new IllegalStateException("Already closed"); - } + @Throws(IOException::class) + fun handleEndOfStreamSignal() { + check(!isClosed.getAndSet(true)) { "Already closed" } - getPendingCronetRead().second.set(ReadResult.END_OF_BODY); + this.pendingCronetRead.second.set(ReadResult.END_OF_BODY) } /** * {@inheritDoc} * - *

This method is executed by the background OkHttp body reading thread. + * This method is executed by the background OkHttp body reading thread. */ - @Override - public void write(Buffer source, long byteCount) throws IOException { + override fun write(source: Buffer, byteCount: Long) { // This is just a safeguard, close() is a no-op if the body length contract is honored. - checkState(!isClosed.get()); + check(!isClosed.get()) - long bytesRemaining = byteCount; + var bytesRemaining = byteCount - while (bytesRemaining != 0) { - Pair> payload = getPendingCronetRead(); + while (bytesRemaining != 0L) { + val payload: Pair> = + this.pendingCronetRead - ByteBuffer readBuffer = payload.first; - SettableFuture future = payload.second; + val readBuffer = payload.first + val future: SettableFuture = payload.second - int originalBufferLimit = readBuffer.limit(); - int bytesToDrain = (int) Math.min(originalBufferLimit, bytesRemaining); + val originalBufferLimit = readBuffer.limit() + val bytesToDrain = min(originalBufferLimit.toLong(), bytesRemaining).toInt() - readBuffer.limit(bytesToDrain); + readBuffer.limit(bytesToDrain) try { - long bytesRead = source.read(readBuffer); - if (bytesRead == -1) { - IOException e = new IOException("The source has been exhausted but we expected more!"); - future.setException(e); - throw e; + val bytesRead = source.read(readBuffer).toLong() + if (bytesRead == -1L) { + val e = IOException("The source has been exhausted but we expected more!") + future.setException(e) + throw e } - bytesRemaining -= bytesRead; - readBuffer.limit(originalBufferLimit); - future.set(ReadResult.SUCCESS); - } catch (IOException e) { - future.setException(e); - throw e; + bytesRemaining -= bytesRead + readBuffer.limit(originalBufferLimit) + future.set(ReadResult.SUCCESS) + } catch (e: IOException) { + future.setException(e) + throw e } } } - private Pair> getPendingCronetRead() throws IOException { - try { - return pendingRead.take(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new IOException("Interrupted while waiting for a read to finish!"); + private val pendingCronetRead: Pair> + get() { + try { + return pendingRead.take() + } catch (e: InterruptedException) { + Thread.currentThread().interrupt() + throw IOException("Interrupted while waiting for a read to finish!") + } } - } - @Override - public void close() { - isClosed.set(true); + override fun close() { + isClosed.set(true) } - @Override - public void flush() { + override fun flush() { // Not necessary, we "flush" by sending the data to Cronet straight away when write() is called. // Note that this class is wrapped with a okio buffer so writes to the outer layer won't be // seen by this class immediately. } - @Override - public Timeout timeout() { - return Timeout.NONE; + override fun timeout(): Timeout { + return Timeout.NONE } - enum ReadResult { + internal enum class ReadResult { SUCCESS, END_OF_BODY } From 78ba9fef1eae833b0c76112f71d1be0563b6e54d Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Wed, 13 Aug 2025 06:18:27 +0100 Subject: [PATCH 05/12] cleanup --- .../android/test/HttpEngineBridgeTest.kt | 99 +++++++----- okhttp/api/android/okhttp.api | 63 ++++++++ .../httpengine/HttpEngineCallFactory.kt | 143 +++++++++--------- .../httpengine/HttpEngineInterceptor.kt | 33 ++-- .../HttpEngineTransportResponseBody.kt | 17 +-- .../android/httpengine/RedirectStrategy.kt | 24 +-- .../httpengine/RequestBodyConverter.kt | 5 +- .../httpengine/RequestResponseConverter.kt | 33 ++-- .../RequestResponseConverterBasedBuilder.kt | 4 +- .../android/httpengine/ResponseConverter.kt | 74 +++++---- .../httpengine/UploadBodyDataBroker.kt | 11 +- 11 files changed, 293 insertions(+), 213 deletions(-) diff --git a/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt b/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt index 31e6a999e4fa..4afdf436973b 100644 --- a/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt +++ b/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt @@ -35,54 +35,71 @@ import org.junit.Test @SdkSuppress(minSdkVersion = 34) class HttpEngineBridgeTest { @Test - fun testNewCall() = runTest { - val context = ApplicationProvider.getApplicationContext() + fun testNewCall() = + runTest { + val context = ApplicationProvider.getApplicationContext() - val httpEngine = HttpEngine.Builder(context) - .setStoragePath(context.filesDir.resolve("httpEngine").apply { - mkdirs() - }.path) - .setConnectionMigrationOptions(ConnectionMigrationOptions.Builder() - .setAllowNonDefaultNetworkUsage(MIGRATION_OPTION_ENABLED) - .setDefaultNetworkMigration(MIGRATION_OPTION_ENABLED) - .setPathDegradationMigration(MIGRATION_OPTION_ENABLED) - .build()) - .addQuicHint("www.google.com", 443, 443) - .addQuicHint("google.com", 443, 443) - .setDnsOptions(DnsOptions.Builder() - .setPersistHostCache(DNS_OPTION_ENABLED) - .setPreestablishConnectionsToStaleDnsResults(DNS_OPTION_ENABLED) - .setUseHttpStackDnsResolver(DNS_OPTION_ENABLED) - .setStaleDnsOptions(DnsOptions.StaleDnsOptions.Builder() - .setUseStaleOnNameNotResolved(DNS_OPTION_ENABLED) - .build()) - .build()) - .setEnableQuic(true) - .setQuicOptions(QuicOptions.Builder() - .addAllowedQuicHost("www.google.com") - .addAllowedQuicHost("google.com") - .build()) - .build() + val httpEngine = + HttpEngine + .Builder(context) + .setStoragePath( + context.filesDir + .resolve("httpEngine") + .apply { + mkdirs() + }.path, + ).setConnectionMigrationOptions( + ConnectionMigrationOptions + .Builder() + .setAllowNonDefaultNetworkUsage(MIGRATION_OPTION_ENABLED) + .setDefaultNetworkMigration(MIGRATION_OPTION_ENABLED) + .setPathDegradationMigration(MIGRATION_OPTION_ENABLED) + .build(), + ).addQuicHint("www.google.com", 443, 443) + .addQuicHint("google.com", 443, 443) + .setDnsOptions( + DnsOptions + .Builder() + .setPersistHostCache(DNS_OPTION_ENABLED) + .setPreestablishConnectionsToStaleDnsResults(DNS_OPTION_ENABLED) + .setUseHttpStackDnsResolver(DNS_OPTION_ENABLED) + .setStaleDnsOptions( + DnsOptions.StaleDnsOptions + .Builder() + .setUseStaleOnNameNotResolved(DNS_OPTION_ENABLED) + .build(), + ).build(), + ).setEnableQuic(true) + .setQuicOptions( + QuicOptions + .Builder() + .addAllowedQuicHost("www.google.com") + .addAllowedQuicHost("google.com") + .build(), + ).build() - val httpEngineInterceptor = HttpEngineInterceptor - .newBuilder(httpEngine = httpEngine) - .build() + val httpEngineInterceptor = + HttpEngineInterceptor + .newBuilder(httpEngine = httpEngine) + .build() - val client = OkHttpClient.Builder() - .addInterceptor(httpEngineInterceptor) - .build() + val client = + OkHttpClient + .Builder() + .addInterceptor(httpEngineInterceptor) + .build() - val call = client.newCall(Request("https://google.com/robots.txt".toHttpUrl())) + val call = client.newCall(Request("https://google.com/robots.txt".toHttpUrl())) - val response = call.executeAsync() + val response = call.executeAsync() - println(response.body.string().take(40)) + println(response.body.string().take(40)) - val call2 = client.newCall(Request("https://google.com/robots.txt".toHttpUrl())) + val call2 = client.newCall(Request("https://google.com/robots.txt".toHttpUrl())) - val response2 = call2.executeAsync() + val response2 = call2.executeAsync() - println(response2.body.string().take(40)) - println(response2.protocol) - } + println(response2.body.string().take(40)) + println(response2.protocol) + } } diff --git a/okhttp/api/android/okhttp.api b/okhttp/api/android/okhttp.api index 0e73b572894f..3eba4f9e1cf6 100644 --- a/okhttp/api/android/okhttp.api +++ b/okhttp/api/android/okhttp.api @@ -1285,3 +1285,66 @@ public abstract class okhttp3/WebSocketListener { public fun onOpen (Lokhttp3/WebSocket;Lokhttp3/Response;)V } +public final class okhttp3/android/httpengine/HttpEngineCallFactory : okhttp3/Call$Factory { + public static final field Companion Lokhttp3/android/httpengine/HttpEngineCallFactory$Companion; + public synthetic fun (Lokhttp3/android/httpengine/RequestResponseConverter;Ljava/util/concurrent/ExecutorService;IIILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun newCall (Lokhttp3/Request;)Lokhttp3/Call; +} + +public final class okhttp3/android/httpengine/HttpEngineCallFactory$Builder : okhttp3/android/httpengine/RequestResponseConverterBasedBuilder { + public static final field Companion Lokhttp3/android/httpengine/HttpEngineCallFactory$Builder$Companion; + public synthetic fun build$okhttp (Lokhttp3/android/httpengine/RequestResponseConverter;)Ljava/lang/Object; + public final fun setCallTimeoutMillis (I)Lokhttp3/android/httpengine/HttpEngineCallFactory$Builder; + public final fun setCallbackExecutorService (Ljava/util/concurrent/ExecutorService;)Lokhttp3/android/httpengine/HttpEngineCallFactory$Builder; + public final fun setReadTimeoutMillis (I)Lokhttp3/android/httpengine/HttpEngineCallFactory$Builder; + public final fun setWriteTimeoutMillis (I)Lokhttp3/android/httpengine/HttpEngineCallFactory$Builder; +} + +public final class okhttp3/android/httpengine/HttpEngineCallFactory$Builder$Companion { +} + +public final class okhttp3/android/httpengine/HttpEngineCallFactory$Companion { + public final fun newBuilder (Landroid/net/http/HttpEngine;)Lokhttp3/android/httpengine/HttpEngineCallFactory$Builder; +} + +public final class okhttp3/android/httpengine/HttpEngineInterceptor : java/lang/AutoCloseable, okhttp3/Interceptor { + public static final field Companion Lokhttp3/android/httpengine/HttpEngineInterceptor$Companion; + public synthetic fun (Lokhttp3/android/httpengine/RequestResponseConverter;Lkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun close ()V + public fun intercept (Lokhttp3/Interceptor$Chain;)Lokhttp3/Response; +} + +public final class okhttp3/android/httpengine/HttpEngineInterceptor$Builder : okhttp3/android/httpengine/RequestResponseConverterBasedBuilder { + public synthetic fun build$okhttp (Lokhttp3/android/httpengine/RequestResponseConverter;)Ljava/lang/Object; +} + +public final class okhttp3/android/httpengine/HttpEngineInterceptor$Companion { + public final fun newBuilder (Landroid/net/http/HttpEngine;)Lokhttp3/android/httpengine/HttpEngineInterceptor$Builder; +} + +public final class okhttp3/android/httpengine/HttpEngineTimeoutException : java/io/IOException { + public fun ()V +} + +public abstract class okhttp3/android/httpengine/RedirectStrategy { + public static final field Companion Lokhttp3/android/httpengine/RedirectStrategy$Companion; + public abstract fun followRedirects ()Z + public abstract fun numberOfRedirectsToFollow ()I +} + +public final class okhttp3/android/httpengine/RedirectStrategy$Companion { + public final fun defaultStrategy ()Lokhttp3/android/httpengine/RedirectStrategy; + public final fun withoutRedirects ()Lokhttp3/android/httpengine/RedirectStrategy; +} + +public abstract class okhttp3/android/httpengine/RequestResponseConverterBasedBuilder { + public static final field Companion Lokhttp3/android/httpengine/RequestResponseConverterBasedBuilder$Companion; + public fun (Landroid/net/http/HttpEngine;)V + public final fun build ()Ljava/lang/Object; + public final fun setRedirectStrategy (Lokhttp3/android/httpengine/RedirectStrategy;)Lokhttp3/android/httpengine/RequestResponseConverterBasedBuilder; + public final fun setUploadDataProviderExecutorSize (I)Lokhttp3/android/httpengine/RequestResponseConverterBasedBuilder; +} + +public final class okhttp3/android/httpengine/RequestResponseConverterBasedBuilder$Companion { +} + diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallFactory.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallFactory.kt index df1cf245db05..54ea2a026c83 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallFactory.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallFactory.kt @@ -42,7 +42,7 @@ class HttpEngineCallFactory private constructor( responseCallbackExecutor: ExecutorService, readTimeoutMillis: Int, writeTimeoutMillis: Int, - callTimeoutMillis: Int + callTimeoutMillis: Int, ) : Call.Factory { private val converter: RequestResponseConverter private val responseCallbackExecutor: ExecutorService @@ -62,15 +62,13 @@ class HttpEngineCallFactory private constructor( this.callTimeoutMillis = callTimeoutMillis } - override fun newCall(request: Request): Call { - return CronetCall(request, this, converter, responseCallbackExecutor) - } + override fun newCall(request: Request): Call = CronetCall(request, this, converter, responseCallbackExecutor) private class CronetCall( private val okHttpRequest: Request, private val motherFactory: HttpEngineCallFactory, private val converter: RequestResponseConverter, - private val responseCallbackExecutor: ExecutorService + private val responseCallbackExecutor: ExecutorService, ) : Call { private val executed = AtomicBoolean() private val canceled = AtomicBoolean() @@ -88,9 +86,7 @@ class HttpEngineCallFactory private constructor( timeout.timeout(motherFactory.callTimeoutMillis.toLong(), TimeUnit.MILLISECONDS) } - override fun request(): Request { - return okHttpRequest - } + override fun request(): Request = okHttpRequest @Throws(IOException::class) override fun execute(): Response { @@ -99,7 +95,9 @@ class HttpEngineCallFactory private constructor( timeout.enter() val requestAndOkHttpResponse: CronetRequestAndOkHttpResponse = converter.convert( - request(), motherFactory.readTimeoutMillis, motherFactory.writeTimeoutMillis + request(), + motherFactory.readTimeoutMillis, + motherFactory.writeTimeoutMillis, ) convertedRequestAndResponse.set(requestAndOkHttpResponse) @@ -124,7 +122,9 @@ class HttpEngineCallFactory private constructor( evaluateExecutionPreconditions() val requestAndOkHttpResponse: CronetRequestAndOkHttpResponse = converter.convert( - request(), motherFactory.readTimeoutMillis, motherFactory.writeTimeoutMillis + request(), + motherFactory.readTimeoutMillis, + motherFactory.writeTimeoutMillis, ) convertedRequestAndResponse.set(requestAndOkHttpResponse) val call = this @@ -151,7 +151,7 @@ class HttpEngineCallFactory private constructor( } } }, - responseCallbackExecutor + responseCallbackExecutor, ) startRequestIfNotCanceled() @@ -164,9 +164,7 @@ class HttpEngineCallFactory private constructor( } } - override fun clone(): Call { - return motherFactory.newCall(request()) - } + override fun clone(): Call = motherFactory.newCall(request()) override fun cancel() { if (canceled.getAndSet(true)) { @@ -177,21 +175,13 @@ class HttpEngineCallFactory private constructor( localConverted?.request?.cancel() // else the cancel signal will be picked up by the execute() / enqueue() methods. } - override fun isExecuted(): Boolean { - return executed.get() - } + override fun isExecuted(): Boolean = executed.get() - override fun isCanceled(): Boolean { - return canceled.get() - } + override fun isCanceled(): Boolean = canceled.get() - override fun timeout(): Timeout { - return timeout - } + override fun timeout(): Timeout = timeout - fun toLoggableString(): String { - return "call to " + request().url.redact() - } + fun toLoggableString(): String = "call to " + request().url.redact() /** * Verifies that the call can be executed and sets the state of the call to "being executed". @@ -232,65 +222,68 @@ class HttpEngineCallFactory private constructor( } class Builder - internal constructor(httpEngine: HttpEngine) : - RequestResponseConverterBasedBuilder(httpEngine) { - private var readTimeoutMillis: Int = DEFAULT_READ_WRITE_TIMEOUT_MILLIS - private var writeTimeoutMillis: Int = DEFAULT_READ_WRITE_TIMEOUT_MILLIS - private var callTimeoutMillis = 0 // No timeout - private var callbackExecutorService: ExecutorService? = null - - fun setReadTimeoutMillis(readTimeoutMillis: Int): Builder { - check(readTimeoutMillis >= 0) { "Read timeout mustn't be negative!" } - this.readTimeoutMillis = readTimeoutMillis - return this - } + internal constructor( + httpEngine: HttpEngine, + ) : RequestResponseConverterBasedBuilder(httpEngine) { + private var readTimeoutMillis: Int = DEFAULT_READ_WRITE_TIMEOUT_MILLIS + private var writeTimeoutMillis: Int = DEFAULT_READ_WRITE_TIMEOUT_MILLIS + private var callTimeoutMillis = 0 // No timeout + private var callbackExecutorService: ExecutorService? = null + + fun setReadTimeoutMillis(readTimeoutMillis: Int): Builder { + check(readTimeoutMillis >= 0) { "Read timeout mustn't be negative!" } + this.readTimeoutMillis = readTimeoutMillis + return this + } - fun setWriteTimeoutMillis(writeTimeoutMillis: Int): Builder { - check(writeTimeoutMillis >= 0) { "Write timeout mustn't be negative!" } - this.writeTimeoutMillis = writeTimeoutMillis - return this - } + fun setWriteTimeoutMillis(writeTimeoutMillis: Int): Builder { + check(writeTimeoutMillis >= 0) { "Write timeout mustn't be negative!" } + this.writeTimeoutMillis = writeTimeoutMillis + return this + } - fun setCallbackExecutorService(callbackExecutorService: ExecutorService?): Builder { - checkNotNull(callbackExecutorService) - this.callbackExecutorService = callbackExecutorService - return this - } + fun setCallbackExecutorService(callbackExecutorService: ExecutorService?): Builder { + checkNotNull(callbackExecutorService) + this.callbackExecutorService = callbackExecutorService + return this + } - fun setCallTimeoutMillis(callTimeoutMillis: Int): Builder { - check(callTimeoutMillis >= 0) { "Call timeout mustn't be negative!" } - this.callTimeoutMillis = callTimeoutMillis + fun setCallTimeoutMillis(callTimeoutMillis: Int): Builder { + check(callTimeoutMillis >= 0) { "Call timeout mustn't be negative!" } + this.callTimeoutMillis = callTimeoutMillis - return this - } + return this + } - override fun build(converter: RequestResponseConverter): HttpEngineCallFactory { - val localCallbackExecutorService = callbackExecutorService ?: - // Consistent with OkHttp impl - Executors.newCachedThreadPool() - - return HttpEngineCallFactory( - converter, - localCallbackExecutorService, - readTimeoutMillis, - writeTimeoutMillis, - callTimeoutMillis - ) - } + override fun build(converter: RequestResponseConverter): HttpEngineCallFactory { + val localCallbackExecutorService = + callbackExecutorService + ?: // Consistent with OkHttp impl + Executors.newCachedThreadPool() + + return HttpEngineCallFactory( + converter, + localCallbackExecutorService, + readTimeoutMillis, + writeTimeoutMillis, + callTimeoutMillis, + ) + } - companion object { - private const val DEFAULT_READ_WRITE_TIMEOUT_MILLIS = 10000 + companion object { + private const val DEFAULT_READ_WRITE_TIMEOUT_MILLIS = 10000 + } } - } companion object { private const val TAG = "CronetCallFactory" - fun newBuilder(httpEngine: HttpEngine): Builder { - return Builder(httpEngine) - } + fun newBuilder(httpEngine: HttpEngine): Builder = Builder(httpEngine) - private fun toCronetCallFactoryResponse(call: CronetCall, response: Response): Response { + private fun toCronetCallFactoryResponse( + call: CronetCall, + response: Response, + ): Response { checkNotNull(response.body) return response @@ -300,8 +293,8 @@ class HttpEngineCallFactory private constructor( override fun customCloseHook() { call.timeout.exit() } - }) - .build() + }, + ).build() } } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt index 87505037a6e9..55d29f38ec52 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt @@ -51,7 +51,10 @@ import okhttp3.ResponseBody * */ @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) -class HttpEngineInterceptor private constructor(converter: RequestResponseConverter?) : Interceptor, AutoCloseable { +class HttpEngineInterceptor private constructor( + converter: RequestResponseConverter?, +) : Interceptor, + AutoCloseable { private val converter: RequestResponseConverter = checkNotNull(converter) private val activeCalls: MutableMap = ConcurrentHashMap() private val scheduledExecutor: ScheduledExecutorService = ScheduledThreadPoolExecutor(1) @@ -79,7 +82,7 @@ class HttpEngineInterceptor private constructor(converter: RequestResponseConver }, CANCELLATION_CHECK_INTERVAL_MILLIS.toLong(), CANCELLATION_CHECK_INTERVAL_MILLIS.toLong(), - TimeUnit.MILLISECONDS + TimeUnit.MILLISECONDS, ) } @@ -116,15 +119,17 @@ class HttpEngineInterceptor private constructor(converter: RequestResponseConver /** A builder for [HttpEngineInterceptor]. */ class Builder - internal constructor(httpEngine: HttpEngine) : - RequestResponseConverterBasedBuilder(httpEngine) { - /** Builds the interceptor. The same builder can be used to build multiple interceptors. */ - override fun build(converter: RequestResponseConverter): HttpEngineInterceptor { - return HttpEngineInterceptor(converter) + internal constructor( + httpEngine: HttpEngine, + ) : RequestResponseConverterBasedBuilder(httpEngine) { + /** Builds the interceptor. The same builder can be used to build multiple interceptors. */ + override fun build(converter: RequestResponseConverter): HttpEngineInterceptor = HttpEngineInterceptor(converter) } - } - private fun toInterceptorResponse(response: Response, call: Call): Response { + private fun toInterceptorResponse( + response: Response, + call: Call, + ): Response { checkNotNull(response.body) if (response.body is HttpEngineInterceptorResponseBody) { @@ -137,8 +142,10 @@ class HttpEngineInterceptor private constructor(converter: RequestResponseConver .build() } - private inner class HttpEngineInterceptorResponseBody(delegate: ResponseBody, private val call: Call) : - HttpEngineTransportResponseBody(delegate) { + private inner class HttpEngineInterceptorResponseBody( + delegate: ResponseBody, + private val call: Call, + ) : HttpEngineTransportResponseBody(delegate) { override fun customCloseHook() { activeCalls.remove(call) } @@ -150,8 +157,6 @@ class HttpEngineInterceptor private constructor(converter: RequestResponseConver private const val CANCELLATION_CHECK_INTERVAL_MILLIS = 500 /** Creates a [HttpEngineInterceptor] builder. */ - fun newBuilder(httpEngine: HttpEngine): Builder { - return Builder(httpEngine) - } + fun newBuilder(httpEngine: HttpEngine): Builder = Builder(httpEngine) } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTransportResponseBody.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTransportResponseBody.kt index 461ed8a9c0f1..c63764a99d02 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTransportResponseBody.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTransportResponseBody.kt @@ -22,19 +22,14 @@ import okhttp3.ResponseBody import okio.BufferedSource @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) -internal abstract class HttpEngineTransportResponseBody protected constructor(private val delegate: ResponseBody) : - ResponseBody() { - override fun contentType(): MediaType? { - return delegate.contentType() - } +internal abstract class HttpEngineTransportResponseBody protected constructor( + private val delegate: ResponseBody, +) : ResponseBody() { + override fun contentType(): MediaType? = delegate.contentType() - override fun contentLength(): Long { - return delegate.contentLength() - } + override fun contentLength(): Long = delegate.contentLength() - override fun source(): BufferedSource { - return delegate.source() - } + override fun source(): BufferedSource = delegate.source() override fun close() { delegate.close() diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.kt index 8a7e41ce28b9..100ccd3b5f81 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.kt @@ -30,23 +30,15 @@ abstract class RedirectStrategy private constructor() { abstract fun numberOfRedirectsToFollow(): Int internal object WithoutRedirectsHolder : RedirectStrategy() { - override fun followRedirects(): Boolean { - return false - } + override fun followRedirects(): Boolean = false - override fun numberOfRedirectsToFollow(): Int { - throw UnsupportedOperationException() - } + override fun numberOfRedirectsToFollow(): Int = throw UnsupportedOperationException() } internal object DefaultRedirectsHolder : RedirectStrategy() { - override fun followRedirects(): Boolean { - return true - } + override fun followRedirects(): Boolean = true - override fun numberOfRedirectsToFollow(): Int { - return DEFAULT_REDIRECTS - } + override fun numberOfRedirectsToFollow(): Int = DEFAULT_REDIRECTS } companion object { @@ -62,16 +54,12 @@ abstract class RedirectStrategy private constructor() { * impossible to retrieve the body of a redirect response. As a result, a dummy empty body will * always be provided. */ - fun withoutRedirects(): RedirectStrategy { - return WithoutRedirectsHolder - } + fun withoutRedirects(): RedirectStrategy = WithoutRedirectsHolder /** * Returns a strategy which will follow redirects up to [.DEFAULT_REDIRECTS] times. If more * redirects are attempted an exception is thrown. */ - fun defaultStrategy(): RedirectStrategy { - return DefaultRedirectsHolder - } + fun defaultStrategy(): RedirectStrategy = DefaultRedirectsHolder } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.kt index 5c6ad83e43a3..051a255ffdd8 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverter.kt @@ -20,5 +20,8 @@ import okhttp3.RequestBody /** An interface for classes converting from OkHttp to Cronet request bodies. */ internal interface RequestBodyConverter { - fun convertRequestBody(requestBody: RequestBody, writeTimeoutMillis: Int): UploadDataProvider + fun convertRequestBody( + requestBody: RequestBody, + writeTimeoutMillis: Int, + ): UploadDataProvider } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt index b27209e2bb06..d7332706055e 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt @@ -33,7 +33,7 @@ internal class RequestResponseConverter( private val uploadDataProviderExecutor: Executor, private val requestBodyConverter: RequestBodyConverter, private val responseConverter: ResponseConverter, - private val redirectStrategy: RedirectStrategy + private val redirectStrategy: RedirectStrategy, ) { /** * Converts OkHttp's [Request] to a corresponding Cronet's [UrlRequest]. @@ -52,12 +52,14 @@ internal class RequestResponseConverter( * Response okHttpResponse = reqResp.getResponse(); * * // use OkHttp Response as usual - * + * */ @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) @Throws(IOException::class) fun convert( - okHttpRequest: Request, readTimeoutMillis: Int, writeTimeoutMillis: Int + okHttpRequest: Request, + readTimeoutMillis: Int, + writeTimeoutMillis: Int, ): CronetRequestAndOkHttpResponse { val callback = OkHttpBridgeRequestCallback(readTimeoutMillis.toLong(), redirectStrategy) @@ -67,9 +69,10 @@ internal class RequestResponseConverter( val builder: UrlRequest.Builder = httpEngine .newUrlRequestBuilder( - okHttpRequest.url.toString(), MoreExecutors.directExecutor(), callback - ) - .setDirectExecutorAllowed(true) + okHttpRequest.url.toString(), + MoreExecutors.directExecutor(), + callback, + ).setDirectExecutorAllowed(true) builder.setHttpMethod(okHttpRequest.method) @@ -93,33 +96,33 @@ internal class RequestResponseConverter( builder.addHeader(CONTENT_TYPE_HEADER_NAME, CONTENT_TYPE_HEADER_DEFAULT_VALUE) } // else use the header - builder.setUploadDataProvider( requestBodyConverter.convertRequestBody(body, writeTimeoutMillis), - uploadDataProviderExecutor + uploadDataProviderExecutor, ) } } return CronetRequestAndOkHttpResponse( - builder.build(), createResponseSupplier(okHttpRequest, callback) + builder.build(), + createResponseSupplier(okHttpRequest, callback), ) } private fun createResponseSupplier( - request: Request, callback: OkHttpBridgeRequestCallback - ): ResponseSupplier { - return object : ResponseSupplier { + request: Request, + callback: OkHttpBridgeRequestCallback, + ): ResponseSupplier = + object : ResponseSupplier { override val response: Response by lazy { responseConverter.toResponse(request, callback) } override val responseFuture: ListenableFuture by lazy { responseConverter.toResponseAsync( request, - callback + callback, ) } } - } /** A [Future] like holder for OkHttp's [Response]. */ internal interface ResponseSupplier { @@ -132,7 +135,7 @@ internal class RequestResponseConverter( /** A simple data class for bundling Cronet request and OkHttp response. */ internal class CronetRequestAndOkHttpResponse( val request: UrlRequest, - private val responseSupplier: ResponseSupplier + private val responseSupplier: ResponseSupplier, ) { val response: Response get() = responseSupplier.response diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.kt index a42a5734b68b..3d16e11d4af6 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.kt @@ -23,7 +23,7 @@ import java.util.concurrent.Executors @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) abstract class RequestResponseConverterBasedBuilder, T>( - private val httpEngine: HttpEngine + private val httpEngine: HttpEngine, ) { private var uploadDataProviderExecutorSize: Int = DEFAULT_THREAD_POOL_SIZE @@ -66,7 +66,7 @@ abstract class RequestResponseConverterBasedBuilder(callback.urlResponseInfo) val responseBuilder: Response.Builder = @@ -64,7 +67,7 @@ internal class ResponseConverter { if (!redirectResponseInfos.isEmpty()) { check( - urlChain.size == redirectResponseInfos.size + 1 + urlChain.size == redirectResponseInfos.size + 1, ) { "The number of redirects should be consistent across URLs and headers!" } @@ -87,11 +90,12 @@ internal class ResponseConverter { } fun toResponseAsync( - request: Request, callback: OkHttpBridgeRequestCallback - ): ListenableFuture { - return Futures.whenAllComplete(callback.urlResponseInfo, callback.bodySource) + request: Request, + callback: OkHttpBridgeRequestCallback, + ): ListenableFuture = + Futures + .whenAllComplete(callback.urlResponseInfo, callback.bodySource) .call({ toResponse(request, callback) }, MoreExecutors.directExecutor()) - } companion object { private const val CONTENT_LENGTH_HEADER_NAME = "Content-Length" @@ -106,7 +110,9 @@ internal class ResponseConverter { @Throws(IOException::class) private fun createResponse( - request: Request, cronetResponseInfo: UrlResponseInfo, bodySource: Source? + request: Request, + cronetResponseInfo: UrlResponseInfo, + bodySource: Source?, ): Response.Builder { val responseBuilder = Response.Builder() @@ -120,16 +126,17 @@ internal class ResponseConverter { // Theoretically, the content encodings can be scattered across multiple comma separated // Content-Encoding headers. This list contains individual encodings. - val contentEncodingItems: List = buildList { - val headerMap = cronetResponseInfo.headers.asMap - headerMap[CONTENT_ENCODING_HEADER_NAME]?.forEach { - addAll(COMMA_SPLITTER.split(it)) + val contentEncodingItems: List = + buildList { + val headerMap = cronetResponseInfo.headers.asMap + headerMap[CONTENT_ENCODING_HEADER_NAME]?.forEach { + addAll(COMMA_SPLITTER.split(it)) + } } - } val keepEncodingAffectedHeaders = - contentEncodingItems.isEmpty() - || !ENCODINGS_HANDLED_BY_CRONET.containsAll(contentEncodingItems) + contentEncodingItems.isEmpty() || + !ENCODINGS_HANDLED_BY_CRONET.containsAll(contentEncodingItems) if (keepEncodingAffectedHeaders) { contentLengthString = getLastHeaderValue(CONTENT_LENGTH_HEADER_NAME, cronetResponseInfo) @@ -143,7 +150,7 @@ internal class ResponseConverter { cronetResponseInfo.httpStatusCode, contentType, contentLengthString, - bodySource + bodySource, ) } @@ -161,8 +168,8 @@ internal class ResponseConverter { for (header in cronetResponseInfo.headers.asList) { var copyHeader = true if (!keepEncodingAffectedHeaders) { - if (Ascii.equalsIgnoreCase(header.key, CONTENT_LENGTH_HEADER_NAME) - || Ascii.equalsIgnoreCase(header.key, CONTENT_ENCODING_HEADER_NAME) + if (Ascii.equalsIgnoreCase(header.key, CONTENT_LENGTH_HEADER_NAME) || + Ascii.equalsIgnoreCase(header.key, CONTENT_ENCODING_HEADER_NAME) ) { copyHeader = false } @@ -189,31 +196,33 @@ internal class ResponseConverter { httpStatusCode: Int, contentType: String?, contentLengthString: String?, - bodySource: Source + bodySource: Source, ): ResponseBody { // Ignore content-length header for HEAD requests (consistency with OkHttp) - val contentLength: Long = if (request.method == "HEAD") { - 0 - } else { - try { - contentLengthString?.toLong() ?: -1 - } catch (e: NumberFormatException) { - // TODO(danstahr): add logging - -1 + val contentLength: Long = + if (request.method == "HEAD") { + 0 + } else { + try { + contentLengthString?.toLong() ?: -1 + } catch (e: NumberFormatException) { + // TODO(danstahr): add logging + -1 + } } - } // Check for absence of body in No Content / Reset Content responses (OkHttp consistency) if ((httpStatusCode == 204 || httpStatusCode == 205) && contentLength > 0) { throw ProtocolException( - "HTTP $httpStatusCode had non-zero Content-Length: $contentLengthString" + "HTTP $httpStatusCode had non-zero Content-Length: $contentLengthString", ) } - return bodySource.buffer() + return bodySource + .buffer() .asResponseBody( contentType?.toMediaTypeOrNull(), - contentLength + contentLength, ) } @@ -238,7 +247,10 @@ internal class ResponseConverter { } /** Returns the last header value for the given name, or null if the header isn't present. */ - private fun getLastHeaderValue(name: String?, responseInfo: UrlResponseInfo): String? { + private fun getLastHeaderValue( + name: String?, + responseInfo: UrlResponseInfo, + ): String? { val headers = responseInfo.headers.asMap[name] if (headers == null || headers.isEmpty()) { return null diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.kt index 4307ecdb1524..9d1ecbc07f07 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.kt @@ -105,7 +105,10 @@ internal class UploadBodyDataBroker : Sink { * * This method is executed by the background OkHttp body reading thread. */ - override fun write(source: Buffer, byteCount: Long) { + override fun write( + source: Buffer, + byteCount: Long, + ) { // This is just a safeguard, close() is a no-op if the body length contract is honored. check(!isClosed.get()) @@ -160,12 +163,10 @@ internal class UploadBodyDataBroker : Sink { // seen by this class immediately. } - override fun timeout(): Timeout { - return Timeout.NONE - } + override fun timeout(): Timeout = Timeout.NONE internal enum class ReadResult { SUCCESS, - END_OF_BODY + END_OF_BODY, } } From 7ca2d2e1e37f463c03c8c32b0d9ad16e48213c84 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Wed, 13 Aug 2025 06:22:46 +0100 Subject: [PATCH 06/12] add image urls --- .../java/okhttp/android/test/HttpEngineBridgeTest.kt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt b/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt index 4afdf436973b..f502580d95f9 100644 --- a/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt +++ b/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt @@ -34,6 +34,15 @@ import org.junit.Test @SdkSuppress(minSdkVersion = 34) class HttpEngineBridgeTest { + val imageUrls: List = listOf( + "https://storage.googleapis.com/cronet/sun.jpg", + "https://storage.googleapis.com/cronet/flower.jpg", + "https://storage.googleapis.com/cronet/chair.jpg", + "https://storage.googleapis.com/cronet/white.jpg", + "https://storage.googleapis.com/cronet/moka.jpg", + "https://storage.googleapis.com/cronet/walnut.jpg" + ) + @Test fun testNewCall() = runTest { From 7589d1147e9fc6b4485d56ac71172264e5b2b337 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Wed, 13 Aug 2025 06:22:56 +0100 Subject: [PATCH 07/12] add image urls --- .../java/okhttp/android/test/HttpEngineBridgeTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt b/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt index f502580d95f9..49a7b173f56b 100644 --- a/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt +++ b/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt @@ -34,7 +34,7 @@ import org.junit.Test @SdkSuppress(minSdkVersion = 34) class HttpEngineBridgeTest { - val imageUrls: List = listOf( + val imageUrls = listOf( "https://storage.googleapis.com/cronet/sun.jpg", "https://storage.googleapis.com/cronet/flower.jpg", "https://storage.googleapis.com/cronet/chair.jpg", From eb200fff4831363808c922afffe179a1f9c64aa6 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sat, 16 Aug 2025 21:30:47 +0100 Subject: [PATCH 08/12] Another attempt --- .../android/test/HttpEngineBridgeTest.kt | 14 +- okhttp/api/android/okhttp.api | 73 ++--- .../httpengine/HttpEngineCallDecorator.kt | 84 +++++ .../httpengine/HttpEngineCallFactory.kt | 300 ------------------ .../httpengine/HttpEngineInterceptor.kt | 71 +---- .../httpengine/OkHttpBridgeRequestCallback.kt | 100 +++--- .../android/httpengine/RedirectStrategy.kt | 65 ---- .../httpengine/RequestBodyConverterImpl.kt | 119 ++++--- .../httpengine/RequestResponseConverter.kt | 31 +- .../RequestResponseConverterBasedBuilder.kt | 78 ----- 10 files changed, 263 insertions(+), 672 deletions(-) create mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallDecorator.kt delete mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallFactory.kt delete mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.kt delete mode 100644 okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.kt diff --git a/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt b/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt index 49a7b173f56b..2821f44f6f5b 100644 --- a/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt +++ b/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt @@ -28,19 +28,20 @@ import kotlinx.coroutines.test.runTest import okhttp3.HttpUrl.Companion.toHttpUrl import okhttp3.OkHttpClient import okhttp3.Request -import okhttp3.android.httpengine.HttpEngineInterceptor +import okhttp3.android.httpengine.HttpEngineCallDecorator.Companion.callDecorator import okhttp3.coroutines.executeAsync import org.junit.Test @SdkSuppress(minSdkVersion = 34) class HttpEngineBridgeTest { - val imageUrls = listOf( + val imageUrls = + listOf( "https://storage.googleapis.com/cronet/sun.jpg", "https://storage.googleapis.com/cronet/flower.jpg", "https://storage.googleapis.com/cronet/chair.jpg", "https://storage.googleapis.com/cronet/white.jpg", "https://storage.googleapis.com/cronet/moka.jpg", - "https://storage.googleapis.com/cronet/walnut.jpg" + "https://storage.googleapis.com/cronet/walnut.jpg", ) @Test @@ -87,15 +88,10 @@ class HttpEngineBridgeTest { .build(), ).build() - val httpEngineInterceptor = - HttpEngineInterceptor - .newBuilder(httpEngine = httpEngine) - .build() - val client = OkHttpClient .Builder() - .addInterceptor(httpEngineInterceptor) + .addCallDecorator(httpEngine.callDecorator) .build() val call = client.newCall(Request("https://google.com/robots.txt".toHttpUrl())) diff --git a/okhttp/api/android/okhttp.api b/okhttp/api/android/okhttp.api index 3eba4f9e1cf6..e56fdbe6a97f 100644 --- a/okhttp/api/android/okhttp.api +++ b/okhttp/api/android/okhttp.api @@ -1285,66 +1285,33 @@ public abstract class okhttp3/WebSocketListener { public fun onOpen (Lokhttp3/WebSocket;Lokhttp3/Response;)V } -public final class okhttp3/android/httpengine/HttpEngineCallFactory : okhttp3/Call$Factory { - public static final field Companion Lokhttp3/android/httpengine/HttpEngineCallFactory$Companion; - public synthetic fun (Lokhttp3/android/httpengine/RequestResponseConverter;Ljava/util/concurrent/ExecutorService;IIILkotlin/jvm/internal/DefaultConstructorMarker;)V - public fun newCall (Lokhttp3/Request;)Lokhttp3/Call; -} - -public final class okhttp3/android/httpengine/HttpEngineCallFactory$Builder : okhttp3/android/httpengine/RequestResponseConverterBasedBuilder { - public static final field Companion Lokhttp3/android/httpengine/HttpEngineCallFactory$Builder$Companion; - public synthetic fun build$okhttp (Lokhttp3/android/httpengine/RequestResponseConverter;)Ljava/lang/Object; - public final fun setCallTimeoutMillis (I)Lokhttp3/android/httpengine/HttpEngineCallFactory$Builder; - public final fun setCallbackExecutorService (Ljava/util/concurrent/ExecutorService;)Lokhttp3/android/httpengine/HttpEngineCallFactory$Builder; - public final fun setReadTimeoutMillis (I)Lokhttp3/android/httpengine/HttpEngineCallFactory$Builder; - public final fun setWriteTimeoutMillis (I)Lokhttp3/android/httpengine/HttpEngineCallFactory$Builder; -} - -public final class okhttp3/android/httpengine/HttpEngineCallFactory$Builder$Companion { +public final class okhttp3/android/httpengine/HttpEngineCallDecorator : okhttp3/Call$Decorator { + public static final field Companion Lokhttp3/android/httpengine/HttpEngineCallDecorator$Companion; + public fun (Landroid/net/http/HttpEngine;Lkotlin/jvm/functions/Function1;)V + public synthetic fun (Landroid/net/http/HttpEngine;Lkotlin/jvm/functions/Function1;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun newCall (Lokhttp3/Call$Chain;)Lokhttp3/Call; } -public final class okhttp3/android/httpengine/HttpEngineCallFactory$Companion { - public final fun newBuilder (Landroid/net/http/HttpEngine;)Lokhttp3/android/httpengine/HttpEngineCallFactory$Builder; +public final class okhttp3/android/httpengine/HttpEngineCallDecorator$Companion { + public final fun getCallDecorator (Landroid/net/http/HttpEngine;)Lokhttp3/android/httpengine/HttpEngineCallDecorator; } -public final class okhttp3/android/httpengine/HttpEngineInterceptor : java/lang/AutoCloseable, okhttp3/Interceptor { - public static final field Companion Lokhttp3/android/httpengine/HttpEngineInterceptor$Companion; - public synthetic fun (Lokhttp3/android/httpengine/RequestResponseConverter;Lkotlin/jvm/internal/DefaultConstructorMarker;)V - public fun close ()V - public fun intercept (Lokhttp3/Interceptor$Chain;)Lokhttp3/Response; -} - -public final class okhttp3/android/httpengine/HttpEngineInterceptor$Builder : okhttp3/android/httpengine/RequestResponseConverterBasedBuilder { - public synthetic fun build$okhttp (Lokhttp3/android/httpengine/RequestResponseConverter;)Ljava/lang/Object; -} - -public final class okhttp3/android/httpengine/HttpEngineInterceptor$Companion { - public final fun newBuilder (Landroid/net/http/HttpEngine;)Lokhttp3/android/httpengine/HttpEngineInterceptor$Builder; +public final class okhttp3/android/httpengine/HttpEngineCallDecorator$HttpEngineCall : okhttp3/Call { + public fun (Lokhttp3/android/httpengine/HttpEngineCallDecorator;Lokhttp3/Call;)V + public fun cancel ()V + public synthetic fun clone ()Ljava/lang/Object; + public fun clone ()Lokhttp3/Call; + public fun enqueue (Lokhttp3/Callback;)V + public fun execute ()Lokhttp3/Response; + public final fun getHttpEngine ()Landroid/net/http/HttpEngine; + public final fun getRealCall ()Lokhttp3/Call; + public fun isCanceled ()Z + public fun isExecuted ()Z + public fun request ()Lokhttp3/Request; + public fun timeout ()Lokio/Timeout; } public final class okhttp3/android/httpengine/HttpEngineTimeoutException : java/io/IOException { public fun ()V } -public abstract class okhttp3/android/httpengine/RedirectStrategy { - public static final field Companion Lokhttp3/android/httpengine/RedirectStrategy$Companion; - public abstract fun followRedirects ()Z - public abstract fun numberOfRedirectsToFollow ()I -} - -public final class okhttp3/android/httpengine/RedirectStrategy$Companion { - public final fun defaultStrategy ()Lokhttp3/android/httpengine/RedirectStrategy; - public final fun withoutRedirects ()Lokhttp3/android/httpengine/RedirectStrategy; -} - -public abstract class okhttp3/android/httpengine/RequestResponseConverterBasedBuilder { - public static final field Companion Lokhttp3/android/httpengine/RequestResponseConverterBasedBuilder$Companion; - public fun (Landroid/net/http/HttpEngine;)V - public final fun build ()Ljava/lang/Object; - public final fun setRedirectStrategy (Lokhttp3/android/httpengine/RedirectStrategy;)Lokhttp3/android/httpengine/RequestResponseConverterBasedBuilder; - public final fun setUploadDataProviderExecutorSize (I)Lokhttp3/android/httpengine/RequestResponseConverterBasedBuilder; -} - -public final class okhttp3/android/httpengine/RequestResponseConverterBasedBuilder$Companion { -} - diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallDecorator.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallDecorator.kt new file mode 100644 index 000000000000..0bdcbf1f7b1c --- /dev/null +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallDecorator.kt @@ -0,0 +1,84 @@ +package okhttp3.android.httpengine + +import android.annotation.SuppressLint +import android.net.http.HttpEngine +import android.os.Build +import android.os.ext.SdkExtensions +import androidx.annotation.RequiresExtension +import okhttp3.Call +import okhttp3.Interceptor +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.internal.cache.CacheInterceptor +import okhttp3.internal.http.BridgeInterceptor +import okhttp3.internal.http.RetryAndFollowUpInterceptor + +class HttpEngineCallDecorator( + private val httpEngine: HttpEngine, + private val useHttpEngine: (Request) -> Boolean = { isHttpEngineSupported() }, +) : Call.Decorator { + private lateinit var client: OkHttpClient + + @SuppressLint("NewApi") + private val httpEngineInterceptor = HttpEngineInterceptor(httpEngine, client) + + override fun newCall(chain: Call.Chain): Call { + val call = httpEngineCall(chain) + + return call ?: chain.proceed(chain.request) + } + + @SuppressLint("NewApi") + @Synchronized + private fun httpEngineCall(chain: Call.Chain): Call? { + if (!useHttpEngine(chain.request)) { + return null + } + + if (!::client.isInitialized) { + val originalClient = chain.client + client = + originalClient + .newBuilder() + .apply { + networkInterceptors.clear() + + // TODO refactor RetryAndFollowUpInterceptor to not require the Client directly + interceptors += RetryAndFollowUpInterceptor(originalClient) + interceptors += BridgeInterceptor(originalClient.cookieJar) + interceptors += CacheInterceptor(originalClient.cache) + interceptors += httpEngineInterceptor + interceptors += + Interceptor { + throw IllegalStateException("Shouldn't attempt to connect with OkHttp") + } + + // Keep decorators after this one in the new client + callDecorators.subList(0, callDecorators.indexOf(this@HttpEngineCallDecorator) + 1).clear() + }.build() + } + + return HttpEngineCall(client.newCall(chain.request)) + } + + @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) + inner class HttpEngineCall( + val realCall: Call, + ) : Call by realCall { + val httpEngine: HttpEngine + get() = this@HttpEngineCallDecorator.httpEngine + + override fun cancel() { + realCall.cancel() + httpEngineInterceptor.cancelCall(realCall) + } + } + + companion object { + val HttpEngine.callDecorator + get() = HttpEngineCallDecorator(this) + } +} + +private fun isHttpEngineSupported(): Boolean = + Build.VERSION.SDK_INT >= Build.VERSION_CODES.R && SdkExtensions.getExtensionVersion(Build.VERSION_CODES.S) >= 7 diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallFactory.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallFactory.kt deleted file mode 100644 index 54ea2a026c83..000000000000 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallFactory.kt +++ /dev/null @@ -1,300 +0,0 @@ -/* - * Copyright 2022 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package okhttp3.android.httpengine - -import android.net.http.HttpEngine -import android.os.Build -import android.util.Log -import androidx.annotation.RequiresExtension -import com.google.common.util.concurrent.FutureCallback -import com.google.common.util.concurrent.Futures -import java.io.IOException -import java.util.concurrent.ExecutorService -import java.util.concurrent.Executors -import java.util.concurrent.TimeUnit -import java.util.concurrent.atomic.AtomicBoolean -import java.util.concurrent.atomic.AtomicReference -import okhttp3.Call -import okhttp3.Callback -import okhttp3.Request -import okhttp3.Response -import okhttp3.android.httpengine.RequestResponseConverter.CronetRequestAndOkHttpResponse -import okio.AsyncTimeout -import okio.Timeout - -/** A [Call.Factory] implementation using Cronet as the transport layer. */ -@RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) -class HttpEngineCallFactory private constructor( - converter: RequestResponseConverter, - responseCallbackExecutor: ExecutorService, - readTimeoutMillis: Int, - writeTimeoutMillis: Int, - callTimeoutMillis: Int, -) : Call.Factory { - private val converter: RequestResponseConverter - private val responseCallbackExecutor: ExecutorService - private val readTimeoutMillis: Int - private val writeTimeoutMillis: Int - private val callTimeoutMillis: Int - - init { - check(readTimeoutMillis >= 0) { "Read timeout mustn't be negative!" } - check(writeTimeoutMillis >= 0) { "Write timeout mustn't be negative!" } - check(callTimeoutMillis >= 0) { "Call timeout mustn't be negative!" } - - this.converter = converter - this.responseCallbackExecutor = responseCallbackExecutor - this.readTimeoutMillis = readTimeoutMillis - this.writeTimeoutMillis = writeTimeoutMillis - this.callTimeoutMillis = callTimeoutMillis - } - - override fun newCall(request: Request): Call = CronetCall(request, this, converter, responseCallbackExecutor) - - private class CronetCall( - private val okHttpRequest: Request, - private val motherFactory: HttpEngineCallFactory, - private val converter: RequestResponseConverter, - private val responseCallbackExecutor: ExecutorService, - ) : Call { - private val executed = AtomicBoolean() - private val canceled = AtomicBoolean() - private val convertedRequestAndResponse: AtomicReference = - AtomicReference() - internal val timeout: AsyncTimeout - - init { - this.timeout = - object : AsyncTimeout() { - override fun timedOut() { - this@CronetCall.cancel() // Timeout has its own method named cancel - } - } - timeout.timeout(motherFactory.callTimeoutMillis.toLong(), TimeUnit.MILLISECONDS) - } - - override fun request(): Request = okHttpRequest - - @Throws(IOException::class) - override fun execute(): Response { - evaluateExecutionPreconditions() - try { - timeout.enter() - val requestAndOkHttpResponse: CronetRequestAndOkHttpResponse = - converter.convert( - request(), - motherFactory.readTimeoutMillis, - motherFactory.writeTimeoutMillis, - ) - convertedRequestAndResponse.set(requestAndOkHttpResponse) - - startRequestIfNotCanceled() - - return toCronetCallFactoryResponse(this, requestAndOkHttpResponse.response) - } catch (e: RuntimeException) { - // If the request finished successfully don't exit the timeout yet. Reading the body also - // needs to be considered and the body object will take care of exiting it. See - // toCronetCallFactoryResponse() for details. - timeout.exit() - throw e - } catch (e: IOException) { - timeout.exit() - throw e - } - } - - override fun enqueue(responseCallback: Callback) { - try { - timeout.enter() - evaluateExecutionPreconditions() - val requestAndOkHttpResponse: CronetRequestAndOkHttpResponse = - converter.convert( - request(), - motherFactory.readTimeoutMillis, - motherFactory.writeTimeoutMillis, - ) - convertedRequestAndResponse.set(requestAndOkHttpResponse) - val call = this - - Futures.addCallback( - requestAndOkHttpResponse.responseAsync, - object : FutureCallback { - public override fun onSuccess(result: Response) { - try { - responseCallback.onResponse(call, toCronetCallFactoryResponse(call, result)) - } catch (e: IOException) { - // The call factory doesn't really mind this - the application code - // threw an exception while handling the response, they should have taken care - // of it. Just logging the error is consistent with plain OkHttp implementation. - Log.i(TAG, "Callback failure for " + toLoggableString(), e) - } - } - - public override fun onFailure(t: Throwable) { - if (t is IOException) { - responseCallback.onFailure(call, t) - } else { - responseCallback.onFailure(call, IOException(t)) - } - } - }, - responseCallbackExecutor, - ) - - startRequestIfNotCanceled() - } catch (e: IOException) { - // If the request finished successfully don't exit the timeout yet. Reading the body also - // needs to be considered and the body object will take care of exiting it. See - // toCronetCallFactoryResponse() for details. - timeout.exit() - responseCallback.onFailure(this, e) - } - } - - override fun clone(): Call = motherFactory.newCall(request()) - - override fun cancel() { - if (canceled.getAndSet(true)) { - // already canceled - return - } - val localConverted: CronetRequestAndOkHttpResponse? = convertedRequestAndResponse.get() - localConverted?.request?.cancel() // else the cancel signal will be picked up by the execute() / enqueue() methods. - } - - override fun isExecuted(): Boolean = executed.get() - - override fun isCanceled(): Boolean = canceled.get() - - override fun timeout(): Timeout = timeout - - fun toLoggableString(): String = "call to " + request().url.redact() - - /** - * Verifies that the call can be executed and sets the state of the call to "being executed". - * - * @throws IllegalStateException if the request has already been executed. - * @throws IOException if the request was canceled - */ - @Throws(IOException::class) - fun evaluateExecutionPreconditions() { - if (canceled.get()) { - throw IOException("Can't execute canceled requests") - } - check(!executed.getAndSet(true)) { "Already Executed" } - } - - fun startRequestIfNotCanceled() { - val requestAndOkHttpResponse: CronetRequestAndOkHttpResponse? = convertedRequestAndResponse.get() - check(requestAndOkHttpResponse != null) { "convertedRequestAndResponse must be set!" } - - // There might be a race between the execution and cancellation - // evaluateExecutionPreconditions check didn't capture and cancel() might have missed that - // as well. Check once again that the request isn't canceled. - // This way, no matter how the instructions of the two threads are interleaved, we always end - // up with a serialized-like outcome (either cancel() was fully run before execute(), or vice - // versa). - - // Thread 1 (cancel() call) | Thread 2 (execute() call) - // ------------------------------------------------------------------------------- - // canceled = true | if (canceled) throw; - // convertedRequest?.cancel() | convertedRequest = convert(request) - // | if (canceled) convertedRequest.cancel() - if (canceled.get()) { - requestAndOkHttpResponse.request.cancel() - } else { - requestAndOkHttpResponse.request.start() - } - } - } - - class Builder - internal constructor( - httpEngine: HttpEngine, - ) : RequestResponseConverterBasedBuilder(httpEngine) { - private var readTimeoutMillis: Int = DEFAULT_READ_WRITE_TIMEOUT_MILLIS - private var writeTimeoutMillis: Int = DEFAULT_READ_WRITE_TIMEOUT_MILLIS - private var callTimeoutMillis = 0 // No timeout - private var callbackExecutorService: ExecutorService? = null - - fun setReadTimeoutMillis(readTimeoutMillis: Int): Builder { - check(readTimeoutMillis >= 0) { "Read timeout mustn't be negative!" } - this.readTimeoutMillis = readTimeoutMillis - return this - } - - fun setWriteTimeoutMillis(writeTimeoutMillis: Int): Builder { - check(writeTimeoutMillis >= 0) { "Write timeout mustn't be negative!" } - this.writeTimeoutMillis = writeTimeoutMillis - return this - } - - fun setCallbackExecutorService(callbackExecutorService: ExecutorService?): Builder { - checkNotNull(callbackExecutorService) - this.callbackExecutorService = callbackExecutorService - return this - } - - fun setCallTimeoutMillis(callTimeoutMillis: Int): Builder { - check(callTimeoutMillis >= 0) { "Call timeout mustn't be negative!" } - this.callTimeoutMillis = callTimeoutMillis - - return this - } - - override fun build(converter: RequestResponseConverter): HttpEngineCallFactory { - val localCallbackExecutorService = - callbackExecutorService - ?: // Consistent with OkHttp impl - Executors.newCachedThreadPool() - - return HttpEngineCallFactory( - converter, - localCallbackExecutorService, - readTimeoutMillis, - writeTimeoutMillis, - callTimeoutMillis, - ) - } - - companion object { - private const val DEFAULT_READ_WRITE_TIMEOUT_MILLIS = 10000 - } - } - - companion object { - private const val TAG = "CronetCallFactory" - - fun newBuilder(httpEngine: HttpEngine): Builder = Builder(httpEngine) - - private fun toCronetCallFactoryResponse( - call: CronetCall, - response: Response, - ): Response { - checkNotNull(response.body) - - return response - .newBuilder() - .body( - object : HttpEngineTransportResponseBody(response.body) { - override fun customCloseHook() { - call.timeout.exit() - } - }, - ).build() - } - } -} diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt index 55d29f38ec52..ebbd3f70bbbc 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt @@ -18,16 +18,15 @@ package okhttp3.android.httpengine import android.net.http.HttpEngine import android.net.http.UrlRequest import android.os.Build -import android.util.Log import androidx.annotation.RequiresExtension import java.io.IOException import java.lang.AutoCloseable import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.ScheduledExecutorService import java.util.concurrent.ScheduledThreadPoolExecutor -import java.util.concurrent.TimeUnit import okhttp3.Call import okhttp3.Interceptor +import okhttp3.OkHttpClient import okhttp3.Response import okhttp3.ResponseBody @@ -35,57 +34,27 @@ import okhttp3.ResponseBody * An OkHttp interceptor that redirects HTTP traffic to use Cronet instead of using the OkHttp * network stack. * - * * The interceptor should be used as the last application interceptor to ensure that all other * interceptors are visited before sending the request on wire and after a response is returned. * - * * The interceptor is a plug-and-play replacement for the OkHttp stack for the most part, * however, there are some caveats to keep in mind: * - * - * 1. The entirety of OkHttp core is bypassed. This includes caching configuration and network + * 1. The majority of OkHttp core is bypassed. This includes all network * interceptors. - * 1. Some response fields are not being populated due to mismatches between Cronet's and - * OkHttp's architecture. TODO(danstahr): add a concrete list). - * + * 2. Some response fields are not being populated due to mismatches between Cronet's and + * OkHttp's architecture. */ @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) -class HttpEngineInterceptor private constructor( - converter: RequestResponseConverter?, +internal class HttpEngineInterceptor( + httpEngine: HttpEngine, + client: OkHttpClient, ) : Interceptor, AutoCloseable { - private val converter: RequestResponseConverter = checkNotNull(converter) + private val converter: RequestResponseConverter = RequestResponseConverter.build(httpEngine, client) private val activeCalls: MutableMap = ConcurrentHashMap() private val scheduledExecutor: ScheduledExecutorService = ScheduledThreadPoolExecutor(1) - init { - - // TODO(danstahr): There's no other way to know if the call is canceled but polling - // (https://github.com/square/okhttp/issues/7164). - val unusedFuture = - scheduledExecutor.scheduleAtFixedRate( - Runnable { - val activeCallsIterator = - activeCalls.entries.iterator() - while (activeCallsIterator.hasNext()) { - try { - val activeCall = activeCallsIterator.next() - if (activeCall.key!!.isCanceled()) { - activeCallsIterator.remove() - activeCall.value!!.cancel() - } - } catch (e: RuntimeException) { - Log.w(TAG, "Unable to propagate cancellation status", e) - } - } - }, - CANCELLATION_CHECK_INTERVAL_MILLIS.toLong(), - CANCELLATION_CHECK_INTERVAL_MILLIS.toLong(), - TimeUnit.MILLISECONDS, - ) - } - @Throws(IOException::class) override fun intercept(chain: Interceptor.Chain): Response { if (chain.call().isCanceled()) { @@ -95,7 +64,7 @@ class HttpEngineInterceptor private constructor( val request = chain.request() val requestAndOkHttpResponse: RequestResponseConverter.CronetRequestAndOkHttpResponse = - converter.convert(request, chain.readTimeoutMillis(), chain.writeTimeoutMillis()) + converter.convert(request) activeCalls[chain.call()] = requestAndOkHttpResponse.request @@ -117,15 +86,6 @@ class HttpEngineInterceptor private constructor( scheduledExecutor.shutdown() } - /** A builder for [HttpEngineInterceptor]. */ - class Builder - internal constructor( - httpEngine: HttpEngine, - ) : RequestResponseConverterBasedBuilder(httpEngine) { - /** Builds the interceptor. The same builder can be used to build multiple interceptors. */ - override fun build(converter: RequestResponseConverter): HttpEngineInterceptor = HttpEngineInterceptor(converter) - } - private fun toInterceptorResponse( response: Response, call: Call, @@ -142,6 +102,10 @@ class HttpEngineInterceptor private constructor( .build() } + fun cancelCall(call: Call) { + activeCalls.remove(call) + } + private inner class HttpEngineInterceptorResponseBody( delegate: ResponseBody, private val call: Call, @@ -150,13 +114,4 @@ class HttpEngineInterceptor private constructor( activeCalls.remove(call) } } - - companion object { - private const val TAG = "CronetInterceptor" - - private const val CANCELLATION_CHECK_INTERVAL_MILLIS = 500 - - /** Creates a [HttpEngineInterceptor] builder. */ - fun newBuilder(httpEngine: HttpEngine): Builder = Builder(httpEngine) - } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.kt index c8b8714caa39..046b6e41d303 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.kt @@ -23,14 +23,14 @@ import androidx.annotation.RequiresExtension import com.google.common.util.concurrent.ListenableFuture import com.google.common.util.concurrent.SettableFuture import java.io.IOException -import java.net.ProtocolException import java.nio.ByteBuffer -import java.util.* +import java.util.Collections import java.util.concurrent.ArrayBlockingQueue import java.util.concurrent.BlockingQueue import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean import kotlin.concurrent.Volatile +import okhttp3.OkHttpClient import okio.Buffer import okio.Source import okio.Timeout @@ -51,8 +51,9 @@ import okio.Timeout * scheduling, especially when handling cancellations. */ @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) -internal class OkHttpBridgeRequestCallback(readTimeoutMillis: Long, redirectStrategy: RedirectStrategy) : - UrlRequest.Callback { +internal class OkHttpBridgeRequestCallback( + private val client: OkHttpClient, +) : UrlRequest.Callback { /** A bridge between Cronet's asynchronous callbacks and OkHttp's blocking stream-like reads. */ private val bodySourceFuture: SettableFuture = SettableFuture.create() @@ -74,31 +75,13 @@ internal class OkHttpBridgeRequestCallback(readTimeoutMillis: Long, redirectStra /** The response headers. */ private val headersFuture: SettableFuture = SettableFuture.create() - /** The read timeout as specified by OkHttp. * */ - private val readTimeoutMillis: Long - /** The previous responses as reported to [.onRedirectReceived], from oldest to newest. * */ private val urlResponseInfoChain: MutableList = ArrayList() - private val redirectStrategy: RedirectStrategy - /** The request being processed. Set when the request is first seen by the callback. */ @Volatile private var request: UrlRequest? = null - init { - check(readTimeoutMillis >= 0) - - // So that we don't have to special case infinity. Int.MAX_VALUE is ~infinity for all practical - // use cases. - if (readTimeoutMillis == 0L) { - this.readTimeoutMillis = Int.MAX_VALUE.toLong() - } else { - this.readTimeoutMillis = readTimeoutMillis - } - this.redirectStrategy = redirectStrategy - } - val urlResponseInfo: ListenableFuture /** Returns the [UrlResponseInfo] for the request associated with this callback. */ get() = headersFuture @@ -112,42 +95,24 @@ internal class OkHttpBridgeRequestCallback(readTimeoutMillis: Long, redirectStra */ get() = bodySourceFuture - fun getUrlResponseInfoChain(): MutableList { - return Collections.unmodifiableList(urlResponseInfoChain) - } + fun getUrlResponseInfoChain(): MutableList = Collections.unmodifiableList(urlResponseInfoChain) override fun onRedirectReceived( - urlRequest: UrlRequest, urlResponseInfo: UrlResponseInfo, nextUrl: String + urlRequest: UrlRequest, + urlResponseInfo: UrlResponseInfo, + nextUrl: String, ) { - // We shouldn't follow redirects - pass the given UrlResponseInfo as the ultimate result - if (!redirectStrategy.followRedirects()) { - check(headersFuture.set(urlResponseInfo)) - // Note: This might not match the content length headers but we have no way of accessing - // the actual body with current Cronet's APIs (see RedirectStrategy). - check(bodySourceFuture.set(Buffer())) - urlRequest.cancel() - return - } - - // We should follow redirects and we haven't hit the cap yet - urlResponseInfoChain.add(urlResponseInfo) - if (urlResponseInfo.urlChain.size <= redirectStrategy.numberOfRedirectsToFollow()) { - urlRequest.followRedirect() - return - } - - // Cap reached - cancel the request and fail. Exception crafted to match OkHttp. + check(headersFuture.set(urlResponseInfo)) + // Note: This might not match the content length headers but we have no way of accessing + // the actual body with current Cronet's APIs (see RedirectStrategy). + check(bodySourceFuture.set(Buffer())) urlRequest.cancel() - - val e: IOException = - ProtocolException( - "Too many follow-up requests: " + (redirectStrategy.numberOfRedirectsToFollow() + 1) - ) - headersFuture.setException(e) - bodySourceFuture.setException(e) } - override fun onResponseStarted(urlRequest: UrlRequest, urlResponseInfo: UrlResponseInfo) { + override fun onResponseStarted( + urlRequest: UrlRequest, + urlResponseInfo: UrlResponseInfo, + ) { request = urlRequest check(headersFuture.set(urlResponseInfo)) @@ -155,16 +120,25 @@ internal class OkHttpBridgeRequestCallback(readTimeoutMillis: Long, redirectStra } override fun onReadCompleted( - urlRequest: UrlRequest, urlResponseInfo: UrlResponseInfo, byteBuffer: ByteBuffer + urlRequest: UrlRequest, + urlResponseInfo: UrlResponseInfo, + byteBuffer: ByteBuffer, ) { callbackResults.add(CallbackResult(CallbackStep.ON_READ_COMPLETED, byteBuffer, null)) } - override fun onSucceeded(urlRequest: UrlRequest, urlResponseInfo: UrlResponseInfo) { + override fun onSucceeded( + urlRequest: UrlRequest, + urlResponseInfo: UrlResponseInfo, + ) { callbackResults.add(CallbackResult(CallbackStep.ON_SUCCESS, null, null)) } - override fun onFailed(urlRequest: UrlRequest, urlResponseInfo: UrlResponseInfo?, e: HttpException) { + override fun onFailed( + urlRequest: UrlRequest, + urlResponseInfo: UrlResponseInfo?, + e: HttpException, + ) { // If this was called before we start reading the body, the exception will // propagate in the future providing headers and the body wrapper. if (headersFuture.setException(e) && bodySourceFuture.setException(e)) { @@ -176,7 +150,10 @@ internal class OkHttpBridgeRequestCallback(readTimeoutMillis: Long, redirectStra callbackResults.add(CallbackResult(CallbackStep.ON_FAILED, null, e)) } - override fun onCanceled(urlRequest: UrlRequest, responseInfo: UrlResponseInfo?) { + override fun onCanceled( + urlRequest: UrlRequest, + responseInfo: UrlResponseInfo?, + ) { canceled.set(true) callbackResults.add(CallbackResult(CallbackStep.ON_CANCELED, null, null)) @@ -197,7 +174,10 @@ internal class OkHttpBridgeRequestCallback(readTimeoutMillis: Long, redirectStra private var closed = false @Throws(IOException::class) - override fun read(sink: Buffer, byteCount: Long): Long { + override fun read( + sink: Buffer, + byteCount: Long, + ): Long { if (canceled.get()) { throw IOException("The request was canceled!") } @@ -218,7 +198,7 @@ internal class OkHttpBridgeRequestCallback(readTimeoutMillis: Long, redirectStra var result: CallbackResult? try { - result = callbackResults.poll(readTimeoutMillis, TimeUnit.MILLISECONDS) + result = callbackResults.poll(client.readTimeoutMillis.toLong(), TimeUnit.MILLISECONDS) } catch (e: InterruptedException) { Thread.currentThread().interrupt() result = null @@ -279,14 +259,14 @@ internal class OkHttpBridgeRequestCallback(readTimeoutMillis: Long, redirectStra private class CallbackResult( val callbackStep: CallbackStep, val buffer: ByteBuffer?, - val exception: HttpException? + val exception: HttpException?, ) private enum class CallbackStep { ON_READ_COMPLETED, ON_SUCCESS, ON_FAILED, - ON_CANCELED + ON_CANCELED, } companion object { diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.kt deleted file mode 100644 index 100ccd3b5f81..000000000000 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RedirectStrategy.kt +++ /dev/null @@ -1,65 +0,0 @@ -/* - * Copyright 2022 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package okhttp3.android.httpengine - -/** Defines a redirect strategy for the Cronet OkHttp transport layer. */ -abstract class RedirectStrategy private constructor() { - /** - * Returns whether redirects should be followed at all. If set to false, the redirect response - * will be returned. - */ - abstract fun followRedirects(): Boolean - - /** - * Returns the maximum number of redirects to follow. If more redirects are attempted an exception - * should be thrown by the component handling the request. Shouldn't be called at all if [ ][.followRedirects] return false. - */ - abstract fun numberOfRedirectsToFollow(): Int - - internal object WithoutRedirectsHolder : RedirectStrategy() { - override fun followRedirects(): Boolean = false - - override fun numberOfRedirectsToFollow(): Int = throw UnsupportedOperationException() - } - - internal object DefaultRedirectsHolder : RedirectStrategy() { - override fun followRedirects(): Boolean = true - - override fun numberOfRedirectsToFollow(): Int = DEFAULT_REDIRECTS - } - - companion object { - /** The default number of redirects to follow. Should be less than the Chromium wide safeguard. */ - private const val DEFAULT_REDIRECTS = 16 - - /** - * Returns a strategy which will not follow redirects. - * - * - * Note that because of Cronet's limitations - * (https://developer.android.com/guide/topics/connectivity/cronet/lifecycle#overview) it is - * impossible to retrieve the body of a redirect response. As a result, a dummy empty body will - * always be provided. - */ - fun withoutRedirects(): RedirectStrategy = WithoutRedirectsHolder - - /** - * Returns a strategy which will follow redirects up to [.DEFAULT_REDIRECTS] times. If more - * redirects are attempted an exception is thrown. - */ - fun defaultStrategy(): RedirectStrategy = DefaultRedirectsHolder - } -} diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt index 2f2b7ed4b7b4..70451c3724c4 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt @@ -21,10 +21,19 @@ import android.os.Build import androidx.annotation.RequiresExtension import androidx.annotation.VisibleForTesting import com.google.common.base.Verify -import com.google.common.util.concurrent.* +import com.google.common.util.concurrent.FutureCallback +import com.google.common.util.concurrent.Futures +import com.google.common.util.concurrent.ListenableFuture +import com.google.common.util.concurrent.ListeningExecutorService +import com.google.common.util.concurrent.MoreExecutors +import com.google.common.util.concurrent.Uninterruptibles import java.io.IOException import java.nio.ByteBuffer -import java.util.concurrent.* +import java.util.concurrent.Callable +import java.util.concurrent.ExecutionException +import java.util.concurrent.ExecutorService +import java.util.concurrent.TimeUnit +import java.util.concurrent.TimeoutException import kotlin.concurrent.Volatile import okhttp3.RequestBody import okhttp3.android.httpengine.UploadBodyDataBroker.ReadResult @@ -35,10 +44,13 @@ import okio.buffer @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) internal class RequestBodyConverterImpl( private val inMemoryRequestBodyConverter: InMemoryRequestBodyConverter, - private val streamingRequestBodyConverter: StreamingRequestBodyConverter + private val streamingRequestBodyConverter: StreamingRequestBodyConverter, ) : RequestBodyConverter { @Throws(IOException::class) - override fun convertRequestBody(requestBody: RequestBody, writeTimeoutMillis: Int): UploadDataProvider { + override fun convertRequestBody( + requestBody: RequestBody, + writeTimeoutMillis: Int, + ): UploadDataProvider { val contentLength = requestBody.contentLength() if (contentLength == -1L || contentLength > IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES) { return streamingRequestBodyConverter.convertRequestBody(requestBody, writeTimeoutMillis) @@ -65,20 +77,27 @@ internal class RequestBodyConverterImpl( * This is repeated until the entire body has been read. */ @VisibleForTesting - internal class StreamingRequestBodyConverter(private val readerExecutor: ExecutorService) : RequestBodyConverter { + internal class StreamingRequestBodyConverter( + private val readerExecutor: ExecutorService, + ) : RequestBodyConverter { @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) - override fun convertRequestBody(requestBody: RequestBody, writeTimeoutMillis: Int): UploadDataProvider { - return StreamingUploadDataProvider( - requestBody, UploadBodyDataBroker(), readerExecutor, writeTimeoutMillis.toLong() + override fun convertRequestBody( + requestBody: RequestBody, + writeTimeoutMillis: Int, + ): UploadDataProvider = + StreamingUploadDataProvider( + requestBody, + UploadBodyDataBroker(), + readerExecutor, + writeTimeoutMillis.toLong(), ) - } @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) private class StreamingUploadDataProvider( private val okHttpRequestBody: RequestBody, private val broker: UploadBodyDataBroker, readTaskExecutor: ExecutorService, - writeTimeoutMillis: Long + writeTimeoutMillis: Long, ) : UploadDataProvider() { private val readTaskExecutor: ListeningExecutorService = readTaskExecutor as? ListeningExecutorService ?: MoreExecutors.listeningDecorator(readTaskExecutor) @@ -95,12 +114,13 @@ internal class RequestBodyConverterImpl( private var totalBytesReadFromOkHttp: Long = 0 @Throws(IOException::class) - override fun getLength(): Long { - return okHttpRequestBody.contentLength() - } + override fun getLength(): Long = okHttpRequestBody.contentLength() @Throws(IOException::class) - override fun read(uploadDataSink: UploadDataSink, byteBuffer: ByteBuffer) { + override fun read( + uploadDataSink: UploadDataSink, + byteBuffer: ByteBuffer, + ) { ensureReadTaskStarted() if (length == -1L) { @@ -111,7 +131,10 @@ internal class RequestBodyConverterImpl( } @Throws(IOException::class) - fun readKnownBodyLength(uploadDataSink: UploadDataSink, byteBuffer: ByteBuffer) { + fun readKnownBodyLength( + uploadDataSink: UploadDataSink, + byteBuffer: ByteBuffer, + ) { try { val readResult: ReadResult = readFromOkHttp(byteBuffer) @@ -143,8 +166,10 @@ internal class RequestBodyConverterImpl( * result, when we read the advertised number of bytes, we need to make sure that the stream * is indeed finished. */ - @Throws(IOException::class, TimeoutException::class, ExecutionException::class) - fun handleLastBodyRead(uploadDataSink: UploadDataSink, filledByteBuffer: ByteBuffer) { + fun handleLastBodyRead( + uploadDataSink: UploadDataSink, + filledByteBuffer: ByteBuffer, + ) { // We reuse the same buffer for the END_OF_DATA read (it should be non-destructive and if // it overwrites what's in there we don't mind as that's an error anyway). We just need // to make sure we restore the original position afterwards. We don't use mark() / reset() @@ -160,7 +185,7 @@ internal class RequestBodyConverterImpl( Verify.verify( filledByteBuffer.position() == 0, - "END_OF_BODY reads shouldn't write anything to the buffer" + "END_OF_BODY reads shouldn't write anything to the buffer", ) // revert the position change @@ -169,7 +194,10 @@ internal class RequestBodyConverterImpl( uploadDataSink.onReadSucceeded(false) } - fun readUnknownBodyLength(uploadDataSink: UploadDataSink, byteBuffer: ByteBuffer) { + fun readUnknownBodyLength( + uploadDataSink: UploadDataSink, + byteBuffer: ByteBuffer, + ) { try { val readResult: ReadResult = readFromOkHttp(byteBuffer) uploadDataSink.onReadSucceeded(readResult == ReadResult.END_OF_BODY) @@ -192,7 +220,8 @@ internal class RequestBodyConverterImpl( okHttpRequestBody.writeTo(bufferedSink) bufferedSink.flush() broker.handleEndOfStreamSignal() - }) + }, + ) Futures.addCallback( readTaskFuture!!, @@ -203,7 +232,7 @@ internal class RequestBodyConverterImpl( broker.setBackgroundReadError(t) } }, - MoreExecutors.directExecutor() + MoreExecutors.directExecutor(), ) } } @@ -213,7 +242,9 @@ internal class RequestBodyConverterImpl( val positionBeforeRead = byteBuffer.position() val readResult: ReadResult = Uninterruptibles.getUninterruptibly( - broker.enqueueBodyRead(byteBuffer), writeTimeoutMillis, TimeUnit.MILLISECONDS + broker.enqueueBodyRead(byteBuffer), + writeTimeoutMillis, + TimeUnit.MILLISECONDS, ) val bytesRead = byteBuffer.position() - positionBeforeRead totalBytesReadFromOkHttp += bytesRead.toLong() @@ -227,12 +258,12 @@ internal class RequestBodyConverterImpl( companion object { private fun prepareBodyTooLongException( - expectedLength: Long, minActualLength: Long - ): IOException { - return IOException( - "Expected " + expectedLength + " bytes but got at least " + minActualLength + expectedLength: Long, + minActualLength: Long, + ): IOException = + IOException( + "Expected " + expectedLength + " bytes but got at least " + minActualLength, ) - } } } } @@ -249,16 +280,21 @@ internal class RequestBodyConverterImpl( @VisibleForTesting internal class InMemoryRequestBodyConverter : RequestBodyConverter { @Throws(IOException::class) - override fun convertRequestBody(requestBody: RequestBody, writeTimeoutMillis: Int): UploadDataProvider { + override fun convertRequestBody( + requestBody: RequestBody, + writeTimeoutMillis: Int, + ): UploadDataProvider { // content length is immutable by contract val length = requestBody.contentLength() if (length < 0 || length > IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES) { throw IOException( - ("Expected definite length less than " - + IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES - + "but got " - + length) + ( + "Expected definite length less than " + + IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES + + "but got " + + length + ), ) } @@ -267,12 +303,13 @@ internal class RequestBodyConverterImpl( private var isMaterialized = false private val materializedBody = Buffer() - override fun getLength(): Long { - return length - } + override fun getLength(): Long = length @Throws(IOException::class) - override fun read(uploadDataSink: UploadDataSink, byteBuffer: ByteBuffer) { + override fun read( + uploadDataSink: UploadDataSink, + byteBuffer: ByteBuffer, + ) { // We're not expecting any concurrent calls here so a simple flag should be sufficient. if (!isMaterialized) { requestBody.writeTo(materializedBody) @@ -282,7 +319,7 @@ internal class RequestBodyConverterImpl( val actualLength = materializedBody.size if (actualLength != reportedLength) { throw IOException( - "Expected " + reportedLength + " bytes but got " + actualLength + "Expected " + reportedLength + " bytes but got " + actualLength, ) } } @@ -301,10 +338,10 @@ internal class RequestBodyConverterImpl( companion object { private val IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES = (1024 * 1024).toLong() - fun create(bodyReaderExecutor: ExecutorService): RequestBodyConverterImpl { - return RequestBodyConverterImpl( - InMemoryRequestBodyConverter(), StreamingRequestBodyConverter(bodyReaderExecutor) + fun create(bodyReaderExecutor: ExecutorService): RequestBodyConverterImpl = + RequestBodyConverterImpl( + InMemoryRequestBodyConverter(), + StreamingRequestBodyConverter(bodyReaderExecutor), ) - } } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt index d7332706055e..5e84e2a1e682 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt @@ -23,6 +23,8 @@ import com.google.common.util.concurrent.ListenableFuture import com.google.common.util.concurrent.MoreExecutors import java.io.IOException import java.util.concurrent.Executor +import java.util.concurrent.Executors +import okhttp3.OkHttpClient import okhttp3.Request import okhttp3.Response @@ -33,7 +35,7 @@ internal class RequestResponseConverter( private val uploadDataProviderExecutor: Executor, private val requestBodyConverter: RequestBodyConverter, private val responseConverter: ResponseConverter, - private val redirectStrategy: RedirectStrategy, + private val client: OkHttpClient, ) { /** * Converts OkHttp's [Request] to a corresponding Cronet's [UrlRequest]. @@ -56,13 +58,9 @@ internal class RequestResponseConverter( */ @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) @Throws(IOException::class) - fun convert( - okHttpRequest: Request, - readTimeoutMillis: Int, - writeTimeoutMillis: Int, - ): CronetRequestAndOkHttpResponse { + fun convert(okHttpRequest: Request): CronetRequestAndOkHttpResponse { val callback = - OkHttpBridgeRequestCallback(readTimeoutMillis.toLong(), redirectStrategy) + OkHttpBridgeRequestCallback(client) // The OkHttp request callback methods are lightweight, the heavy lifting is done by OkHttp / // app owned threads. Use a direct executor to avoid extra thread hops. @@ -97,7 +95,7 @@ internal class RequestResponseConverter( } // else use the header builder.setUploadDataProvider( - requestBodyConverter.convertRequestBody(body, writeTimeoutMillis), + requestBodyConverter.convertRequestBody(body, client.writeTimeoutMillis), uploadDataProviderExecutor, ) } @@ -148,5 +146,22 @@ internal class RequestResponseConverter( private const val CONTENT_LENGTH_HEADER_NAME = "Content-Length" private const val CONTENT_TYPE_HEADER_NAME = "Content-Type" private const val CONTENT_TYPE_HEADER_DEFAULT_VALUE = "application/octet-stream" + + fun build( + httpEngine: HttpEngine, + client: OkHttpClient, + ): RequestResponseConverter { + val executor = Executors.newCachedThreadPool() + + return RequestResponseConverter( + httpEngine, + executor, + // There must always be enough executors to blocking-read the OkHttp request bodies + // otherwise deadlocks can occur. + RequestBodyConverterImpl.create(executor), + ResponseConverter(), + client, + ) + } } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.kt deleted file mode 100644 index 3d16e11d4af6..000000000000 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverterBasedBuilder.kt +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright 2022 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package okhttp3.android.httpengine - -import android.net.http.HttpEngine -import android.os.Build -import androidx.annotation.RequiresExtension -import com.google.common.base.Preconditions.checkArgument -import java.util.concurrent.Executors - -@RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) -abstract class RequestResponseConverterBasedBuilder, T>( - private val httpEngine: HttpEngine, -) { - private var uploadDataProviderExecutorSize: Int = DEFAULT_THREAD_POOL_SIZE - - // Not setting the default straight away to lazy initialize the object if it ends up not being - // used. - private var redirectStrategy: RedirectStrategy = RedirectStrategy.defaultStrategy() - - /** - * Sets the size of upload data provider executor. The same executor is used for all upload data - * providers within the interceptor. - * - * @see android.net.http.UrlRequest.Builder.setUploadDataProvider - */ - fun setUploadDataProviderExecutorSize(size: Int): B { - checkArgument(size > 0, "The number of threads must be positive!") - uploadDataProviderExecutorSize = size - return this as B - } - - /** - * Sets the strategy for following redirects. - * - * - * Note that the Cronet (i.e. Chromium) wide safeguards will still apply if one attempts to - * follow redirects too many times. - */ - fun setRedirectStrategy(redirectStrategy: RedirectStrategy): B { - this.redirectStrategy = redirectStrategy - return this as B - } - - internal abstract fun build(converter: RequestResponseConverter): T - - fun build(): T { - val converter = - RequestResponseConverter( - httpEngine, - Executors.newFixedThreadPool(uploadDataProviderExecutorSize), - // There must always be enough executors to blocking-read the OkHttp request bodies - // otherwise deadlocks can occur. - RequestBodyConverterImpl.create(Executors.newCachedThreadPool()), - ResponseConverter(), - redirectStrategy, - ) - - return build(converter) - } - - companion object { - private const val DEFAULT_THREAD_POOL_SIZE = 4 - } -} From 4afa454d4f903e8a80456f7815c3ba29590df750 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sat, 16 Aug 2025 21:55:43 +0100 Subject: [PATCH 09/12] Cleanup --- .../android/test/HttpEngineBridgeTest.kt | 125 +++++++++++------- .../httpengine/HttpEngineCallDecorator.kt | 7 +- .../httpengine/HttpEngineInterceptor.kt | 18 +-- .../httpengine/RequestBodyConverterImpl.kt | 16 +-- .../httpengine/RequestResponseConverter.kt | 32 ++--- .../android/httpengine/ResponseConverter.kt | 8 +- 6 files changed, 109 insertions(+), 97 deletions(-) diff --git a/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt b/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt index 2821f44f6f5b..78b58fc21863 100644 --- a/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt +++ b/android-test/src/androidTest/java/okhttp/android/test/HttpEngineBridgeTest.kt @@ -25,15 +25,65 @@ import android.net.http.QuicOptions import androidx.test.core.app.ApplicationProvider import androidx.test.filters.SdkSuppress import kotlinx.coroutines.test.runTest +import okhttp3.Cache import okhttp3.HttpUrl.Companion.toHttpUrl import okhttp3.OkHttpClient import okhttp3.Request import okhttp3.android.httpengine.HttpEngineCallDecorator.Companion.callDecorator import okhttp3.coroutines.executeAsync +import okio.Path.Companion.toPath +import okio.fakefilesystem.FakeFileSystem import org.junit.Test @SdkSuppress(minSdkVersion = 34) class HttpEngineBridgeTest { + val context = ApplicationProvider.getApplicationContext() + + val httpEngine = + HttpEngine + .Builder(context) + .setStoragePath( + context.filesDir + .resolve("httpEngine") + .apply { + mkdirs() + }.path, + ).setConnectionMigrationOptions( + ConnectionMigrationOptions + .Builder() + .setAllowNonDefaultNetworkUsage(MIGRATION_OPTION_ENABLED) + .setDefaultNetworkMigration(MIGRATION_OPTION_ENABLED) + .setPathDegradationMigration(MIGRATION_OPTION_ENABLED) + .build(), + ).addQuicHint("www.google.com", 443, 443) + .addQuicHint("google.com", 443, 443) + .setDnsOptions( + DnsOptions + .Builder() + .setPersistHostCache(DNS_OPTION_ENABLED) + .setPreestablishConnectionsToStaleDnsResults(DNS_OPTION_ENABLED) + .setUseHttpStackDnsResolver(DNS_OPTION_ENABLED) + .setStaleDnsOptions( + DnsOptions.StaleDnsOptions + .Builder() + .setUseStaleOnNameNotResolved(DNS_OPTION_ENABLED) + .build(), + ).build(), + ).setEnableQuic(true) + .setQuicOptions( + QuicOptions + .Builder() + .addAllowedQuicHost("www.google.com") + .addAllowedQuicHost("google.com") + .build(), + ).build() + + var client = + OkHttpClient + .Builder() + .addCallDecorator(httpEngine.callDecorator) + .build() + val imageUrls = listOf( "https://storage.googleapis.com/cronet/sun.jpg", @@ -42,58 +92,11 @@ class HttpEngineBridgeTest { "https://storage.googleapis.com/cronet/white.jpg", "https://storage.googleapis.com/cronet/moka.jpg", "https://storage.googleapis.com/cronet/walnut.jpg", - ) + ).map { it.toHttpUrl() } @Test fun testNewCall() = runTest { - val context = ApplicationProvider.getApplicationContext() - - val httpEngine = - HttpEngine - .Builder(context) - .setStoragePath( - context.filesDir - .resolve("httpEngine") - .apply { - mkdirs() - }.path, - ).setConnectionMigrationOptions( - ConnectionMigrationOptions - .Builder() - .setAllowNonDefaultNetworkUsage(MIGRATION_OPTION_ENABLED) - .setDefaultNetworkMigration(MIGRATION_OPTION_ENABLED) - .setPathDegradationMigration(MIGRATION_OPTION_ENABLED) - .build(), - ).addQuicHint("www.google.com", 443, 443) - .addQuicHint("google.com", 443, 443) - .setDnsOptions( - DnsOptions - .Builder() - .setPersistHostCache(DNS_OPTION_ENABLED) - .setPreestablishConnectionsToStaleDnsResults(DNS_OPTION_ENABLED) - .setUseHttpStackDnsResolver(DNS_OPTION_ENABLED) - .setStaleDnsOptions( - DnsOptions.StaleDnsOptions - .Builder() - .setUseStaleOnNameNotResolved(DNS_OPTION_ENABLED) - .build(), - ).build(), - ).setEnableQuic(true) - .setQuicOptions( - QuicOptions - .Builder() - .addAllowedQuicHost("www.google.com") - .addAllowedQuicHost("google.com") - .build(), - ).build() - - val client = - OkHttpClient - .Builder() - .addCallDecorator(httpEngine.callDecorator) - .build() - val call = client.newCall(Request("https://google.com/robots.txt".toHttpUrl())) val response = call.executeAsync() @@ -107,4 +110,30 @@ class HttpEngineBridgeTest { println(response2.body.string().take(40)) println(response2.protocol) } + + @Test + fun testWithCache() = + runTest { + client = + client + .newBuilder() + .cache(Cache(FakeFileSystem(), "/cache".toPath(), 100_000_000)) + .build() + + repeat(10) { + imageUrls.forEach { + val call = client.newCall(Request(it)) + + val response = call.executeAsync() + + println( + "${response.request.url} cached=${response.cacheResponse != null} " + + response.body + .byteString() + .md5() + .hex(), + ) + } + } + } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallDecorator.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallDecorator.kt index 0bdcbf1f7b1c..1c73e7b48eb6 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallDecorator.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallDecorator.kt @@ -14,13 +14,14 @@ import okhttp3.internal.http.BridgeInterceptor import okhttp3.internal.http.RetryAndFollowUpInterceptor class HttpEngineCallDecorator( - private val httpEngine: HttpEngine, + internal val httpEngine: HttpEngine, private val useHttpEngine: (Request) -> Boolean = { isHttpEngineSupported() }, ) : Call.Decorator { - private lateinit var client: OkHttpClient + // TODO make this work with forked clients + internal lateinit var client: OkHttpClient @SuppressLint("NewApi") - private val httpEngineInterceptor = HttpEngineInterceptor(httpEngine, client) + private val httpEngineInterceptor = HttpEngineInterceptor(this) override fun newCall(chain: Call.Chain): Call { val call = httpEngineCall(chain) diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt index ebbd3f70bbbc..f615b842e58a 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt @@ -15,18 +15,13 @@ */ package okhttp3.android.httpengine -import android.net.http.HttpEngine import android.net.http.UrlRequest import android.os.Build import androidx.annotation.RequiresExtension import java.io.IOException -import java.lang.AutoCloseable import java.util.concurrent.ConcurrentHashMap -import java.util.concurrent.ScheduledExecutorService -import java.util.concurrent.ScheduledThreadPoolExecutor import okhttp3.Call import okhttp3.Interceptor -import okhttp3.OkHttpClient import okhttp3.Response import okhttp3.ResponseBody @@ -47,13 +42,10 @@ import okhttp3.ResponseBody */ @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) internal class HttpEngineInterceptor( - httpEngine: HttpEngine, - client: OkHttpClient, -) : Interceptor, - AutoCloseable { - private val converter: RequestResponseConverter = RequestResponseConverter.build(httpEngine, client) + httpEngineDecorator: HttpEngineCallDecorator, +) : Interceptor { + private val converter: RequestResponseConverter = RequestResponseConverter.build(httpEngineDecorator) private val activeCalls: MutableMap = ConcurrentHashMap() - private val scheduledExecutor: ScheduledExecutorService = ScheduledThreadPoolExecutor(1) @Throws(IOException::class) override fun intercept(chain: Interceptor.Chain): Response { @@ -82,10 +74,6 @@ internal class HttpEngineInterceptor( } } - override fun close() { - scheduledExecutor.shutdown() - } - private fun toInterceptorResponse( response: Response, call: Call, diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt index 70451c3724c4..12ebb409f52e 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt @@ -31,7 +31,6 @@ import java.io.IOException import java.nio.ByteBuffer import java.util.concurrent.Callable import java.util.concurrent.ExecutionException -import java.util.concurrent.ExecutorService import java.util.concurrent.TimeUnit import java.util.concurrent.TimeoutException import kotlin.concurrent.Volatile @@ -78,7 +77,7 @@ internal class RequestBodyConverterImpl( */ @VisibleForTesting internal class StreamingRequestBodyConverter( - private val readerExecutor: ExecutorService, + private val httpEngineCallDecorator: HttpEngineCallDecorator, ) : RequestBodyConverter { @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) override fun convertRequestBody( @@ -88,7 +87,7 @@ internal class RequestBodyConverterImpl( StreamingUploadDataProvider( requestBody, UploadBodyDataBroker(), - readerExecutor, + httpEngineCallDecorator, writeTimeoutMillis.toLong(), ) @@ -96,11 +95,12 @@ internal class RequestBodyConverterImpl( private class StreamingUploadDataProvider( private val okHttpRequestBody: RequestBody, private val broker: UploadBodyDataBroker, - readTaskExecutor: ExecutorService, + httpEngineCallDecorator: HttpEngineCallDecorator, writeTimeoutMillis: Long, ) : UploadDataProvider() { - private val readTaskExecutor: ListeningExecutorService = - readTaskExecutor as? ListeningExecutorService ?: MoreExecutors.listeningDecorator(readTaskExecutor) + private val readTaskExecutor: ListeningExecutorService by lazy { + MoreExecutors.listeningDecorator(httpEngineCallDecorator.client.dispatcher.executorService) + } // So that we don't have to special case infinity. Int.MAX_VALUE is ~infinity for all // practical use cases. @@ -338,10 +338,10 @@ internal class RequestBodyConverterImpl( companion object { private val IN_MEMORY_BODY_LENGTH_THRESHOLD_BYTES = (1024 * 1024).toLong() - fun create(bodyReaderExecutor: ExecutorService): RequestBodyConverterImpl = + fun create(httpEngineCallDecorator: HttpEngineCallDecorator): RequestBodyConverterImpl = RequestBodyConverterImpl( InMemoryRequestBodyConverter(), - StreamingRequestBodyConverter(bodyReaderExecutor), + StreamingRequestBodyConverter(httpEngineCallDecorator), ) } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt index 5e84e2a1e682..2dff4ae48625 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt @@ -15,28 +15,24 @@ */ package okhttp3.android.httpengine -import android.net.http.HttpEngine import android.net.http.UrlRequest import android.os.Build import androidx.annotation.RequiresExtension import com.google.common.util.concurrent.ListenableFuture import com.google.common.util.concurrent.MoreExecutors import java.io.IOException -import java.util.concurrent.Executor -import java.util.concurrent.Executors -import okhttp3.OkHttpClient import okhttp3.Request import okhttp3.Response /** Converts OkHttp requests to Cronet requests. */ @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) internal class RequestResponseConverter( - private val httpEngine: HttpEngine, - private val uploadDataProviderExecutor: Executor, + private val httpEngineDecorator: HttpEngineCallDecorator, private val requestBodyConverter: RequestBodyConverter, private val responseConverter: ResponseConverter, - private val client: OkHttpClient, ) { + private val executor = httpEngineDecorator.client.dispatcher.executorService + /** * Converts OkHttp's [Request] to a corresponding Cronet's [UrlRequest]. * @@ -60,12 +56,12 @@ internal class RequestResponseConverter( @Throws(IOException::class) fun convert(okHttpRequest: Request): CronetRequestAndOkHttpResponse { val callback = - OkHttpBridgeRequestCallback(client) + OkHttpBridgeRequestCallback(httpEngineDecorator.client) // The OkHttp request callback methods are lightweight, the heavy lifting is done by OkHttp / // app owned threads. Use a direct executor to avoid extra thread hops. val builder: UrlRequest.Builder = - httpEngine + httpEngineDecorator.httpEngine .newUrlRequestBuilder( okHttpRequest.url.toString(), MoreExecutors.directExecutor(), @@ -95,7 +91,7 @@ internal class RequestResponseConverter( } // else use the header builder.setUploadDataProvider( - requestBodyConverter.convertRequestBody(body, client.writeTimeoutMillis), + requestBodyConverter.convertRequestBody(body, httpEngineDecorator.client.writeTimeoutMillis), uploadDataProviderExecutor, ) } @@ -147,21 +143,13 @@ internal class RequestResponseConverter( private const val CONTENT_TYPE_HEADER_NAME = "Content-Type" private const val CONTENT_TYPE_HEADER_DEFAULT_VALUE = "application/octet-stream" - fun build( - httpEngine: HttpEngine, - client: OkHttpClient, - ): RequestResponseConverter { - val executor = Executors.newCachedThreadPool() - - return RequestResponseConverter( - httpEngine, - executor, + fun build(httpEngineDecorator: HttpEngineCallDecorator): RequestResponseConverter = + RequestResponseConverter( + httpEngineDecorator, // There must always be enough executors to blocking-read the OkHttp request bodies // otherwise deadlocks can occur. - RequestBodyConverterImpl.create(executor), + RequestBodyConverterImpl.create(httpEngineDecorator), ResponseConverter(), - client, ) - } } } diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.kt index 10c0c4524240..51c7ba231cc0 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.kt @@ -30,12 +30,15 @@ import java.io.IOException import java.net.ProtocolException import java.util.concurrent.ExecutionException import java.util.concurrent.Future +import okhttp3.CipherSuite +import okhttp3.Handshake import okhttp3.MediaType.Companion.toMediaTypeOrNull import okhttp3.Protocol import okhttp3.Request import okhttp3.Response import okhttp3.ResponseBody import okhttp3.ResponseBody.Companion.asResponseBody +import okhttp3.TlsVersion import okio.Source import okio.buffer @@ -159,7 +162,10 @@ internal class ResponseConverter { .code(cronetResponseInfo.httpStatusCode) .message(cronetResponseInfo.httpStatusText) .protocol(convertProtocol(cronetResponseInfo.negotiatedProtocol)) - .apply { + // TODO don't fake this + .handshake( + Handshake(TlsVersion.TLS_1_3, CipherSuite.TLS_AES_128_CCM_8_SHA256, listOf(), { listOf() }), + ).apply { if (responseBody != null) { body(responseBody) } From afa4c38ce44630ca2c2791157659e9f20b9c6f0e Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sat, 16 Aug 2025 21:58:50 +0100 Subject: [PATCH 10/12] Fix --- .../okhttp3/android/httpengine/RequestResponseConverter.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt index 2dff4ae48625..f338637a3047 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt @@ -31,7 +31,9 @@ internal class RequestResponseConverter( private val requestBodyConverter: RequestBodyConverter, private val responseConverter: ResponseConverter, ) { - private val executor = httpEngineDecorator.client.dispatcher.executorService + private val executor by lazy { + httpEngineDecorator.client.dispatcher.executorService + } /** * Converts OkHttp's [Request] to a corresponding Cronet's [UrlRequest]. @@ -92,7 +94,7 @@ internal class RequestResponseConverter( builder.setUploadDataProvider( requestBodyConverter.convertRequestBody(body, httpEngineDecorator.client.writeTimeoutMillis), - uploadDataProviderExecutor, + executor, ) } } From cafb8a738d7fdb3d4301bdfb73f5eac382bc84d9 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sun, 17 Aug 2025 07:13:48 +0100 Subject: [PATCH 11/12] More fixes --- android-test/build.gradle.kts | 6 +++--- okhttp/build.gradle.kts | 2 +- .../okhttp3/android/httpengine/HttpEngineCallDecorator.kt | 2 ++ .../okhttp3/android/httpengine/HttpEngineInterceptor.kt | 2 ++ .../android/httpengine/HttpEngineTransportResponseBody.kt | 2 ++ .../android/httpengine/OkHttpBridgeRequestCallback.kt | 2 ++ .../okhttp3/android/httpengine/RequestBodyConverterImpl.kt | 2 ++ .../okhttp3/android/httpengine/RequestResponseConverter.kt | 2 ++ .../kotlin/okhttp3/android/httpengine/ResponseConverter.kt | 2 ++ .../okhttp3/android/httpengine/UploadBodyDataBroker.kt | 2 ++ regression-test/build.gradle.kts | 4 ++-- 11 files changed, 22 insertions(+), 6 deletions(-) diff --git a/android-test/build.gradle.kts b/android-test/build.gradle.kts index e703eb13f73c..2082799a6363 100644 --- a/android-test/build.gradle.kts +++ b/android-test/build.gradle.kts @@ -9,7 +9,7 @@ plugins { val androidBuild = property("androidBuild").toString().toBoolean() android { - compileSdk = 35 + compileSdk = 36 namespace = "okhttp.android.test" @@ -20,7 +20,7 @@ android { testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" testInstrumentationRunnerArguments += mapOf( "runnerBuilder" to "de.mannodermaus.junit5.AndroidJUnit5Builder", - "notPackage" to "org.bouncycastle", + "notPackage" to "org.bouncycastle,com.google.common", "configurationParameters" to "junit.jupiter.extensions.autodetection.enabled=true" ) } @@ -40,7 +40,7 @@ android { } testOptions { - targetSdk = 34 + targetSdk = 36 unitTests.isIncludeAndroidResources = true } diff --git a/okhttp/build.gradle.kts b/okhttp/build.gradle.kts index e9c0903cae63..9b538c09c3ae 100644 --- a/okhttp/build.gradle.kts +++ b/okhttp/build.gradle.kts @@ -187,7 +187,7 @@ if (platform == "jdk8alpn") { } android { - compileSdk = 35 + compileSdk = 36 namespace = "okhttp.okhttp3" diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallDecorator.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallDecorator.kt index 1c73e7b48eb6..4aff68781681 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallDecorator.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineCallDecorator.kt @@ -9,10 +9,12 @@ import okhttp3.Call import okhttp3.Interceptor import okhttp3.OkHttpClient import okhttp3.Request +import okhttp3.internal.SuppressSignatureCheck import okhttp3.internal.cache.CacheInterceptor import okhttp3.internal.http.BridgeInterceptor import okhttp3.internal.http.RetryAndFollowUpInterceptor +@SuppressSignatureCheck class HttpEngineCallDecorator( internal val httpEngine: HttpEngine, private val useHttpEngine: (Request) -> Boolean = { isHttpEngineSupported() }, diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt index f615b842e58a..617594ef61cd 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineInterceptor.kt @@ -24,6 +24,7 @@ import okhttp3.Call import okhttp3.Interceptor import okhttp3.Response import okhttp3.ResponseBody +import okhttp3.internal.SuppressSignatureCheck /** * An OkHttp interceptor that redirects HTTP traffic to use Cronet instead of using the OkHttp @@ -40,6 +41,7 @@ import okhttp3.ResponseBody * 2. Some response fields are not being populated due to mismatches between Cronet's and * OkHttp's architecture. */ +@SuppressSignatureCheck @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) internal class HttpEngineInterceptor( httpEngineDecorator: HttpEngineCallDecorator, diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTransportResponseBody.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTransportResponseBody.kt index c63764a99d02..05418cc4c36c 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTransportResponseBody.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/HttpEngineTransportResponseBody.kt @@ -19,8 +19,10 @@ import android.os.Build import androidx.annotation.RequiresExtension import okhttp3.MediaType import okhttp3.ResponseBody +import okhttp3.internal.SuppressSignatureCheck import okio.BufferedSource +@SuppressSignatureCheck @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) internal abstract class HttpEngineTransportResponseBody protected constructor( private val delegate: ResponseBody, diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.kt index 046b6e41d303..c3fe2bfac760 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/OkHttpBridgeRequestCallback.kt @@ -31,6 +31,7 @@ import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean import kotlin.concurrent.Volatile import okhttp3.OkHttpClient +import okhttp3.internal.SuppressSignatureCheck import okio.Buffer import okio.Source import okio.Timeout @@ -50,6 +51,7 @@ import okio.Timeout * request in flight (which is safe to assume), and relies on reasonable fairness of thread * scheduling, especially when handling cancellations. */ +@SuppressSignatureCheck @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) internal class OkHttpBridgeRequestCallback( private val client: OkHttpClient, diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt index 12ebb409f52e..8bdef964187e 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestBodyConverterImpl.kt @@ -36,10 +36,12 @@ import java.util.concurrent.TimeoutException import kotlin.concurrent.Volatile import okhttp3.RequestBody import okhttp3.android.httpengine.UploadBodyDataBroker.ReadResult +import okhttp3.internal.SuppressSignatureCheck import okio.Buffer import okio.BufferedSink import okio.buffer +@SuppressSignatureCheck @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) internal class RequestBodyConverterImpl( private val inMemoryRequestBodyConverter: InMemoryRequestBodyConverter, diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt index f338637a3047..ac8c539ea0fb 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/RequestResponseConverter.kt @@ -23,7 +23,9 @@ import com.google.common.util.concurrent.MoreExecutors import java.io.IOException import okhttp3.Request import okhttp3.Response +import okhttp3.internal.SuppressSignatureCheck +@SuppressSignatureCheck /** Converts OkHttp requests to Cronet requests. */ @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) internal class RequestResponseConverter( diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.kt index 51c7ba231cc0..c9a3b7f8ef6f 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/ResponseConverter.kt @@ -39,12 +39,14 @@ import okhttp3.Response import okhttp3.ResponseBody import okhttp3.ResponseBody.Companion.asResponseBody import okhttp3.TlsVersion +import okhttp3.internal.SuppressSignatureCheck import okio.Source import okio.buffer /** * Converts Cronet's responses (or, more precisely, its chunks as they come from Cronet's [ ]), to OkHttp's [Response]. */ +@SuppressSignatureCheck @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) internal class ResponseConverter { /** diff --git a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.kt b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.kt index 9d1ecbc07f07..bb4dfa39789a 100644 --- a/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.kt +++ b/okhttp/src/androidMain/kotlin/okhttp3/android/httpengine/UploadBodyDataBroker.kt @@ -27,10 +27,12 @@ import java.util.concurrent.Future import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicReference import kotlin.math.min +import okhttp3.internal.SuppressSignatureCheck import okio.Buffer import okio.Sink import okio.Timeout +@SuppressSignatureCheck @RequiresExtension(extension = Build.VERSION_CODES.S, version = 7) internal class UploadBodyDataBroker : Sink { /** diff --git a/regression-test/build.gradle.kts b/regression-test/build.gradle.kts index 7b5f28988f73..7f7e5810f66b 100644 --- a/regression-test/build.gradle.kts +++ b/regression-test/build.gradle.kts @@ -4,13 +4,13 @@ plugins { } android { - compileSdk = 35 + compileSdk = 36 namespace = "okhttp.android.regression" defaultConfig { minSdk = 21 - targetSdk = 34 + targetSdk = 36 // Make sure to use the AndroidJUnitRunner (or a sub-class) in order to hook in the JUnit 5 Test Builder testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" From 16432591b8e2be2bfcafd593633a8dd08a10ef53 Mon Sep 17 00:00:00 2001 From: Yuri Schimke Date: Sun, 17 Aug 2025 07:48:51 +0100 Subject: [PATCH 12/12] More fixes --- android-test/build.gradle.kts | 4 ++-- okhttp/build.gradle.kts | 2 +- regression-test/build.gradle.kts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/android-test/build.gradle.kts b/android-test/build.gradle.kts index 2082799a6363..d945f83dd3a1 100644 --- a/android-test/build.gradle.kts +++ b/android-test/build.gradle.kts @@ -9,7 +9,7 @@ plugins { val androidBuild = property("androidBuild").toString().toBoolean() android { - compileSdk = 36 + compileSdk = 35 namespace = "okhttp.android.test" @@ -40,7 +40,7 @@ android { } testOptions { - targetSdk = 36 + targetSdk = 34 unitTests.isIncludeAndroidResources = true } diff --git a/okhttp/build.gradle.kts b/okhttp/build.gradle.kts index 9b538c09c3ae..e9c0903cae63 100644 --- a/okhttp/build.gradle.kts +++ b/okhttp/build.gradle.kts @@ -187,7 +187,7 @@ if (platform == "jdk8alpn") { } android { - compileSdk = 36 + compileSdk = 35 namespace = "okhttp.okhttp3" diff --git a/regression-test/build.gradle.kts b/regression-test/build.gradle.kts index 7f7e5810f66b..7b5f28988f73 100644 --- a/regression-test/build.gradle.kts +++ b/regression-test/build.gradle.kts @@ -4,13 +4,13 @@ plugins { } android { - compileSdk = 36 + compileSdk = 35 namespace = "okhttp.android.regression" defaultConfig { minSdk = 21 - targetSdk = 36 + targetSdk = 34 // Make sure to use the AndroidJUnitRunner (or a sub-class) in order to hook in the JUnit 5 Test Builder testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"