fix: improve fallback to null on empty responses

[knaeckeKami]

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

• I have read the Documentation
• I have searched for a similar pull request in the project and found none
• I have updated this branch with the latest main branch to avoid conflicts (via merge from master or rebase)
• I have added the required tests to prove the fix/feature I'm adding
• I have updated the documentation (if necessary)
• I have run the tests without failures
• I have updated the CHANGELOG.md in the corresponding package

Additional context and info (if any)

[Schwusch]

This is also a problem downstream in package:retrofit, where/when content type is always JSON but response body is empty.
Is there someone that can approve this? @ueman ?

[ueman]

@kuhnroyal @AlexV525 Do we want to release this as 5.7.0 or as a 6.0.0? This could be considered a breaking change, but it doesn't really feel like a big change. What do you think?

[knaeckeKami]

I'd argue it's a bug fix, as the change occurred with #2250, and this restores the old behavior.

But of course, a bug fix can also be viewed as a breaking change if people depend on it ;)

[ueman]

I'd argue it's a bug fix, as the change occurred with #2250, and this restores the old behavior.

But of course, a bug fix can also be viewed as a breaking change if people depend on it ;)

Yeah, I'm tempted to release it as 5.7.0, but I still would like to hear their opinion

[AlexV525]

I didn't take a detailed look into what happened. In general we need to provide tests that can pass in previous specific versions and with the request changes.

[AlexV525]

Two entries? Also please write words in correct capitalization: response, content-type, etc.

[ueman]

The first line are the affected cases, the second line describes the result

[knaeckeKami]

fixed capitalization of "Responses".

The capitalization of the headers are like in in the official spec like https://datatracker.ietf.org/doc/html/rfc2616#section-14.13
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Length

[AlexV525]

What does the value help with?

[knaeckeKami]

It's for adding the byte sequence for "null" to the response stream, in the case that the response stream is unexpectedly empty.
So we return null instead of throwing an exception.

[AlexV525]

Wouldn't that cause a side effect that user won't understand why their response becomes "null"?

[knaeckeKami]

For clarification: The response won't become the String "null", it would become the value null after the json decoder decodes that byte sequence.

---

It has always been the behavior of Dio to return null when the ResponseType is .json and the response is empty.
Only in the recent release with the FusedTransformer there is now a corner case that if

• the ResponseType is .json and
• the Content-Type header is application/json and
• the Content-Length header has a value of > 0 and
• the response is still empty

That an exception is thrown (instead of the old behavior of returning null).
This is because we currently assume that if the Content-Length is >0, that the response body will not be empty. So the response stream is fed into the json decoder, but the decoder throws on empty streams.

This transformation is not applied for other ResponseType than .json.

I would also be okay with removing the behavior of returning null on empty responses when the ResponseType is .json but the body is empty completely, and always throw an exception in that case.
But that would be a breaking change for sure.

The behavior of the current release, where it mostly returns null on empty responses but throws an exception in certain corner cases (mostly responses that are invalid according to the HTTP spec, but still occur in the real world), is not ideal IMO.

[knaeckeKami]

I added a test here:
https://github.com/cfug/dio/pull/2285/files#diff-60c4759d76b4a7475d5c699664cbe409ca3cf5c6f09a403a5f3d2a4c4dd08ed6R330

This test would pass before #2250, and after this PR is merged.
With the current release, it would fail.

A user opened #2279 because of that.

The server sends an invalid response (according to the HTTP spec), but I still think that it's better to handle such cases gracefully in an HTTP client (especially if the old behavior was graceful (return null instead of throwing an exception).

[github-actions]

Code Coverage Report: Only Changed Files listed

Package	Base Coverage	New Coverage	Difference
dio/lib/src/transformers/fused_transformer.dart	🟢 91.3%	🟢 93.33%	🟢 2.03%
dio/lib/src/transformers/util/transform_empty_to_null.dart	🔴 0%	🟢 87.5%	🟢 87.5%
Overall Coverage	🟢 84.39%	🟢 84.46%	🟢 0.07%

Minimum allowed coverage is 0%, this run produced 84.46%