fix(googleapis_auth): keep contentLength when forwarding requests (#765) * fix(googleapis_auth): keep contentLength when forwarding requests AuthenticatedClient and ApiKeyClient copy each request into a RequestImpl without its contentLength, persistentConnection, followRedirects and maxRedirects. IOClient then sends every request chunked, a GET with an empty chunked body that Google's front end intermittently answers as a separate, malformed request. Copy these properties in a shared RequestImpl.copyOf. Fixes #764 * chore(googleapis_auth): prepare 2.3.5 release --------- Co-authored-by: Kevin Moore <kevmoo@google.com>
diff --git a/googleapis_auth/CHANGELOG.md b/googleapis_auth/CHANGELOG.md index 7e519ae..8ff04c0 100644 --- a/googleapis_auth/CHANGELOG.md +++ b/googleapis_auth/CHANGELOG.md
@@ -1,5 +1,9 @@ -## 2.3.5-wip +## 2.3.5 +- `AuthenticatedClient` and `ApiKeyClient` now keep the `contentLength`, + `persistentConnection`, `followRedirects` and `maxRedirects` of the request + they forward. Without the content length, `IOClient` sent every request with + a chunked body, `GET` requests included. - Reject malformed ASN.1 DER multi-byte length encodings and non-sequence root objects in `ASN1Parser`, and fix `Uint8List` slice offset handling. - Reject non-positive RSA private key integer parameters and malformed PKCS#8
diff --git a/googleapis_auth/lib/src/auth_http_utils.dart b/googleapis_auth/lib/src/auth_http_utils.dart index 4530de5..369e6f8 100644 --- a/googleapis_auth/lib/src/auth_http_utils.dart +++ b/googleapis_auth/lib/src/auth_http_utils.dart
@@ -30,12 +30,7 @@ @override Future<StreamedResponse> send(BaseRequest request) async { // Make new request object and perform the authenticated request. - final modifiedRequest = RequestImpl( - request.method, - request.url, - request.finalize(), - ); - modifiedRequest.headers.addAll(request.headers); + final modifiedRequest = RequestImpl.copyOf(request); modifiedRequest.headers['Authorization'] = 'Bearer ${credentials.accessToken.data}'; if (quotaProject != null) { @@ -85,8 +80,7 @@ url = url.replace(query: '${url.query}&key=$_encodedApiKey'); } - final modifiedRequest = RequestImpl(request.method, url, request.finalize()) - ..headers.addAll(request.headers); + final modifiedRequest = RequestImpl.copyOf(request, url: url); addXGoogApiClientHeader(modifiedRequest.headers); return baseClient.send(modifiedRequest); }
diff --git a/googleapis_auth/lib/src/http_client_base.dart b/googleapis_auth/lib/src/http_client_base.dart index 16cfe74..44a6019 100644 --- a/googleapis_auth/lib/src/http_client_base.dart +++ b/googleapis_auth/lib/src/http_client_base.dart
@@ -107,6 +107,20 @@ RequestImpl(super.method, super.url, [Stream<List<int>>? stream]) : _stream = stream ?? const Stream.empty(); + /// Creates a copy of [request] that sends its body, optionally to [url]. + /// + /// Finalizes [request]. Besides its headers, the copy keeps the properties + /// that shape the request on the wire. Without [contentLength], `IOClient` + /// sends the body chunked: a `GET` request then carries an empty chunked + /// body. + factory RequestImpl.copyOf(BaseRequest request, {Uri? url}) => + RequestImpl(request.method, url ?? request.url, request.finalize()) + ..contentLength = request.contentLength + ..persistentConnection = request.persistentConnection + ..followRedirects = request.followRedirects + ..maxRedirects = request.maxRedirects + ..headers.addAll(request.headers); + @override ByteStream finalize() { super.finalize();
diff --git a/googleapis_auth/lib/src/version.dart b/googleapis_auth/lib/src/version.dart index fc2856a..107da28 100644 --- a/googleapis_auth/lib/src/version.dart +++ b/googleapis_auth/lib/src/version.dart
@@ -7,7 +7,7 @@ import 'version_fallback.dart' if (dart.library.io) 'version_io.dart'; /// Must be kept in sync with `pubspec.yaml` (verified by `test/version_test.dart`). -final _xGoogApiClientHeaderValue = 'gl-dart/$dartVersion auth/2.3.5-wip'; +final _xGoogApiClientHeaderValue = 'gl-dart/$dartVersion auth/2.3.5'; /// Adds the fallback `x-goog-api-client` header to [headers] if not already /// present, and returns [headers].
diff --git a/googleapis_auth/pubspec.yaml b/googleapis_auth/pubspec.yaml index 66607a0..b0ad952 100644 --- a/googleapis_auth/pubspec.yaml +++ b/googleapis_auth/pubspec.yaml
@@ -1,5 +1,5 @@ name: googleapis_auth -version: 2.3.5-wip +version: 2.3.5 description: Obtain Access credentials for Google services using OAuth 2.0 repository: https://github.com/google/googleapis.dart/tree/master/googleapis_auth
diff --git a/googleapis_auth/test/http_client_base_test.dart b/googleapis_auth/test/http_client_base_test.dart index 4d3c3a6..c603d1f 100644 --- a/googleapis_auth/test/http_client_base_test.dart +++ b/googleapis_auth/test/http_client_base_test.dart
@@ -110,6 +110,24 @@ client.close(); }); + test('keeps the content length and connection settings', () async { + final recording = RecordingClient(); + final client = ApiKeyClient(recording, key); + + final request = Request('POST', Uri.parse('http://localhost/abc')) + ..body = 'abc' + ..persistentConnection = false + ..followRedirects = false + ..maxRedirects = 2; + await client.send(request); + expect('${recording.request.url}', 'http://localhost/abc?$keyEncoded'); + expect(recording.request.contentLength, 3); + expect(recording.request.persistentConnection, false); + expect(recording.request.followRedirects, false); + expect(recording.request.maxRedirects, 2); + client.close(); + }); + test('with-existing-key throws ArgumentError', () { final mock = mockClient(expectAsync1(_defaultResponseHandler, count: 0));
diff --git a/googleapis_auth/test/oauth2_test.dart b/googleapis_auth/test/oauth2_test.dart index cbb3292..407ed1a 100644 --- a/googleapis_auth/test/oauth2_test.dart +++ b/googleapis_auth/test/oauth2_test.dart
@@ -422,6 +422,30 @@ expect(response.statusCode, 204); }); + test('keeps the content length of a request without a body', () async { + final recording = RecordingClient(); + final client = authenticatedClient(recording, credentials); + + await client.send(Request('GET', url)); + expect(recording.request.contentLength, 0); + }); + + test('keeps the content length and connection settings', () async { + final recording = RecordingClient(); + final client = authenticatedClient(recording, credentials); + + final request = Request('POST', url) + ..body = 'abc' + ..persistentConnection = false + ..followRedirects = false + ..maxRedirects = 2; + await client.send(request); + expect(recording.request.contentLength, 3); + expect(recording.request.persistentConnection, false); + expect(recording.request.followRedirects, false); + expect(recording.request.maxRedirects, 2); + }); + test('throws on access denied', () { final client = authenticatedClient( mockClient(
diff --git a/googleapis_auth/test/test_utils.dart b/googleapis_auth/test/test_utils.dart index 9f083b7..b379c9c 100644 --- a/googleapis_auth/test/test_utils.dart +++ b/googleapis_auth/test/test_utils.dart
@@ -87,6 +87,20 @@ bool expectClose = true, }) => ExpectCloseMockClient(requestHandler, expectClose ? 1 : 0); +/// A client which records the request it is given, unlike a [MockClient], +/// which hands its handler a new [Request] with the content length of the +/// body it read. +class RecordingClient extends BaseClient { + late BaseRequest request; + + @override + Future<StreamedResponse> send(BaseRequest request) async { + this.request = request; + await request.finalize().drain<void>(); + return StreamedResponse(const Stream.empty(), 204); + } +} + /// A client which will keep the VM alive until `close()` was called. class ExpectCloseMockClient extends MockClient { late void Function() _expectedToBeCalled;