fix: returning values instead of pointers from cronet (#7)

.github/workflows/test-package.yml

@@ -61,7 +61,7 @@ jobs:
         name: Install dependencies
         run: dart pub get
       - id: get_binaries
-        name: Download Cronet Binaries
+        name: Download/Build Cronet Binaries
         run: dart run cronet:setup
       - name: Run VM tests
         run: dart test --platform vm

CHANGELOG.md

@@ -1,3 +1,8 @@
+## 0.0.1+1
+
+* `HttpClientResponse`'s `followRedirects` and `maxRedirects` are now modifiable.
+* Fixed: utf8 decoding of newLocation in case of redirects.
+
 ## 0.0.1
 
 * HttpClient with QUIC, HTTP2, brotli support.

README.md

@@ -8,6 +8,12 @@ This is a [GSoC 2021 project](https://summerofcode.withgoogle.com/projects/#4757
 
 Currently, 64 bit Linux and Windows systems are supported.
 
+## Requirements
+
+1. Dart SDK 2.12.0 or above.
+2. CMake 3.15 or above. (If on windows, Visual Studio 2019 with C++ tools)
+3. C++ compiler. (g++/clang/msvc)
+
 ## Usage
 
 1. Add package as a dependency in your `pubspec.yaml`.

bin/setup.dart

@@ -37,9 +37,6 @@ void placeBinaries(String platform, String fileName) {
   logger.stdout('Done! Cleaning up...');
 
   File(fileName).deleteSync();
-  logger.stdout(
-      '${ansi.green}Done! Cronet support for $platform is now available!'
-      '${ansi.none}');
 }
 
 /// Download `cronet` library from Github Releases.
@@ -63,8 +60,11 @@ Future<void> downloadCronetBinaries(String platform) async {
     } catch (error) {
       Exception("Can't download. Check your network connection!");
     }
-
     placeBinaries(platform, fileName);
+    buildWrapper();
+    logger.stdout(
+        '${ansi.green}Done! Cronet support for $platform is now available!'
+        '${ansi.none}');
   } else {
     logger.stdout('${ansi.yellow}Cronet $platform is already available.'
         ' No need to download.${ansi.none}');
@@ -80,8 +80,12 @@ void buildWrapper() {
   Directory.current = Directory(wrapperPath);
   logger.stdout('Building Wrapper...');
   try {
-    final result = Process.runSync(
-        'cmake', ['CMakeLists.txt', '-B', 'out/${Platform.operatingSystem}']);
+    final result = Process.runSync('cmake', [
+      'CMakeLists.txt',
+      '-B',
+      'out/${Platform.operatingSystem}',
+      '-DCMAKE_BUILD_TYPE=Release'
+    ]);
     print(result.stdout);
     print(result.stderr);
   } catch (error) {
@@ -95,8 +99,8 @@ void buildWrapper() {
     }
     return;
   }
-  var result =
-      Process.runSync('cmake', ['--build', 'out/${Platform.operatingSystem}']);
+  var result = Process.runSync('cmake',
+      ['--build', 'out/${Platform.operatingSystem}', '--config', 'Release']);
   print(result.stdout);
   print(result.stderr);
   if (result.exitCode != 0) return;
@@ -105,7 +109,7 @@ void buildWrapper() {
   Directory(moveLocation).createSync(recursive: true);
   final buildOutputPath = Platform.isLinux
       ? '$wrapperPath/out/${Platform.operatingSystem}/${getWrapperName()}'
-      : '$wrapperPath\\out\\${Platform.operatingSystem}\\Debug\\${getWrapperName()}';
+      : '$wrapperPath\\out\\${Platform.operatingSystem}\\Release\\${getWrapperName()}';
   File(buildOutputPath).copySync('$moveLocation/${getWrapperName()}');
   logger.stdout(
       '${ansi.green}Wrapper moved to $moveLocation. Success!${ansi.none}');

example_dart/bin/example_dart.dart

@@ -10,7 +10,7 @@ void main(List<String> args) {
   for (var i = 0; i < 3; i++) {
     // Demo - with concurrent requests
     client
-        .getUrl(Uri.parse('https://postman-echo.com/headers'))
+        .getUrl(Uri.parse('https://example.com'))
         .then((HttpClientRequest request) {
       if (i == 2) {
         client.close(); // We will shut down the client after 3 connections.
@@ -21,6 +21,8 @@ void main(List<String> args) {
         print(contents);
       }, onDone: () {
         print('cronet implemenation took: ${stopwatch.elapsedMilliseconds} ms');
+      }, onError: (Object e) {
+        print(e);
       });
     });
   }

lib/src/constants.dart

@@ -9,6 +9,7 @@ const tag = 'binaries-v0.0.1';
 const cronetBinaryUrl =
     'https://github.com/google/cronet.dart/releases/download/$tag/';
 const cronetVersion = "86.0.4240.198";
+const wrapperVersion = "1";
 
 const binaryStorageDir = '.dart_tool/cronet/';
 

lib/src/globals.dart

@@ -4,12 +4,24 @@
 
 import 'dart:ffi';
 
+import 'package:cli_util/cli_logging.dart';
+import 'package:ffi/ffi.dart';
+
+import 'constants.dart';
 import 'dylib_handler.dart';
 import 'third_party/cronet/generated_bindings.dart';
 import 'wrapper/generated_bindings.dart';
 
 Wrapper loadAndInitWrapper() {
   final wrapper = Wrapper(loadWrapper());
+  if (wrapperVersion != wrapper.VersionString().cast<Utf8>().toDartString()) {
+    final logger = Logger.standard();
+    final ansi = Ansi(Ansi.terminalSupportsAnsi);
+    logger.stderr('${ansi.red}Wrapper is outdated.${ansi.none}');
+    logger.stdout('Update wrapper by running: '
+        '${ansi.yellow}dart run cronet:setup${ansi.none}');
+    throw Error();
+  }
   // Initialize Dart Native API dynamically.
   wrapper.InitDartApiDL(NativeApi.initializeApiDLData);
   // Registers few cronet functions that are required by the wrapper.
@@ -18,7 +30,10 @@ Wrapper loadAndInitWrapper() {
       cronet.addresses.Cronet_Engine_Shutdown.cast(),
       cronet.addresses.Cronet_Engine_Destroy.cast(),
       cronet.addresses.Cronet_Buffer_Create.cast(),
-      cronet.addresses.Cronet_Buffer_InitWithAlloc.cast());
+      cronet.addresses.Cronet_Buffer_InitWithAlloc.cast(),
+      cronet.addresses.Cronet_UrlResponseInfo_http_status_code_get.cast(),
+      cronet.addresses.Cronet_Error_message_get.cast(),
+      cronet.addresses.Cronet_UrlResponseInfo_http_status_text_get.cast());
   // Registers few cronet functions that are required by the executor
   // run from the wrapper for executing network requests.
   // Casting because of https://github.com/dart-lang/ffigen/issues/22

lib/src/http_callback_handler.dart

@@ -64,21 +64,22 @@ class CallbackHandler {
   }
 
   /// Checks status of an URL response.
-  int statusChecker(Pointer<Cronet_UrlResponseInfo> respInfoPtr, int lBound,
-      int uBound, void Function() callback) {
-    final respCode =
-        cronet.Cronet_UrlResponseInfo_http_status_code_get(respInfoPtr);
+  bool statusChecker(int respCode, Pointer<Utf8> status, int lBound, int uBound,
+      void Function() callback) {
     if (!(respCode >= lBound && respCode <= uBound)) {
       // If NOT in range.
+      if (status == nullptr) {
+        _controller.addError(HttpException('$respCode'));
+      } else {
+        final statusStr = status.toDartString();
+        _controller.addError(
+            HttpException(statusStr.isNotEmpty ? statusStr : '$respCode'));
+        malloc.free(status);
+      }
       callback();
-      final exception = HttpException(
-          cronet.Cronet_UrlResponseInfo_http_status_text_get(respInfoPtr)
-              .cast<Utf8>()
-              .toDartString());
-      _controller.addError(exception);
-      _controller.close();
+      return false;
     }
-    return respCode;
+    return true;
   }
 
   /// This listens to the messages sent by native cronet library.
@@ -100,14 +101,16 @@ class CallbackHandler {
       switch (reqMessage.method) {
         case 'OnRedirectReceived':
           {
+            final newUrlPtr = Pointer.fromAddress(args[0]).cast<Utf8>();
             log('New Location: '
-                '${Pointer.fromAddress(args[0]).cast<Utf8>().toDartString()}');
+                '${newUrlPtr.toDartString()}');
+            malloc.free(newUrlPtr);
             // If NOT a 3XX status code, throw Exception.
-            statusChecker(
-                Pointer.fromAddress(args[1]).cast<Cronet_UrlResponseInfo>(),
-                300,
-                399,
-                () => cleanUpRequest(reqPtr, cleanUpClient));
+            final status = statusChecker(args[1], Pointer.fromAddress(args[2]),
+                300, 399, () => cronet.Cronet_UrlRequest_Cancel(reqPtr));
+            if (!status) {
+              break;
+            }
             if (followRedirects && maxRedirects > 0) {
               final res = cronet.Cronet_UrlRequest_FollowRedirect(reqPtr);
               if (res != Cronet_RESULT.Cronet_RESULT_SUCCESS) {
@@ -125,12 +128,12 @@ class CallbackHandler {
         case 'OnResponseStarted':
           {
             // If NOT a 1XX or 2XX status code, throw Exception.
-            statusChecker(
-                Pointer.fromAddress(args[0]).cast<Cronet_UrlResponseInfo>(),
-                100,
-                299,
-                () => cleanUpRequest(reqPtr, cleanUpClient));
+            final status = statusChecker(args[0], Pointer.fromAddress(args[2]),
+                100, 299, () => cronet.Cronet_UrlRequest_Cancel(reqPtr));
             log('Response started');
+            if (!status) {
+              break;
+            }
             final res = cronet.Cronet_UrlRequest_Read(
                 reqPtr, Pointer.fromAddress(args[1]).cast<Cronet_Buffer>());
             if (res != Cronet_RESULT.Cronet_RESULT_SUCCESS) {
@@ -146,14 +149,16 @@ class CallbackHandler {
         case 'OnReadCompleted':
           {
             final request = Pointer<Cronet_UrlRequest>.fromAddress(args[0]);
-            final info = Pointer<Cronet_UrlResponseInfo>.fromAddress(args[1]);
             final buffer = Pointer<Cronet_Buffer>.fromAddress(args[2]);
             final bytesRead = args[3];
 
             log('Recieved: $bytesRead');
             // If NOT a 1XX or 2XX status code, throw Exception.
-            statusChecker(
-                info, 100, 299, () => cleanUpRequest(reqPtr, cleanUpClient));
+            final status = statusChecker(args[1], Pointer.fromAddress(args[4]),
+                100, 299, () => cronet.Cronet_UrlRequest_Cancel(reqPtr));
+            if (!status) {
+              break;
+            }
             final data = cronet.Cronet_Buffer_GetData(buffer)
                 .cast<Uint8>()
                 .asTypedList(bytesRead);
@@ -169,11 +174,9 @@ class CallbackHandler {
         // In case of network error, we will shut down everything.
         case 'OnFailed':
           {
-            final errorPtr = Pointer.fromAddress(args[0]).cast<Cronet_Error>();
-            final error = Pointer.fromAddress(
-                    cronet.Cronet_Error_message_get(errorPtr).address)
-                .cast<Utf8>()
-                .toDartString();
+            final errorStrPtr = Pointer.fromAddress(args[0]).cast<Utf8>();
+            final error = errorStrPtr.toDartString();
+            malloc.free(errorStrPtr);
             cleanUpRequest(reqPtr, cleanUpClient);
             _controller.addError(HttpException(error));
             _controller.close();

lib/src/http_client_request.dart

@@ -52,10 +52,12 @@ abstract class HttpClientRequest implements io.IOSink {
 
   /// Follow the redirects.
   bool get followRedirects;
+  set followRedirects(bool follow);
 
   /// Maximum numbers of redirects to follow.
   /// Have no effect if [followRedirects] is set to false.
   int get maxRedirects;
+  set maxRedirects(int redirects);
 
   /// The uri of the request.
   Uri get uri;
@@ -149,6 +151,7 @@ class HttpClientRequestImpl implements HttpClientRequest {
   /// Follow the redirects.
   @override
   bool get followRedirects => _callbackHandler.followRedirects;
+  @override
   set followRedirects(bool follow) {
     _callbackHandler.followRedirects = follow;
   }
@@ -157,6 +160,7 @@ class HttpClientRequestImpl implements HttpClientRequest {
   /// Have no effect if [followRedirects] is set to false.
   @override
   int get maxRedirects => _callbackHandler.maxRedirects;
+  @override
   set maxRedirects(int redirects) {
     _callbackHandler.maxRedirects = redirects;
   }

lib/src/third_party/cronet/ffigen.yaml

@@ -17,6 +17,9 @@ functions:
       - 'Cronet_Engine_Destroy'
       - 'Cronet_Buffer_Create'
       - 'Cronet_Buffer_InitWithAlloc'
+      - 'Cronet_UrlResponseInfo_http_status_code_get'
+      - 'Cronet_Error_message_get'
+      - 'Cronet_UrlResponseInfo_http_status_text_get'
       # For executor.
       - 'Cronet_Executor_CreateWith'
       - 'Cronet_Executor_SetClientContext'

lib/src/third_party/cronet/generated_bindings.dart

@@ -1691,7 +1691,7 @@ class Cronet {
   }
 
   late final _Cronet_Error_message_get_ptr =
-      _lookup<ffi.NativeFunction<_c_Cronet_Error_message_get>>(
+      _lookup<ffi.NativeFunction<Native_Cronet_Error_message_get>>(
           'Cronet_Error_message_get');
   late final _dart_Cronet_Error_message_get _Cronet_Error_message_get =
       _Cronet_Error_message_get_ptr.asFunction<
@@ -3006,7 +3006,8 @@ class Cronet {
   }
 
   late final _Cronet_UrlResponseInfo_http_status_code_get_ptr = _lookup<
-          ffi.NativeFunction<_c_Cronet_UrlResponseInfo_http_status_code_get>>(
+          ffi.NativeFunction<
+              Native_Cronet_UrlResponseInfo_http_status_code_get>>(
       'Cronet_UrlResponseInfo_http_status_code_get');
   late final _dart_Cronet_UrlResponseInfo_http_status_code_get
       _Cronet_UrlResponseInfo_http_status_code_get =
@@ -3022,7 +3023,8 @@ class Cronet {
   }
 
   late final _Cronet_UrlResponseInfo_http_status_text_get_ptr = _lookup<
-          ffi.NativeFunction<_c_Cronet_UrlResponseInfo_http_status_text_get>>(
+          ffi.NativeFunction<
+              Native_Cronet_UrlResponseInfo_http_status_text_get>>(
       'Cronet_UrlResponseInfo_http_status_text_get');
   late final _dart_Cronet_UrlResponseInfo_http_status_text_get
       _Cronet_UrlResponseInfo_http_status_text_get =
@@ -4678,6 +4680,18 @@ class _SymbolAddresses {
       get Cronet_Engine_Destroy => _library._Cronet_Engine_Destroy_ptr;
   ffi.Pointer<ffi.NativeFunction<Native_Cronet_Engine_Shutdown>>
       get Cronet_Engine_Shutdown => _library._Cronet_Engine_Shutdown_ptr;
+  ffi.Pointer<ffi.NativeFunction<Native_Cronet_Error_message_get>>
+      get Cronet_Error_message_get => _library._Cronet_Error_message_get_ptr;
+  ffi.Pointer<
+          ffi.NativeFunction<
+              Native_Cronet_UrlResponseInfo_http_status_code_get>>
+      get Cronet_UrlResponseInfo_http_status_code_get =>
+          _library._Cronet_UrlResponseInfo_http_status_code_get_ptr;
+  ffi.Pointer<
+          ffi.NativeFunction<
+              Native_Cronet_UrlResponseInfo_http_status_text_get>>
+      get Cronet_UrlResponseInfo_http_status_text_get =>
+          _library._Cronet_UrlResponseInfo_http_status_text_get_ptr;
 }
 
 class Cronet_Buffer extends ffi.Opaque {}
@@ -6048,7 +6062,7 @@ typedef _dart_Cronet_Error_error_code_get = int Function(
   ffi.Pointer<Cronet_Error> self,
 );
 
-typedef _c_Cronet_Error_message_get = ffi.Pointer<ffi.Int8> Function(
+typedef Native_Cronet_Error_message_get = ffi.Pointer<ffi.Int8> Function(
   ffi.Pointer<Cronet_Error> self,
 );
 
@@ -6757,16 +6771,16 @@ typedef _dart_Cronet_UrlResponseInfo_url_chain_clear = void Function(
   ffi.Pointer<Cronet_UrlResponseInfo> self,
 );
 
-typedef _c_Cronet_UrlResponseInfo_http_status_code_get = ffi.Int32 Function(
+typedef Native_Cronet_UrlResponseInfo_http_status_code_get = ffi.Int32 Function(
   ffi.Pointer<Cronet_UrlResponseInfo> self,
 );
 
 typedef _dart_Cronet_UrlResponseInfo_http_status_code_get = int Function(
   ffi.Pointer<Cronet_UrlResponseInfo> self,
 );
 
-typedef _c_Cronet_UrlResponseInfo_http_status_text_get = ffi.Pointer<ffi.Int8>
-    Function(
+typedef Native_Cronet_UrlResponseInfo_http_status_text_get
+    = ffi.Pointer<ffi.Int8> Function(
   ffi.Pointer<Cronet_UrlResponseInfo> self,
 );