Fix issue in arcanist whereby when doing an arc patch involving adding or removing a binary file, it falls over with strlen(null) errors.
Fixes T15617
Details
Details
- Reviewers
avivey - Group Reviewers
O1: Blessed Committers - Maniphest Tasks
- T15617: arc patch fails when binary files are added/removed with PHP 8.1 strlen(null)
- Commits
- rARC8b907d771661: Fix PHP 8.1 arc patch strlen(null) binary file error
arc patch Dxxxx
Diff Detail
Diff Detail
- Repository
- rARC Arcanist
- Branch
- patch (branched from master)
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 783 Build 783: arc lint + arc unit
Event Timeline
| src/parser/ArcanistBundle.php | ||
|---|---|---|
| 752 | Note: Changing the default to '' doesn't work, because $old_data might be set to null by $this->getBlob() on line 762. | |
| 765 | Note: $old_data is explicitly allowed to be null, as per line 781 below. | |
| 790 | Note: as with $old_data, $new_data can be set to null on line 795 below, so changing the default to '' here doesn't work. | |
Comment Actions
phutil_nonempty_string is basically the exact same thing, but with stricter type control
| src/parser/ArcanistBundle.php | ||
|---|---|---|
| 765 | ||
This comment was removed by Sten.
Comment Actions
Right - but we are expecting only a null or a string. If we get anything else then I want strlen to fall over, following the fail fast principle.
Comment Actions
I think phutil_nonempty_string fails faster then strlen (strlen allowing automatic conversion of string-like things to string and phutil_nonempty_string doesn't), but I'm a little confused right now...
Comment Actions
/me checks phutil_nonempty_string()
OK - I was not expecting that side effect of throwing an exception if passed something other than a null or string. I would have expected it to return false.