Setting the stage
By default, the PHP native [lr]trim() functions, trim ASCII whitespace and the NUL byte character.
PHP 8.6 changes the default value of the $characters parameter to also include the form feed - "\f" - character, which was previously not trimmed.
⚠️ Keep in mind that [lr]trim() only operates on the leading and/or trailing characters for a text string. It does not affect the characters in the "middle" !
Refs:
What has already been done
Requests uses [lr]trim() in various places throughout the codebase.
To make this code PHP cross-version compatible, PR #1083 made the following changes:
- Introduced two class constants to represent the different default values for the
$characters parameter.
- Made the
$characters being trimmed explicit in each of the [lr]trim() function calls (if it wasn't already).
To determine which characters should be trimmed, the following rule of thumb has been used for the initial changes made in PR #1083:
"Use the PHP 8.6 default (Trim::WHITESPACE_CHARS), except when the trimming may be subject to an RFC or other documented rules, in which case use the PHP < 8.6 default (Trim::WHITESPACE_CHARS_NO_FF)"
That way, we preserved existing behaviour in "important" places, while benefitting from the new default value everywhere else.
Task
For all calls to [lr]trim() which currently use the Trim::WHITESPACE_CHARS_NO_FF value for $characters, it should be researched whether that is correct by checking RFCs and other applicable rules. In some cases, it may also be prudent to evaluate how other projects similar to Requests, like for instance Guzzle, handle these text strings.
If the trimming is correct, it should be documented that this was verified via a comment in this issue.
If a change is necessary, please submit a PR and include tests safeguarding the change to the value passed for $characters.
After PR #1083, there will be 14 places in the codebase where this review task is necessary.
Setting the stage
By default, the PHP native
[lr]trim()functions, trim ASCII whitespace and the NUL byte character.PHP 8.6 changes the default value of the
$charactersparameter to also include the form feed -"\f"- character, which was previously not trimmed.[lr]trim()only operates on the leading and/or trailing characters for a text string. It does not affect the characters in the "middle" !Refs:
What has already been done
Requests uses
[lr]trim()in various places throughout the codebase.To make this code PHP cross-version compatible, PR #1083 made the following changes:
$charactersparameter.$charactersbeing trimmed explicit in each of the[lr]trim()function calls (if it wasn't already).To determine which characters should be trimmed, the following rule of thumb has been used for the initial changes made in PR #1083:
"Use the PHP 8.6 default (
Trim::WHITESPACE_CHARS), except when the trimming may be subject to an RFC or other documented rules, in which case use the PHP < 8.6 default (Trim::WHITESPACE_CHARS_NO_FF)"That way, we preserved existing behaviour in "important" places, while benefitting from the new default value everywhere else.
Task
For all calls to
[lr]trim()which currently use theTrim::WHITESPACE_CHARS_NO_FFvalue for$characters, it should be researched whether that is correct by checking RFCs and other applicable rules. In some cases, it may also be prudent to evaluate how other projects similar to Requests, like for instance Guzzle, handle these text strings.If the trimming is correct, it should be documented that this was verified via a comment in this issue.
If a change is necessary, please submit a PR and include tests safeguarding the change to the value passed for
$characters.After PR #1083, there will be 14 places in the codebase where this review task is necessary.