Skip to content

Commit

Permalink
fix: improve fallback to null on empty responses (#2285)
Browse files Browse the repository at this point in the history
fixes #2279 

This improves handling of the following case:

- ResponseType is json
- Server Content-Type is json
- Server Content-Length is nonzero
- Response Body is empty

This can happen in some cases according to HTTP spec (e.g. HEAD
requests), but some servers return responses like this (see #2279) also
in other cases, even if it violates the HTTP spec.

With #2250, in some of these cases dio would throw an exception, which
caused a regression for some users; see #2279
I do believe that dio should handle these cases for gracefully,

This PR brings back the previous behavior of returning null.  

### 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

### Additional context and info (if any)

<!-- Provide more context and info about the PR. -->

---------

Signed-off-by: Martin Kamleithner <[email protected]>
Co-authored-by: Jonas Uekötter <[email protected]>
  • Loading branch information
knaeckeKami and ueman authored Sep 3, 2024
1 parent 679a144 commit d7cad71
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 16 deletions.
4 changes: 3 additions & 1 deletion dio/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ See the [Migration Guide][] for the complete breaking changes list.**

## Unreleased

*None.*
- Graceful handling of responses with nonzero `Content-Length`, `Content-Type` json, but empty body
- Empty responses are now transformed to `null`


## 5.6.0

Expand Down
24 changes: 9 additions & 15 deletions dio/lib/src/transformers/fused_transformer.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'dart:async';
import 'dart:convert';
import 'dart:typed_data';

Expand All @@ -7,6 +8,7 @@ import '../headers.dart';
import '../options.dart';
import '../transformer.dart';
import 'util/consolidate_bytes.dart';
import 'util/transform_empty_to_null.dart';

/// A [Transformer] that has a fast path for decoding UTF8-encoded JSON.
/// If the response is utf8-encoded JSON and no custom decoder is specified in the [RequestOptions], this transformer
Expand Down Expand Up @@ -118,11 +120,6 @@ class FusedTransformer extends Transformer {
// of the response or the length of the eagerly decoded response bytes
final int contentLength;

// Successful HEAD requests don't have a response body, even if the content-length header
// present.
final mightNotHaveResponseBodyDespiteContentLength =
options.method == 'HEAD';

// The eagerly decoded response bytes
// which is set if the content length is not specified and
// null otherwise (we'll feed the stream directly to the decoder in that case)
Expand All @@ -131,8 +128,7 @@ class FusedTransformer extends Transformer {
// If the content length is not specified, we need to consolidate the stream
// and count the bytes to determine if we should use an isolate
// otherwise we use the content length header
if (!hasContentLengthHeader ||
mightNotHaveResponseBodyDespiteContentLength) {
if (!hasContentLengthHeader) {
responseBytes = await consolidateBytes(responseBody.stream);
contentLength = responseBytes.length;
} else {
Expand All @@ -153,13 +149,7 @@ class FusedTransformer extends Transformer {
responseBytes ?? await consolidateBytes(responseBody.stream),
);
} else {
if (!hasContentLengthHeader || contentLength == 0) {
// This path is for backwards compatibility.
// If content-type indicates a json response,
// but the body is empty, null is returned.
// _utf8JsonDecoder.bind(responseBody.stream) would throw if the body is empty.
// So we need to check if the body is empty and return null in that case
responseBytes ??= await consolidateBytes(responseBody.stream);
if (responseBytes != null) {
if (responseBytes.isEmpty) {
return null;
}
Expand All @@ -168,7 +158,11 @@ class FusedTransformer extends Transformer {
assert(responseBytes == null);
// The content length is specified and we can feed the stream directly to the decoder,
// without eagerly decoding the response bytes first.
final decodedStream = _utf8JsonDecoder.bind(responseBody.stream);
// If the response is empty, return null;
// This is done by the DefaultNullIfEmptyStreamTransformer
final streamWithNullFallback = responseBody.stream
.transform(const DefaultNullIfEmptyStreamTransformer());
final decodedStream = _utf8JsonDecoder.bind(streamWithNullFallback);
final decoded = await decodedStream.toList();
if (decoded.isEmpty) {
return null;
Expand Down
48 changes: 48 additions & 0 deletions dio/lib/src/transformers/util/transform_empty_to_null.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import 'dart:async';
import 'dart:typed_data';

/// A [StreamTransformer] that replaces an empty stream of Uint8List with a default value
/// - the utf8-encoded string "null".
/// Feeding an empty stream to a JSON decoder will throw an exception, so this transformer
/// is used to prevent that; the JSON decoder will instead return null.
class DefaultNullIfEmptyStreamTransformer
extends StreamTransformerBase<Uint8List, Uint8List> {
const DefaultNullIfEmptyStreamTransformer();

@override
Stream<Uint8List> bind(Stream<Uint8List> stream) {
return Stream.eventTransformed(
stream,
(sink) => _DefaultIfEmptyStreamSink(sink),
);
}
}

class _DefaultIfEmptyStreamSink implements EventSink<Uint8List> {
_DefaultIfEmptyStreamSink(this._outputSink);

/// Hard-coded constant for replacement value, "null"
static final Uint8List _nullUtf8Value =
Uint8List.fromList(const [110, 117, 108, 108]);

final EventSink<Uint8List> _outputSink;
bool _hasData = false;

@override
void add(Uint8List data) {
_hasData = _hasData || data.isNotEmpty;
_outputSink.add(data);
}

@override
void addError(e, [st]) => _outputSink.addError(e, st);

@override
void close() {
if (!_hasData) {
_outputSink.add(_nullUtf8Value);
}

_outputSink.close();
}
}
43 changes: 43 additions & 0 deletions dio/test/transformer_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,30 @@ void main() {
expect(response, {'foo': 'bar'});
});

test('transformResponse transforms json array', () async {
final transformer = FusedTransformer();
const jsonString = '[{"foo": "bar"}]';
final response = await transformer.transformResponse(
RequestOptions(responseType: ResponseType.json),
ResponseBody.fromString(
jsonString,
200,
headers: {
Headers.contentTypeHeader: ['application/json'],
Headers.contentLengthHeader: [
utf8.encode(jsonString).length.toString(),
],
},
),
);
expect(
response,
[
{'foo': 'bar'},
],
);
});

test('transforms json in background isolate', () async {
final transformer = FusedTransformer(contentLengthIsolateThreshold: 0);
final jsonString = '{"foo": "bar"}';
Expand Down Expand Up @@ -299,6 +323,25 @@ void main() {
expect(response, null);
},
);

test(
'can handle status 304 responses with content-length but empty body',
() async {
final transformer = FusedTransformer();
final response = await transformer.transformResponse(
RequestOptions(responseType: ResponseType.json),
ResponseBody(
Stream.value(Uint8List(0)),
304,
headers: {
Headers.contentTypeHeader: ['application/json'],
Headers.contentLengthHeader: ['123'],
},
),
);
expect(response, null);
},
);
});

group('consolidate bytes', () {
Expand Down

0 comments on commit d7cad71

Please sign in to comment.