On 16-Mar-07, at 4:49 AM, Timm Friebe wrote:
1) These changes are necessary. I don't see a bug ID in the CVS commit log anywhere.
There was no specific bug the change was addressing. The goal was to avoid potential buffer overflows inside the code possible to due invalid length calculations for sprintf() in a good number of instances. The change was made globally, so some safe uses, such as the one in the sybase extension were modified as well.
2) Although phpt-tests *are* provided, these most probably weren't run after committing these changes? They would have uncovered these problems right away. 3) While I can imagine the committers maybe don't have a sybase server at hand to test with, and that that's the reason for not running the tests, why don't they simply send the maintainers, who usually have things like this at hand to test with, a patch with the changes? Or at least send it to the list...
Maintainer is not always on hand and snapshots are often tested in other environments so often a check to determine if the code is working or not can be made based on user run tests. Unfortunately, the sybase extension did not have anyone try it out as part of the test suit, so this issue was not noticed until you've come across it.
4) When this new guideline of "avoiding sprintf" was introduced (must've been a couple of weeks ago, according to the commits) and why it was not announced - IIRC - to php-internals? Is it some kind of secret? Should I be on IRC or at conferences to know stuff like this?
The guideline is a work in progress and by the time 5.2.2 will be released the appropriate README files will be updated.
5) If I had not - by chance - recompiled PHP this would've probably been in 5.2.2-release, right? For code cleanliness and undocumented anti-sprintf reasons you risk breaking existing functionality...
I am finalizing a sp(p)rintf() checker for PHP code right now, so it would've detected the invalid usage, so 5.2.2 would've been safe. That said it is a good thing you've noticed the problem and brought it to everyone's attention.
Ilia Alshanetsky -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
