Skip to content

Commit

Permalink
🐛 Only produce null response body when the response is a JSON type (#…
Browse files Browse the repository at this point in the history
…1874)

Resolves #1834.

### New Pull Request Checklist

- [x] I have read the
[Documentation](https://pub.dev/documentation/dio/latest/)
- [x] I have searched for a similar pull request in the
[project](https://github.com/cfug/dio/pulls) and found none
- [x] I have updated this branch with the latest `main` branch to avoid
conflicts (via merge from master or rebase)
- [x] I have added the required tests to prove the fix/feature I'm
adding
- [x] I have updated the documentation (if necessary)
- [x] I have run the tests without failures
- [x] I have updated the `CHANGELOG.md` in the corresponding package
  • Loading branch information
AlexV525 authored Jul 10, 2023
1 parent ec88e89 commit c301ffe
Show file tree
Hide file tree
Showing 9 changed files with 127 additions and 64 deletions.
1 change: 1 addition & 0 deletions dio/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ See the [Migration Guide][] for the complete breaking changes list.**
## Unreleased

- Remove `http` from `dev_dependencies`.
- Only produce null response body when `ResponseType.json`.

## 5.2.1+1

Expand Down
17 changes: 14 additions & 3 deletions dio/lib/src/dio_mixin.dart
Original file line number Diff line number Diff line change
Expand Up @@ -537,10 +537,9 @@ abstract class DioMixin implements Dio {
// Initiate Http requests
Future<Response<dynamic>> _dispatchRequest<T>(RequestOptions reqOpt) async {
final cancelToken = reqOpt.cancelToken;
ResponseBody responseBody;
try {
final stream = await _transformData(reqOpt);
responseBody = await httpClientAdapter.fetch(
final responseBody = await httpClientAdapter.fetch(
reqOpt,
stream,
cancelToken?.whenCancel,
Expand All @@ -559,7 +558,19 @@ abstract class DioMixin implements Dio {
);
final statusOk = reqOpt.validateStatus(responseBody.statusCode);
if (statusOk || reqOpt.receiveDataWhenStatusError == true) {
ret.data = await transformer.transformResponse(reqOpt, responseBody);
Object? data = await transformer.transformResponse(
reqOpt,
responseBody,
);
// Make the response as null before returned as JSON.
if (data is String &&
data.isEmpty &&
T != dynamic &&
T != String &&
reqOpt.responseType == ResponseType.json) {
data = null;
}
ret.data = data;
} else {
await responseBody.stream.listen(null).cancel();
}
Expand Down
5 changes: 3 additions & 2 deletions dio/lib/src/transformer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ abstract class Transformer {
/// [transformResponse] allows changes to the response data before
/// it is passed to [ResponseInterceptor].
///
/// **Note**: As an agreement, you must return the [response]
/// **Note**: As an agreement, you must return the [responseBody]
/// when the Options.responseType is [ResponseType.stream].
Future transformResponse(RequestOptions options, ResponseBody response);
// TODO(AlexV525): Add generic type for the method in v6.0.0.
Future transformResponse(RequestOptions options, ResponseBody responseBody);

/// Deep encode the [Map<String, dynamic>] to percent-encoding.
/// It is mostly used with the "application/x-www-form-urlencoded" content-type.
Expand Down
79 changes: 45 additions & 34 deletions dio/lib/src/transformers/sync_transformer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,86 +42,97 @@ class SyncTransformer extends Transformer {
}
}

/// As an agreement, we return the [response] when the
/// Options.responseType is [ResponseType.stream].
@override
Future<dynamic> transformResponse(
RequestOptions options,
ResponseBody response,
ResponseBody responseBody,
) async {
final responseType = options.responseType;
// Do not handled the body for streams.
if (options.responseType == ResponseType.stream) {
return response;
return responseBody;
}
int length = 0;
int received = 0;

final showDownloadProgress = options.onReceiveProgress != null;
final int totalLength;
if (showDownloadProgress) {
length = int.parse(
response.headers[Headers.contentLengthHeader]?.first ?? '-1',
totalLength = int.parse(
responseBody.headers[Headers.contentLengthHeader]?.first ?? '-1',
);
} else {
totalLength = 0;
}
final completer = Completer();
final stream = response.stream.transform<Uint8List>(

int received = 0;
final stream = responseBody.stream.transform<Uint8List>(
StreamTransformer.fromHandlers(
handleData: (data, sink) {
sink.add(data);
if (showDownloadProgress) {
received += data.length;
options.onReceiveProgress?.call(received, length);
options.onReceiveProgress?.call(received, totalLength);
}
},
),
);

final streamCompleter = Completer<void>();
int finalLength = 0;
// Keep references to the data chunks and concatenate them later.
final chunks = <Uint8List>[];
int finalSize = 0;
final StreamSubscription subscription = stream.listen(
final subscription = stream.listen(
(chunk) {
finalSize += chunk.length;
finalLength += chunk.length;
chunks.add(chunk);
},
onError: (Object error, StackTrace stackTrace) {
completer.completeError(error, stackTrace);
streamCompleter.completeError(error, stackTrace);
},
onDone: () {
streamCompleter.complete();
},
onDone: () => completer.complete(),
cancelOnError: true,
);
options.cancelToken?.whenCancel.then((_) {
return subscription.cancel();
});
await completer.future;
// Copy all chunks into a final Uint8List.
final responseBytes = Uint8List(finalSize);
await streamCompleter.future;

// Copy all chunks into the final bytes.
final responseBytes = Uint8List(finalLength);
int chunkOffset = 0;
for (final chunk in chunks) {
responseBytes.setAll(chunkOffset, chunk);
chunkOffset += chunk.length;
}

if (options.responseType == ResponseType.bytes) {
// Return the finalized bytes if the response type is bytes.
if (responseType == ResponseType.bytes) {
return responseBytes;
}

final String? responseBody;
final isJsonContent = Transformer.isJsonMimeType(
responseBody.headers[Headers.contentTypeHeader]?.first,
);
final String? response;
if (options.responseDecoder != null) {
responseBody = options.responseDecoder!(
response = options.responseDecoder!(
responseBytes,
options,
response..stream = Stream.empty(),
responseBody..stream = Stream.empty(),
);
} else if (responseBytes.isNotEmpty) {
responseBody = utf8.decode(responseBytes, allowMalformed: true);
} else if (!isJsonContent || responseBytes.isNotEmpty) {
response = utf8.decode(responseBytes, allowMalformed: true);
} else {
responseBody = null;
response = null;
}
if (responseBody != null &&
responseBody.isNotEmpty &&
options.responseType == ResponseType.json &&
Transformer.isJsonMimeType(
response.headers[Headers.contentTypeHeader]?.first,
)) {
return jsonDecodeCallback(responseBody);

if (response != null &&
response.isNotEmpty &&
responseType == ResponseType.json &&
isJsonContent) {
return jsonDecodeCallback(response);
}
return responseBody;
return response;
}
}
4 changes: 2 additions & 2 deletions dio/test/mock/http_mock.mocks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -672,8 +672,8 @@ class MockTransformer extends _i1.Mock implements _i5.Transformer {
returnValue: Future<String>.value('')) as _i4.Future<String>);
@override
_i4.Future<dynamic> transformResponse(
_i6.RequestOptions? options, _i7.ResponseBody? response) =>
_i6.RequestOptions? options, _i7.ResponseBody? responseBody) =>
(super.noSuchMethod(
Invocation.method(#transformResponse, [options, response]),
Invocation.method(#transformResponse, [options, responseBody]),
returnValue: Future<dynamic>.value()) as _i4.Future<dynamic>);
}
8 changes: 8 additions & 0 deletions dio/test/options_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,14 @@ void main() {
r10.requestOptions.contentType,
startsWith(Headers.multipartFormDataContentType),
);

// Regression: https://github.com/cfug/dio/issues/1834
final r11 = await dio.get('');
expect(r11.data, '');
final r12 = await dio.get<Map>('');
expect(r12.data, null);
final r13 = await dio.get<Map<String, Object>>('');
expect(r13.data, null);
});

test('default content-type 2', () async {
Expand Down
52 changes: 52 additions & 0 deletions dio/test/transformer_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import 'package:dio/dio.dart';
import 'package:test/test.dart';

void main() {
group(BackgroundTransformer(), () {
test('transformResponse transforms the request', () async {
final transformer = BackgroundTransformer();
final response = await transformer.transformResponse(
RequestOptions(responseType: ResponseType.json),
ResponseBody.fromString(
'{"foo": "bar"}',
200,
headers: {
Headers.contentTypeHeader: ['application/json'],
},
),
);
expect(response, {'foo': 'bar'});
});
});

// Regression: https://github.com/cfug/dio/issues/1834
test('null response body only when the response is JSON', () async {
final transformer = BackgroundTransformer();
final r1 = await transformer.transformResponse(
RequestOptions(responseType: ResponseType.json),
ResponseBody.fromBytes([], 200),
);
expect(r1, '');
final r2 = await transformer.transformResponse(
RequestOptions(responseType: ResponseType.bytes),
ResponseBody.fromBytes([], 200),
);
expect(r2, []);
final r3 = await transformer.transformResponse(
RequestOptions(responseType: ResponseType.plain),
ResponseBody.fromBytes([], 200),
);
expect(r3, '');
final r4 = await transformer.transformResponse(
RequestOptions(responseType: ResponseType.json),
ResponseBody.fromBytes(
[],
200,
headers: {
Headers.contentTypeHeader: [Headers.jsonContentType],
},
),
);
expect(r4, null);
});
}
21 changes: 0 additions & 21 deletions dio/test/transformers/background_transformer_test.dart

This file was deleted.

4 changes: 2 additions & 2 deletions example/lib/transformer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ class MyTransformer extends BackgroundTransformer {
@override
Future transformResponse(
RequestOptions options,
ResponseBody response,
ResponseBody responseBody,
) async {
options.extra['self'] = 'XX';
return super.transformResponse(options, response);
return super.transformResponse(options, responseBody);
}
}

Expand Down

0 comments on commit c301ffe

Please sign in to comment.