Requests.php: always set body text from response

[granthony]

This fixes a bug in which response body text satisfying PHP "empty()" is not returned to the caller.

As substr() always returns a string, there is no need for the guard condition here. This has been removed accordingly, resolving the issue.

Pull Request Type

• I have checked there is no other PR open for the same change.

This is a:

• Bug fix
• New feature
• Documentation improvement
• Code quality improvement

Context

In dealing with a third-party API we found that empty responses were sometimes returned when we were expecting the non-empty response "0". Requests turned out to be at fault.

Expected behaviour: the response string "0" is returned.
Observed behaviour: no response is returned.

Detailed Description

In limited circumstances, the response body text is not correctly returned to the caller. PHP empty() treats some unintuitive things as empty, including the string "0".

Quality assurance

• This change does NOT contain a breaking change (fix or feature that would cause existing functionality to change).
• I have commented my code, particularly in hard-to-understand areas.
• I have added unit tests to accompany this PR.
• The (new/existing) tests cover this PR 100%.
• I have (manually) tested this code to the best of my abilities.
• My code follows the style guidelines of this project.

This is a breaking change but is likely to be of minimal impact.

A unit test could be added for requests returning the response string "0".

Documentation

For new features:

• I have added a code example showing how to use this feature to the examples directory.
• I have added documentation about this feature to the docs directory.
  If the documentation is in a new markdown file, I have added a link to this new file to the Docs folder README.md file.

[jrfnl]

@granthony Thanks for this PR and your willingness to contribute. However, this "bug fix" introduces a new bug.

As substr() always returns a string, there is no need for the guard condition here. This has been removed accordingly, resolving the issue.

This is true for PHP >= 8.0, but not for PHP < 8.0, in which case, substr() could return false. And false is not a valid value for the Response::$body.

https://www.php.net/manual/en/function.substr.php#refsect1-function.substr-changelog

[granthony]

My mistake, thank you. A strict comparison against false should do then.

[jrfnl]

My mistake, thank you. A strict comparison against false should do then.

As a rule of thumb, it's better to check for what you want, than what you don't want, so maybe the check should be a is_string() instead ?

Aside from that, I believe this change should be covered by a test. Could you please add one ?