diff --git a/mobile/lib/features/channels/channels_page/badges.dart b/mobile/lib/features/channels/channels_page/badges.dart index 680ee9a85a..71e93aac10 100644 --- a/mobile/lib/features/channels/channels_page/badges.dart +++ b/mobile/lib/features/channels/channels_page/badges.dart @@ -47,7 +47,7 @@ class _ErrorView extends HookConsumerWidget { return 'Server error (${error.statusCode}). Try again later.'; } if (error is SocketException || - urlSessionFailure(error) == TransportFailure.unreachable) { + transportFailure(error) == TransportFailure.unreachable) { return 'Could not reach the relay server.'; } return 'Something went wrong. Check your connection.'; diff --git a/mobile/lib/features/invites/invite_join_provider.dart b/mobile/lib/features/invites/invite_join_provider.dart index 49a7b4e6a8..34fc6a2036 100644 --- a/mobile/lib/features/invites/invite_join_provider.dart +++ b/mobile/lib/features/invites/invite_join_provider.dart @@ -516,14 +516,14 @@ String _friendlyInviteError(Object error) { if (message.contains('join_policy_required')) { return 'Review this community’s join requirements in your browser, then return to Buzz.'; } - final transportFailure = urlSessionFailure(error); - if (transportFailure == TransportFailure.insecure) { + final failure = transportFailure(error); + if (failure == TransportFailure.insecure) { return 'Secure connection failed. Check your network settings and try again.'; } - if (transportFailure == TransportFailure.timedOut) { + if (failure == TransportFailure.timedOut) { return 'Connection timed out. Check your internet connection and try again.'; } - if (transportFailure == TransportFailure.unreachable || + if (failure == TransportFailure.unreachable || message.contains('SocketException') || message.contains('Connection refused') || message.contains('Network is unreachable') || @@ -535,7 +535,7 @@ String _friendlyInviteError(Object error) { String _friendlyStarterSetupError(Object error) { final message = error.toString(); - if (urlSessionFailure(error) == TransportFailure.unreachable || + if (transportFailure(error) == TransportFailure.unreachable || message.contains('SocketException') || message.contains('Connection refused') || message.contains('Network is unreachable') || diff --git a/mobile/lib/features/pairing/pairing_provider_helpers.dart b/mobile/lib/features/pairing/pairing_provider_helpers.dart index e2def264e8..6e363c59ad 100644 --- a/mobile/lib/features/pairing/pairing_provider_helpers.dart +++ b/mobile/lib/features/pairing/pairing_provider_helpers.dart @@ -2,8 +2,8 @@ part of 'pairing_provider.dart'; String _friendlyErrorMessage(Object error) { final message = error.toString(); - final transportFailure = urlSessionFailure(error); - if (transportFailure == TransportFailure.unreachable || + final failure = transportFailure(error); + if (failure == TransportFailure.unreachable || message.contains('SocketException') || message.contains('Connection refused') || message.contains('Network is unreachable') || @@ -20,13 +20,13 @@ String _friendlyErrorMessage(Object error) { message.contains('Null check operator used on a null value')) { return 'Pairing stopped because of an internal error. Please try again.'; } - if (transportFailure == TransportFailure.insecure || + if (failure == TransportFailure.insecure || message.contains('HandshakeException') || message.contains('CERTIFICATE_VERIFY_FAILED')) { return 'Secure connection failed. Check your network settings ' 'and try again.'; } - if (transportFailure == TransportFailure.timedOut || + if (failure == TransportFailure.timedOut || message.contains('TimeoutException') || message.contains('timed out')) { return 'Connection timed out. Check your internet connection and ' diff --git a/mobile/lib/shared/http/app_http_client.dart b/mobile/lib/shared/http/app_http_client.dart index faf8253769..6a1ba4da80 100644 --- a/mobile/lib/shared/http/app_http_client.dart +++ b/mobile/lib/shared/http/app_http_client.dart @@ -1,10 +1,12 @@ import 'dart:async'; import 'dart:io' show Platform; +import 'package:cronet_http/cronet_http.dart'; import 'package:cupertino_http/cupertino_http.dart'; import 'package:http/http.dart' as http; +import 'package:jni/jni.dart' show JThrowable; import 'package:objective_c/objective_c.dart'; -import 'package:flutter/foundation.dart' show visibleForTesting; +import 'package:flutter/foundation.dart' show debugPrint, visibleForTesting; /// Which app-wide connection pool a request belongs to. enum HttpTraffic { @@ -15,20 +17,34 @@ enum HttpTraffic { media, } -final _sessions = {}; +/// The app-wide session for each traffic class, or null where the platform +/// has none. +final _sessions = {}; /// An HTTP client for [traffic]. Callers own it and close it as usual. /// -/// On iOS, every client for the same [traffic] shares one `NSURLSession`, so -/// requests to a host multiplex over one HTTP/2 connection instead of opening -/// an HTTP/1.1 connection each, and gzip is inflated natively, off the Dart -/// isolate. Media has its own session, and so its own connection, so image -/// bytes cannot starve queries queued behind them on a shared connection. +/// On iOS and Android, every client for the same [traffic] shares one native +/// session (`NSURLSession` or a Cronet engine), so requests to a host +/// multiplex over one HTTP/2 connection instead of opening an HTTP/1.1 +/// connection each, and gzip is inflated natively, off the Dart isolate. +/// Media has its own session, and so its own connection, so image bytes +/// cannot starve queries queued behind them on a shared connection. /// /// Elsewhere this is `http.Client()`, which also honors `http.runWithClient`. http.Client newAppHttpClient([HttpTraffic traffic = HttpTraffic.api]) { - if (!Platform.isIOS) return http.Client(); - return SharedSessionClient(_sessions[traffic] ??= _openUrlSession()); + final session = _sessions.putIfAbsent(traffic, _openNativeSession); + if (session == null) return http.Client(); + return SharedSessionClient(session, streamsUploads: !Platform.isAndroid); +} + +/// Whether [newAppHttpClient] uses a native session on this device. +bool get hasNativeHttpSession => + _sessions.putIfAbsent(HttpTraffic.api, _openNativeSession) != null; + +http.Client? _openNativeSession() { + if (Platform.isIOS) return _openUrlSession(); + if (Platform.isAndroid) return _openCronetSession(); + return null; } /// Matches the `dart:io` transport apart from the protocol: no HTTP cache and @@ -42,22 +58,55 @@ http.Client _openUrlSession() { return CupertinoClient.fromSessionConfiguration(config); } +/// Matches the `dart:io` transport apart from the protocol: no HTTP cache, +/// and Cronet keeps no cookies. QUIC stays off, so this changes only the +/// HTTP version. +/// +/// Null when no Cronet provider is available: Cronet comes from Google Play +/// services, which some Android devices don't have. Those keep `dart:io`. +http.Client? _openCronetSession() { + final CronetEngine engine; + try { + engine = CronetEngine.build( + cacheMode: CacheMode.disabled, + enableHttp2: true, + enableQuic: false, + ); + } on JThrowable catch (error) { + debugPrint('Cronet is unavailable, using dart:io: ${error.message}'); + return null; + } + return CronetClient.fromCronetEngine(engine); +} + /// A caller's handle on an app-wide session. Like closing a `dart:io` client, /// [close] cancels the handle's requests still in flight and refuses later /// ones, but leaves the session open for other handles. Closing a -/// `CupertinoClient` while any of its requests run would also throw. +/// `CupertinoClient` while any of its requests run would also throw, and +/// closing a `CronetClient` would stop the shared engine. /// /// App Transport Security blocks cleartext `URLSession` loads from public /// hosts, while saved community icons and remote media may still use /// `http://`. Those keep the handle's own `dart:io` client, as before. +/// +/// Unless [streamsUploads], a streamed request with a body (media and voice +/// note uploads) also keeps `dart:io`: `CronetClient` reads the whole body +/// into memory before sending it, so progress would reach 100% before any +/// byte left and a large attachment would be copied in full. @visibleForTesting class SharedSessionClient extends http.BaseClient { - SharedSessionClient(this._session, {http.Client Function()? cleartext}) - : _newCleartext = cleartext ?? http.Client.new; + SharedSessionClient( + this._session, { + http.Client Function()? cleartext, + this.streamsUploads = true, + }) : _newCleartext = cleartext ?? http.Client.new; final http.Client _session; final http.Client Function() _newCleartext; + /// Whether the session streams request bodies instead of buffering them. + final bool streamsUploads; + /// One abort trigger per request still running, so [close] can cancel it. /// A request leaves the set once its response body is done, failed or /// cancelled, or once the caller's own abort trigger fires. A body nobody @@ -75,7 +124,8 @@ class SharedSessionClient extends http.BaseClient { request.url, ); } - if (request.url.scheme != 'https') { + if (request.url.scheme != 'https' || + (!streamsUploads && _isStreamedUpload(request))) { return (_cleartext ??= _newCleartext()).send(request); } final abort = Completer(); @@ -130,6 +180,11 @@ class SharedSessionClient extends http.BaseClient { ); } + static bool _isStreamedUpload(http.BaseRequest request) => + request is! http.Request && + request.method != 'GET' && + request.method != 'HEAD'; + /// A copy of [request] that the session cancels when [trigger] completes. http.BaseRequest _withAbortTrigger( http.BaseRequest request, @@ -178,17 +233,20 @@ class SharedSessionClient extends http.BaseClient { } } -/// How an iOS transport request failed, for user-facing messages. `dart:io` -/// failures are recognized by their messages instead. +/// How a native transport request failed, for user-facing messages. +/// `dart:io` failures are recognized by their messages instead. enum TransportFailure { unreachable, insecure, timedOut } -/// The [TransportFailure] for a `URLSession` error, or null for any other -/// error. -TransportFailure? urlSessionFailure(Object error) { - if (error is! NSErrorClientException) return null; - if (error.error.domain.toDartString() != 'NSURLErrorDomain') return null; - return urlErrorFailure(error.error.code); -} +/// The [TransportFailure] for a `URLSession` or Cronet error, or null for +/// any other error. +TransportFailure? transportFailure(Object error) => switch (error) { + NSErrorClientException(:final error) + when error.domain.toDartString() == 'NSURLErrorDomain' => + urlErrorFailure(error.code), + NetworkException(:final errorCode, :final cronetInternalErrorCode) => + cronetErrorFailure(errorCode, cronetInternalErrorCode), + _ => null, +}; /// Classifies an `NSURLErrorDomain` code. @visibleForTesting @@ -207,3 +265,21 @@ TransportFailure? urlErrorFailure(int code) => switch (code) { >= -1206 && <= -1200 => TransportFailure.insecure, _ => null, }; + +/// Classifies a Cronet `NetworkException` by its `ERROR_*` code and, for +/// `ERROR_OTHER`, Chromium's internal net error code. +@visibleForTesting +TransportFailure? cronetErrorFailure(int errorCode, int internalCode) => + switch (errorCode) { + 4 || 6 => TransportFailure.timedOut, + // Host not resolved, offline, network changed, connection closed, + // refused or reset, address unreachable. + 1 || 2 || 3 || 5 || 7 || 8 || 9 => TransportFailure.unreachable, + // SSL protocol error, cipher mismatch, and the certificate errors. + 11 + when internalCode == -107 || + internalCode == -113 || + (internalCode <= -200 && internalCode >= -299) => + TransportFailure.insecure, + _ => null, + }; diff --git a/mobile/lib/shared/relay/relay_http_query_client.dart b/mobile/lib/shared/relay/relay_http_query_client.dart index 0feea2261b..d5f12dce2a 100644 --- a/mobile/lib/shared/relay/relay_http_query_client.dart +++ b/mobile/lib/shared/relay/relay_http_query_client.dart @@ -5,15 +5,16 @@ import 'package:http/http.dart' as http; import 'package:http/io_client.dart'; import 'package:buzz/shared/http/app_http_client.dart'; -/// The default transport. On iOS, the app-wide `NSURLSession` inflates gzip -/// natively, off the Dart isolate. `dart:io` would inflate on the calling -/// isolate, so leave its responses compressed for the response decoder. +/// The default transport. On iOS and Android, the app-wide native session +/// inflates gzip natively, off the Dart isolate. `dart:io` would inflate on +/// the calling isolate, so leave its responses compressed for the response +/// decoder. /// /// A non-`dart:io` client supplied through `http.runWithClient`, such as a /// `MockClient`, is used as is. An `IOClient` is replaced, not closed, since /// it may be shared. http.Client _defaultTransport() { - if (Platform.isIOS) return newAppHttpClient(); + if (hasNativeHttpSession) return newAppHttpClient(); final client = http.Client(); if (client is! IOClient) return client; return IOClient(HttpClient()..autoUncompress = false); diff --git a/mobile/pubspec.lock b/mobile/pubspec.lock index 298254bbbd..68144c7529 100644 --- a/mobile/pubspec.lock +++ b/mobile/pubspec.lock @@ -288,6 +288,14 @@ packages: url: "https://pub.dev" source: hosted version: "1.15.0" + cronet_http: + dependency: "direct main" + description: + name: cronet_http + sha256: "7f8976ce1531399e42b4c5ad0937bd57bd93fbcff564f7d51c695be19850e977" + url: "https://pub.dev" + source: hosted + version: "1.10.1" cross_file: dependency: transitive description: @@ -808,13 +816,13 @@ packages: source: hosted version: "1.0.5" jni: - dependency: transitive + dependency: "direct main" description: name: jni - sha256: c2230682d5bc2362c1c9e8d3c7f406d9cbba23ab3f2e203a025dd47e0fb2e68f + sha256: "2e91e6087291affd10b31b6c872f3e3ac7dc5d93b51b5ba485d930c8f08bde6c" url: "https://pub.dev" source: hosted - version: "1.0.0" + version: "1.1.2" jni_flutter: dependency: transitive description: @@ -823,6 +831,14 @@ packages: url: "https://pub.dev" source: hosted version: "1.0.1" + jni_util: + dependency: transitive + description: + name: jni_util + sha256: "1ba86da04a5f2bf18fde2edb235587e70c5b0fc5bd4ba955f46b00942c3fc35f" + url: "https://pub.dev" + source: hosted + version: "1.0.0" json_annotation: dependency: transitive description: diff --git a/mobile/pubspec.yaml b/mobile/pubspec.yaml index 99811b86a5..6b66650559 100644 --- a/mobile/pubspec.yaml +++ b/mobile/pubspec.yaml @@ -18,9 +18,11 @@ dependencies: hooks_riverpod: ^3.0.3 flutter_hooks: ^0.21.3 http: ^1.4.0 - # iOS HTTP transport (NSURLSession): HTTP/2 and native gzip. See - # lib/shared/http/app_http_client.dart. + # Native HTTP transports, NSURLSession on iOS and Cronet on Android: HTTP/2 + # and native gzip. See lib/shared/http/app_http_client.dart. + cronet_http: ^1.10.1 cupertino_http: ^3.1.0 + jni: ^1.1.0 objective_c: ^9.5.0 crypto: ^3.0.7 flutter_secure_storage: ^10.0.0 diff --git a/mobile/test/shared/http/app_http_client_test.dart b/mobile/test/shared/http/app_http_client_test.dart index a857852c44..ce5456fb2e 100644 --- a/mobile/test/shared/http/app_http_client_test.dart +++ b/mobile/test/shared/http/app_http_client_test.dart @@ -8,7 +8,7 @@ import 'package:http/http.dart' as http; import 'package:http/testing.dart'; void main() { - test('off iOS, clients come from http.Client()', () { + test('without a native session, clients come from http.Client()', () { final mock = MockClient((_) async => http.Response('', 200)); for (final traffic in HttpTraffic.values) { expect( @@ -213,6 +213,47 @@ void main() { expect(cleartextClosed, isTrue); }); + test( + 'streamed uploads keep dart:io when the session buffers bodies', + () async { + final sessionRequests = []; + final fallbackRequests = []; + final client = SharedSessionClient( + MockClient.streaming((request, body) async { + await body.drain(); + sessionRequests.add('${request.method} ${request.url.path}'); + return http.StreamedResponse(const Stream.empty(), 200); + }), + cleartext: () => MockClient.streaming((request, body) async { + await body.drain(); + fallbackRequests.add('${request.method} ${request.url.path}'); + return http.StreamedResponse(const Stream.empty(), 200); + }), + streamsUploads: false, + ); + + final upload = http.StreamedRequest( + 'PUT', + Uri.parse('https://media.example/upload'), + ); + upload.sink + ..add([1, 2, 3]) + ..close(); + await client.send(upload); + final download = http.StreamedRequest( + 'GET', + Uri.parse('https://media.example/video'), + ); + unawaited(download.sink.close()); + await client.send(download); + await client.post(Uri.parse('https://relay.example/query'), body: '{}'); + client.close(); + + expect(fallbackRequests, ['PUT /upload']); + expect(sessionRequests, ['GET /video', 'POST /query']); + }, + ); + test('URLSession error codes map to user-facing failure kinds', () { expect(urlErrorFailure(-1009), TransportFailure.unreachable); expect(urlErrorFailure(-1004), TransportFailure.unreachable); @@ -220,7 +261,19 @@ void main() { expect(urlErrorFailure(-1200), TransportFailure.insecure); expect(urlErrorFailure(-1202), TransportFailure.insecure); expect(urlErrorFailure(-999), isNull); - expect(urlSessionFailure(http.ClientException('offline')), isNull); + expect(transportFailure(http.ClientException('offline')), isNull); + }); + + test('Cronet error codes map to user-facing failure kinds', () { + expect(cronetErrorFailure(4, -7), TransportFailure.timedOut); + expect(cronetErrorFailure(6, -118), TransportFailure.timedOut); + expect(cronetErrorFailure(1, -105), TransportFailure.unreachable); + expect(cronetErrorFailure(2, -106), TransportFailure.unreachable); + expect(cronetErrorFailure(7, -102), TransportFailure.unreachable); + expect(cronetErrorFailure(11, -202), TransportFailure.insecure); + expect(cronetErrorFailure(11, -107), TransportFailure.insecure); + expect(cronetErrorFailure(11, -324), isNull); + expect(cronetErrorFailure(10, -356), isNull); }); }