All,
In 7.0.11, the behavior of iconv_substr() was changed so that if the length argument is equal to the length of the string - an empty string is returned, as opposed to FALSE.
While I think there's a case to be made that this makes sense - what worries me is that we've introduced a behavioral change for a function in a bugfix release - one that has bitten us in Zend Framework - and I think it's safe to assume that it will affect plenty of other codebases.
Personally, I think we should be much more selective about introducing behavioral changes in maintenance releases, even if they can be considered bugfixes. Depending on their scope and impact, they should either go into the next mini version (this one probably falls in that category) or the next major version. Of course - there are plenty of bugfixes where the existing behavior is a crash, or undefined/unpredictable - those are obviously fine to fix without further discussion. I'm talking about the cases where the current behavior is very much defined and has been that way for a long time, but for whatever reason, people think it should change.
We've worked hard to establish trust that people can upgrade into the next maintenance version with minimal testing - which greatly helps people keep current with the latest and greatest fixes incl. security, and even the next minor version with relatively limited testing, when we ensured downwards compatibility within these versions. It does mean that our finger needs to be a lot more hesitant on the trigger when we evaluate behavioral changes such as these.
It's arguably too late to revert this patch now, but I think it's a good opportunity to discuss the guidelines for dealing with such issues.
Thoughts?
Zeev
All,
In 7.0.11, the behavior of
iconv_substr()was changed so that if the length argument is equal to the length of the string - an empty string is returned, as opposed to FALSE.While I think there's a case to be made that this makes sense - what worries me is that we've introduced a behavioral change for a function in a bugfix release - one that has bitten us in Zend Framework - and I think it's safe to assume that it will affect plenty of other codebases.
Personally, I think we should be much more selective about introducing behavioral changes in maintenance releases, even if they can be considered bugfixes. Depending on their scope and impact, they should either go into the next mini version (this one probably falls in that category) or the next major version. Of course - there are plenty of bugfixes where the existing behavior is a crash, or undefined/unpredictable - those are obviously fine to fix without further discussion. I'm talking about the cases where the current behavior is very much defined and has been that way for a long time, but for whatever reason, people think it should change.
We've worked hard to establish trust that people can upgrade into the next maintenance version with minimal testing - which greatly helps people keep current with the latest and greatest fixes incl. security, and even the next minor version with relatively limited testing, when we ensured downwards compatibility within these versions. It does mean that our finger needs to be a lot more hesitant on the trigger when we evaluate behavioral changes such as these.
It's arguably too late to revert this patch now, but I think it's a good opportunity to discuss the guidelines for dealing with such issues.
Thoughts?
What about bug #73817? (I'm omitting a proper link due to the current
mailing list issues regarding "spammy URLs" from php dot net.)
Some related code[1] is obviously wrong, but it is wrong since at least
PHP 5.4.x[2]. Fixing this may very well introduce a BC break, as any
user relying on the current behavior might already have introduced some
fix in userland. So, should the fix be postponed to PHP 7.2.0? :-)
[1]
https://github.com/php/php-src/blob/PHP-7.1.0/ext/standard/html.c#L1600-L1601
[2]
https://github.com/php/php-src/blob/PHP-5.4/ext/standard/html.c#L1583-L1584
--
Christoph M. Becker