From 0fe7fbcc82b1bcb8f7e2c8e234cc31ded46ff534 Mon Sep 17 00:00:00 2001 From: Anikate-De Date: Tue, 11 Jun 2024 10:38:16 +0530 Subject: [PATCH 1/4] pkgs/ok_http: add passing testRequestCookies --- pkgs/ok_http/example/integration_test/client_test.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/pkgs/ok_http/example/integration_test/client_test.dart b/pkgs/ok_http/example/integration_test/client_test.dart index b888cf6bf4..b49ab706cf 100644 --- a/pkgs/ok_http/example/integration_test/client_test.dart +++ b/pkgs/ok_http/example/integration_test/client_test.dart @@ -25,6 +25,7 @@ Future testConformance() async { testServerErrors(OkHttpClient()); testClose(OkHttpClient.new); testIsolate(OkHttpClient.new); + testRequestCookies(OkHttpClient(), canSendCookieHeaders: true); testResponseCookies(OkHttpClient(), canReceiveSetCookieHeaders: true); }); } From a7bd0915363e37880c57375c6e759dad94ce9701 Mon Sep 17 00:00:00 2001 From: Anikate-De Date: Tue, 11 Jun 2024 10:51:13 +0530 Subject: [PATCH 2/4] create a client configured for each call --- .../example/integration_test/client_test.dart | 25 ++++++++++--------- pkgs/ok_http/lib/src/ok_http_client.dart | 10 +++++++- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/pkgs/ok_http/example/integration_test/client_test.dart b/pkgs/ok_http/example/integration_test/client_test.dart index b49ab706cf..a5f1802972 100644 --- a/pkgs/ok_http/example/integration_test/client_test.dart +++ b/pkgs/ok_http/example/integration_test/client_test.dart @@ -15,17 +15,18 @@ void main() async { Future testConformance() async { group('ok_http client', () { - testRequestBody(OkHttpClient()); - testResponseBody(OkHttpClient(), canStreamResponseBody: false); - testRequestHeaders(OkHttpClient()); - testRequestMethods(OkHttpClient(), preservesMethodCase: true); - testResponseHeaders(OkHttpClient(), supportsFoldedHeaders: false); - testResponseStatusLine(OkHttpClient()); - testCompressedResponseBody(OkHttpClient()); - testServerErrors(OkHttpClient()); - testClose(OkHttpClient.new); - testIsolate(OkHttpClient.new); - testRequestCookies(OkHttpClient(), canSendCookieHeaders: true); - testResponseCookies(OkHttpClient(), canReceiveSetCookieHeaders: true); + // testRequestBody(OkHttpClient()); + // testResponseBody(OkHttpClient(), canStreamResponseBody: false); + // testRequestHeaders(OkHttpClient()); + // testRequestMethods(OkHttpClient(), preservesMethodCase: true); + // testResponseHeaders(OkHttpClient(), supportsFoldedHeaders: false); + // testResponseStatusLine(OkHttpClient()); + // testCompressedResponseBody(OkHttpClient()); + testRedirect(OkHttpClient()); + // testServerErrors(OkHttpClient()); + // testClose(OkHttpClient.new); + // testIsolate(OkHttpClient.new); + // testRequestCookies(OkHttpClient(), canSendCookieHeaders: true); + // testResponseCookies(OkHttpClient(), canReceiveSetCookieHeaders: true); }); } diff --git a/pkgs/ok_http/lib/src/ok_http_client.dart b/pkgs/ok_http/lib/src/ok_http_client.dart index c2d19b90b7..c43f7cbbaf 100644 --- a/pkgs/ok_http/lib/src/ok_http_client.dart +++ b/pkgs/ok_http/lib/src/ok_http_client.dart @@ -112,9 +112,16 @@ class OkHttpClient extends BaseClient { okReqBody, ); + // To configure the client per-request, we create a new client with the + // builder associated with `_client`. + // They share the same connection pool and dispatcher. + // https://square.github.io/okhttp/recipes/#per-call-configuration-kt-java + final reqConfiguredClient = + _client.newBuilder().followRedirects(request.followRedirects).build(); + // `enqueue()` schedules the request to be executed in the future. // https://square.github.io/okhttp/5.x/okhttp/okhttp3/-call/enqueue.html - _client + reqConfiguredClient .newCall(reqBuilder.build()) .enqueue(bindings.Callback.implement(bindings.$CallbackImpl( onResponse: (bindings.Call call, bindings.Response response) { @@ -159,6 +166,7 @@ class OkHttpClient extends BaseClient { headers: responseHeaders, request: request, contentLength: contentLength, + isRedirect: response.isRedirect(), )); }, onFailure: (bindings.Call call, JObject ioException) { From 673f1fdbbbf1e68df2e368b12e031fc118fb93e2 Mon Sep 17 00:00:00 2001 From: Anikate-De Date: Tue, 11 Jun 2024 11:13:40 +0530 Subject: [PATCH 3/4] generate bindings for custom redirect interceptor --- pkgs/ok_http/android/build.gradle | 9 + .../example/ok_http/RedirectInterceptor.kt | 53 +++++ pkgs/ok_http/jnigen.yaml | 1 + pkgs/ok_http/lib/src/jni/bindings.dart | 181 ++++++++++++++++++ 4 files changed, 244 insertions(+) create mode 100644 pkgs/ok_http/android/src/main/kotlin/com/example/ok_http/RedirectInterceptor.kt diff --git a/pkgs/ok_http/android/build.gradle b/pkgs/ok_http/android/build.gradle index ea25148737..c7de868f22 100644 --- a/pkgs/ok_http/android/build.gradle +++ b/pkgs/ok_http/android/build.gradle @@ -26,11 +26,20 @@ rootProject.allprojects { } apply plugin: "com.android.library" +apply plugin: 'kotlin-android' android { if (project.android.hasProperty("namespace")) { namespace = "com.example.ok_http" } + + kotlinOptions { + jvmTarget = '1.8' + } + + sourceSets { + main.java.srcDirs += 'src/main/kotlin' + } // Bumping the plugin compileSdk version requires all clients of this plugin // to bump the version in their app. diff --git a/pkgs/ok_http/android/src/main/kotlin/com/example/ok_http/RedirectInterceptor.kt b/pkgs/ok_http/android/src/main/kotlin/com/example/ok_http/RedirectInterceptor.kt new file mode 100644 index 0000000000..f9b3bd19cf --- /dev/null +++ b/pkgs/ok_http/android/src/main/kotlin/com/example/ok_http/RedirectInterceptor.kt @@ -0,0 +1,53 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// To cause a request failure [with a suitable message] due to too many redirects, +// we need to throw an IOException. This cannot be done using Dart JNI bindings, +// which lead to a deadlock and eventually a `java.net.SocketTimeoutException`. +// https://github.com/dart-lang/native/issues/561 + +package com.example.ok_http + +import okhttp3.Interceptor +import okhttp3.OkHttpClient +import java.io.IOException + +class RedirectInterceptor { + companion object { + + /** + * Adds a redirect interceptor to the OkHttpClient.Builder + * + * @param clientBuilder The `OkHttpClient.Builder` to add the interceptor to + * @param maxRedirects The maximum number of redirects to follow + * @param followRedirects Whether to follow redirects + * + * @return OkHttpClient.Builder + */ + fun addRedirectInterceptor( + clientBuilder: OkHttpClient.Builder, maxRedirects: Int, followRedirects: Boolean + ): OkHttpClient.Builder { + return clientBuilder.addInterceptor(Interceptor { chain -> + + var req = chain.request() + var response = chain.proceed(req) + var redirectCount = 0 + + while (response.isRedirect && followRedirects) { + if (redirectCount > maxRedirects) { + throw IOException("Redirect limit exceeded") + } + + val location = response.header("location") + req = req.newBuilder().url(location!!).build() + response.close() + response = chain.proceed(req) + redirectCount++ + } + + response + }) + } + } +} diff --git a/pkgs/ok_http/jnigen.yaml b/pkgs/ok_http/jnigen.yaml index fbb5bdf776..d0ebd3c547 100644 --- a/pkgs/ok_http/jnigen.yaml +++ b/pkgs/ok_http/jnigen.yaml @@ -27,6 +27,7 @@ classes: - "okhttp3.ConnectionPool" - "okhttp3.Dispatcher" - "okhttp3.Cache" + - "com.example.ok_http.RedirectInterceptor" # Exclude the deprecated methods listed below # They cause syntax errors during the `dart format` step of JNIGen. diff --git a/pkgs/ok_http/lib/src/jni/bindings.dart b/pkgs/ok_http/lib/src/jni/bindings.dart index 53945e6581..71116e07e7 100644 --- a/pkgs/ok_http/lib/src/jni/bindings.dart +++ b/pkgs/ok_http/lib/src/jni/bindings.dart @@ -9437,3 +9437,184 @@ final class $CacheType extends jni.JObjType { return other.runtimeType == ($CacheType) && other is $CacheType; } } + +/// from: com.example.ok_http.RedirectInterceptor$Companion +class RedirectInterceptor_Companion extends jni.JObject { + @override + late final jni.JObjType $type = type; + + RedirectInterceptor_Companion.fromReference( + jni.JReference reference, + ) : super.fromReference(reference); + + static final _class = + jni.JClass.forName(r"com/example/ok_http/RedirectInterceptor$Companion"); + + /// The type which includes information such as the signature of this class. + static const type = $RedirectInterceptor_CompanionType(); + static final _id_addRedirectInterceptor = _class.instanceMethodId( + r"addRedirectInterceptor", + r"(Lokhttp3/OkHttpClient$Builder;IZ)Lokhttp3/OkHttpClient$Builder;", + ); + + static final _addRedirectInterceptor = ProtectedJniExtensions.lookup< + ffi.NativeFunction< + jni.JniResult Function( + ffi.Pointer, + jni.JMethodIDPtr, + ffi.VarArgs< + ( + ffi.Pointer, + ffi.Int64, + ffi.Int64 + )>)>>("globalEnv_CallObjectMethod") + .asFunction< + jni.JniResult Function(ffi.Pointer, jni.JMethodIDPtr, + ffi.Pointer, int, int)>(); + + /// from: public final okhttp3.OkHttpClient$Builder addRedirectInterceptor(okhttp3.OkHttpClient$Builder builder, int i, boolean z) + /// The returned object must be released after use, by calling the [release] method. + OkHttpClient_Builder addRedirectInterceptor( + OkHttpClient_Builder builder, + int i, + bool z, + ) { + return _addRedirectInterceptor( + reference.pointer, + _id_addRedirectInterceptor as jni.JMethodIDPtr, + builder.reference.pointer, + i, + z ? 1 : 0) + .object(const $OkHttpClient_BuilderType()); + } + + static final _id_new0 = _class.constructorId( + r"(Lkotlin/jvm/internal/DefaultConstructorMarker;)V", + ); + + static final _new0 = ProtectedJniExtensions.lookup< + ffi.NativeFunction< + jni.JniResult Function( + ffi.Pointer, + jni.JMethodIDPtr, + ffi.VarArgs<(ffi.Pointer,)>)>>( + "globalEnv_NewObject") + .asFunction< + jni.JniResult Function(ffi.Pointer, jni.JMethodIDPtr, + ffi.Pointer)>(); + + /// from: public void (kotlin.jvm.internal.DefaultConstructorMarker defaultConstructorMarker) + /// The returned object must be released after use, by calling the [release] method. + factory RedirectInterceptor_Companion( + jni.JObject defaultConstructorMarker, + ) { + return RedirectInterceptor_Companion.fromReference(_new0( + _class.reference.pointer, + _id_new0 as jni.JMethodIDPtr, + defaultConstructorMarker.reference.pointer) + .reference); + } +} + +final class $RedirectInterceptor_CompanionType + extends jni.JObjType { + const $RedirectInterceptor_CompanionType(); + + @override + String get signature => + r"Lcom/example/ok_http/RedirectInterceptor$Companion;"; + + @override + RedirectInterceptor_Companion fromReference(jni.JReference reference) => + RedirectInterceptor_Companion.fromReference(reference); + + @override + jni.JObjType get superType => const jni.JObjectType(); + + @override + final superCount = 1; + + @override + int get hashCode => ($RedirectInterceptor_CompanionType).hashCode; + + @override + bool operator ==(Object other) { + return other.runtimeType == ($RedirectInterceptor_CompanionType) && + other is $RedirectInterceptor_CompanionType; + } +} + +/// from: com.example.ok_http.RedirectInterceptor +class RedirectInterceptor extends jni.JObject { + @override + late final jni.JObjType $type = type; + + RedirectInterceptor.fromReference( + jni.JReference reference, + ) : super.fromReference(reference); + + static final _class = + jni.JClass.forName(r"com/example/ok_http/RedirectInterceptor"); + + /// The type which includes information such as the signature of this class. + static const type = $RedirectInterceptorType(); + static final _id_Companion = _class.staticFieldId( + r"Companion", + r"Lcom/example/ok_http/RedirectInterceptor$Companion;", + ); + + /// from: static public final com.example.ok_http.RedirectInterceptor$Companion Companion + /// The returned object must be released after use, by calling the [release] method. + static RedirectInterceptor_Companion get Companion => + _id_Companion.get(_class, const $RedirectInterceptor_CompanionType()); + + static final _id_new0 = _class.constructorId( + r"()V", + ); + + static final _new0 = ProtectedJniExtensions.lookup< + ffi.NativeFunction< + jni.JniResult Function( + ffi.Pointer, + jni.JMethodIDPtr, + )>>("globalEnv_NewObject") + .asFunction< + jni.JniResult Function( + ffi.Pointer, + jni.JMethodIDPtr, + )>(); + + /// from: public void () + /// The returned object must be released after use, by calling the [release] method. + factory RedirectInterceptor() { + return RedirectInterceptor.fromReference( + _new0(_class.reference.pointer, _id_new0 as jni.JMethodIDPtr) + .reference); + } +} + +final class $RedirectInterceptorType extends jni.JObjType { + const $RedirectInterceptorType(); + + @override + String get signature => r"Lcom/example/ok_http/RedirectInterceptor;"; + + @override + RedirectInterceptor fromReference(jni.JReference reference) => + RedirectInterceptor.fromReference(reference); + + @override + jni.JObjType get superType => const jni.JObjectType(); + + @override + final superCount = 1; + + @override + int get hashCode => ($RedirectInterceptorType).hashCode; + + @override + bool operator ==(Object other) { + return other.runtimeType == ($RedirectInterceptorType) && + other is $RedirectInterceptorType; + } +} From a12a7feab32e5771a717a43581b61fb36dff750d Mon Sep 17 00:00:00 2001 From: Anikate-De Date: Tue, 11 Jun 2024 11:36:03 +0530 Subject: [PATCH 4/4] configure call to follow custom redirection interceptor --- .../example/ok_http/RedirectInterceptor.kt | 7 +++--- .../example/integration_test/client_test.dart | 24 +++++++++---------- pkgs/ok_http/lib/src/ok_http_client.dart | 17 +++++++++++-- 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/pkgs/ok_http/android/src/main/kotlin/com/example/ok_http/RedirectInterceptor.kt b/pkgs/ok_http/android/src/main/kotlin/com/example/ok_http/RedirectInterceptor.kt index f9b3bd19cf..9f59e2fc3a 100644 --- a/pkgs/ok_http/android/src/main/kotlin/com/example/ok_http/RedirectInterceptor.kt +++ b/pkgs/ok_http/android/src/main/kotlin/com/example/ok_http/RedirectInterceptor.kt @@ -29,18 +29,17 @@ class RedirectInterceptor { clientBuilder: OkHttpClient.Builder, maxRedirects: Int, followRedirects: Boolean ): OkHttpClient.Builder { return clientBuilder.addInterceptor(Interceptor { chain -> - var req = chain.request() var response = chain.proceed(req) var redirectCount = 0 while (response.isRedirect && followRedirects) { - if (redirectCount > maxRedirects) { + if (redirectCount >= maxRedirects) { throw IOException("Redirect limit exceeded") } - val location = response.header("location") - req = req.newBuilder().url(location!!).build() + val location = response.header("location") ?: break + req = req.newBuilder().url(location).build() response.close() response = chain.proceed(req) redirectCount++ diff --git a/pkgs/ok_http/example/integration_test/client_test.dart b/pkgs/ok_http/example/integration_test/client_test.dart index a5f1802972..83998f4f38 100644 --- a/pkgs/ok_http/example/integration_test/client_test.dart +++ b/pkgs/ok_http/example/integration_test/client_test.dart @@ -15,18 +15,18 @@ void main() async { Future testConformance() async { group('ok_http client', () { - // testRequestBody(OkHttpClient()); - // testResponseBody(OkHttpClient(), canStreamResponseBody: false); - // testRequestHeaders(OkHttpClient()); - // testRequestMethods(OkHttpClient(), preservesMethodCase: true); - // testResponseHeaders(OkHttpClient(), supportsFoldedHeaders: false); - // testResponseStatusLine(OkHttpClient()); - // testCompressedResponseBody(OkHttpClient()); + testRequestBody(OkHttpClient()); + testResponseBody(OkHttpClient(), canStreamResponseBody: false); + testRequestHeaders(OkHttpClient()); + testRequestMethods(OkHttpClient(), preservesMethodCase: true); + testResponseHeaders(OkHttpClient(), supportsFoldedHeaders: false); + testResponseStatusLine(OkHttpClient()); + testCompressedResponseBody(OkHttpClient()); testRedirect(OkHttpClient()); - // testServerErrors(OkHttpClient()); - // testClose(OkHttpClient.new); - // testIsolate(OkHttpClient.new); - // testRequestCookies(OkHttpClient(), canSendCookieHeaders: true); - // testResponseCookies(OkHttpClient(), canReceiveSetCookieHeaders: true); + testServerErrors(OkHttpClient()); + testClose(OkHttpClient.new); + testIsolate(OkHttpClient.new); + testRequestCookies(OkHttpClient(), canSendCookieHeaders: true); + testResponseCookies(OkHttpClient(), canReceiveSetCookieHeaders: true); }); } diff --git a/pkgs/ok_http/lib/src/ok_http_client.dart b/pkgs/ok_http/lib/src/ok_http_client.dart index c43f7cbbaf..3479a236e5 100644 --- a/pkgs/ok_http/lib/src/ok_http_client.dart +++ b/pkgs/ok_http/lib/src/ok_http_client.dart @@ -89,6 +89,8 @@ class OkHttpClient extends BaseClient { var requestHeaders = request.headers; var requestMethod = request.method; var requestBody = await request.finalize().toBytes(); + var maxRedirects = request.maxRedirects; + var followRedirects = request.followRedirects; final responseCompleter = Completer(); @@ -116,8 +118,14 @@ class OkHttpClient extends BaseClient { // builder associated with `_client`. // They share the same connection pool and dispatcher. // https://square.github.io/okhttp/recipes/#per-call-configuration-kt-java - final reqConfiguredClient = - _client.newBuilder().followRedirects(request.followRedirects).build(); + // + // `followRedirects` is set to `false` to handle redirects manually. + // (Since OkHttp sets a hard limit of 20 redirects.) + // https://github.com/square/okhttp/blob/54238b4c713080c3fd32fb1a070fb5d6814c9a09/okhttp/src/main/kotlin/okhttp3/internal/http/RetryAndFollowUpInterceptor.kt#L350 + final reqConfiguredClient = bindings.RedirectInterceptor.Companion + .addRedirectInterceptor(_client.newBuilder().followRedirects(false), + maxRedirects, followRedirects) + .build(); // `enqueue()` schedules the request to be executed in the future. // https://square.github.io/okhttp/5.x/okhttp/okhttp3/-call/enqueue.html @@ -170,6 +178,11 @@ class OkHttpClient extends BaseClient { )); }, onFailure: (bindings.Call call, JObject ioException) { + if (ioException.toString().contains('Redirect limit exceeded')) { + responseCompleter.completeError( + ClientException('Redirect limit exceeded', request.url)); + return; + } responseCompleter.completeError( ClientException(ioException.toString(), request.url)); },