TLK: exit and MM
NEW: PHP 5.1 beta 2
NEW: Bug category change
TLK: include considered harmful
TLK: allow_url_fopen
CVS: ini_set/ini_get
PAT: Wanted - FastCGI patch

TLK: exit and MM

Marcus Börger found a strange issue while testing. Test scripts containing exit(0) weren't reporting memory leaks. Was this something that should be fixed?

Michael Wallner picked up on another aspect of the test scripts; he wasn't able to see memory leaks on CLI under win32 when outputting a debug string alongside fprintf(). Wez Furlong suggested skipping the fprintf() part under Windows, and Marcus suggested that this was probably a flushing issue exclusive to the Windows platform.

Zeev Suraski answered Marcus' original question when he next stepped off a plane. He explained that when zend_bailout() is used - as it is when exit() is called from userland PHP code - there could be memory blocks that are not freed in the proper sense. The Zend memory manager is relied upon to clean up in such cases.

Marcus wondered if it might be possible to change that situation when an exit() was not problematic, but Zeev pointed out that it wouldn't be straightforward to detect a non-problematic exit(), and demonstrated that leaks could occur even when exit() was okay. Derick backed Zeev on this, saying he'd had to overload the EXIT opcode to get profiling to work properly in Xdebug.

Short version: It's deliberate.

NEW: PHP 5.1 beta 2

Close to the beginning of the week, Andi Gutmans announced on internals@ that he'd rolled 'the new beta 2' and made it available for initial testing. He hoped to put it live the next day, barring any nasty surprises.

Sebastian Nohn almost immediately highlighted an unfixed bug, and Nuno Lopes reported a configuration problem over embedded SAPI library support on Solaris 9. Derick Rethans, who spent all last week sweating over the new core datetime extension ext/date, mentioned in passing that it was completely missing from the beta 2 tarball. Ilia Alshanetsky followed this with a report that the date extension hadn't been tagged.

Jani Taskinen was quick to fix Nuno's configure issue and Andi re-rolled the beta, this time with the date extension in it. Rasmus Lerdorf reported some issues there, including a bug in the new version of strtotime() which, he said, had broken one of his apps 'in a rather hard-to-debug way'. Derick promptly fixed the bug, but decided (along with Andi) that it would be best to get the beta out to a wider public and get the bug reports flowing. Rasmus' concern was that the brand new strtotime() implementation in PHP 5.1 would break code that used to work under PHP 4.3.*/5.0.*, and he felt that extensive testing should be applied to both the new and old implementations prior to any release. Derick said he had an exhaustive set of tests for precisely that purpose, but hadn't yet had the time to port them to the .phpt format used for the PHP test suite. Rasmus suggested he should 'let a worker bee have at it' rather than try to do everything himself.

Meanwhile the initial test suite results from all kinds of weird platforms all around the world were starting to drift in, with the vast majority reporting anything between 12 and 50 test failures. Irix 6.5 was a notable exception, with Irix user Joerg Behrens reporting 162 failed tests. Ilia pointed out that a large chunk of tests were failing because PHP couldn't run them - there was a missing DB connection. Over 70 such tests in ext/mysqli had the potential of failing for that reason and, Ilia added a little bitterly, 26 more were in ext/PDO_ODBC, where only IBM people have CVS access.

Wez wrote, somewhere between London and midnight, to explain that 'technically, IBM can't touch it either'. He offered to apply any patches that showed up in his inbox the following day, including 'a config.m4 patch' that ought to have been in beta 2 but which had slipped his mind. A confused Andi asked what he was talking about (it wasn't a patch for config.m4) and whether it was urgent enough to hold up the beta, but timezones intervened and the plea fell on deaf ears. Wez gave himself access to ext/PDO_ODBC and committed a fix for the test problem, plus a 'tweak' allowing the module to load correctly, the next time he woke up.

Meantime Kamesh Jayachandran, hoping to run the PHP test suite under NetWare, asked where run-tests2.php had gone. Nuno explained that the file is now known as server-tests.php and has had a new lease of life under that name.

Andi went ahead and rolled 'beta 2 take infinity', pleading, 'Try not to find show stoppers unless they format your hard drive'. Sean Coates chose that moment to suggest that the copyright years should be updated to include this year for php -v and phpversion(). He's still alive.

Short version: This release could take a while to struggle into daylight.

NEW: Bug category change

Andi asked whether the bugs database could possibly be altered so that all Zend Engine bugs were reported to the same place. This was agreed and done on the spot; anyone reporting a ZE 2 bug now will need to put it under 'Scripting Engine problem'.

Short version: A simpler life for ZE bug reporters and fixers.

TLK: include considered harmful

Someone with no name (apparently) wrote to the internals list suggesting that include() is intrinsically harmful. S/he suggested the dev team should google for 'PHP security flaw', before going on to say that the problem outlined in the top return page for the search term had been introduced to the company server by two 'naive' PHP programmers in succession. S/he then pointed out that 'crackers have created security tools designed specifically to exploit this flaw', concluding, 'This security flaw is common'. S/he was aware that it was avoidable, but the evidence showed that many PHP script programmers were not. The suggestion s/he made was that include() should be altered to only include files at or below the current directory level, and a second function named something like includeremotesecurityhole() should behave the way include() currently does, i.e. have the ability to 'fetch remote hostile content and execute it with local privileges'.

Someone else with no name (apparently) suggested that the first someone should disable allow_url_fopen() on the server. This lead almost directly to the next, horrendously long thread...

Short version: User education failure? Or a failure in PHP?

TLK: allow_url_fopen

Someone with a name, Yasuo Ohgaki, wrote requesting that the default setting for the php.ini directive allow_url_fopen should be changed back to INI_ALL. It had been changed to INI_SYSTEM in PHP 4.3.4, which meant that it was no longer possible to 'patch' a vulnerable application using ini_set() in the script. He also argued that the programmer, rather than the system administrator, should have control over whether a program was allowed to access remote files. The best solution, he felt, would be to change allow_url_fopen back to INI_ALL, but disable the setting by default, and these changes should be made in both PHP 5.1 and PHP 4.4.

Johannes Schlüter argued for the administrator; if an application needed to use fopen() for URLs, the administrator could check the application was safe before enabling the option on a per-host or per-directory basis.

Sara Golemon showed a workaround for PHP 5, using the streams API:

$allowed_wrappers = array('php', 'file');
foreach(
stream_get_wrappers () as $wrapper) {
    if (!
in_array($wrapper, $allowed_wrappers)) {
        
stream_wrapper_unregister ($wrapper);
    }
}

She pointed out that allow_url_fopen is there so that site administrators can globally prevent users from being stupid. Given that context, it didn't make sense to allow the user to override the setting, and Sara felt that the combination of stream_wrapper_unregister() and stream_wrapper_restore() offered a solution allowing the programmer to achieve everything they wanted without reducing the administrator's control over security on his or her server. The only snag was that the stream_wrapper_*() functions are in PHP 5 only, and although technically it would be fairly straightforward to merge them to PHP 4_4 branch, politically it would depend on the Release Master and general consensus. Derick, speaking as Release Master for PHP 4.4.*, was very quick to respond to her email with a blunt 'no new features'.

Messju Mohr argued that allowing the server administrator control at this point would simply break otherwise working applications. Stefan Esser disagreed, giving his view that 'any application that cannot deal with different setups is simply broken'. The more pacific Yasuo looked for a base statement that everyone could agree upon before the battle commenced in earnest: 'allow_url_fopen = On is dangerous and the feature is not useful most of the time'.

The point he'd been trying to make was that allow_url_fopen is actually ON by default, and the setting cannot be changed from within the script. This was obviously less secure than having allow_url_fopen OFF by default and enabled by the programmer when needed. This wasn't a new feature, but a reversion to the original behaviour; could Derick please respond to this request?

Short version: Not that long a thread? - they're still arguing over it :-(

CVS: ini_set/ini_get

Tony Dovgal caused a minor riot when he fixed an elderly bug whereby Boolean .ini options could be incorrectly displayed as Off when they were in fact On. The fix included Zend Engine changes allowing ini_set() and CLI -d to use "yes" and "true" for certain .ini settings. Ilia wondered if there was really a need 'to support every synonym of the word "On" on the planet', but Stas Malyshev supported Tony's position, saying it was a source of confusion to allow synonyms in one place but not another. Ilia argued that fixing that particular bug could in fact add to the confusion, given that true/yes could mean Off in old PHP versions and On in PHP 5.1b2+. Stas felt it was more confusing to live with the bug. The argument went on for some time, ending only when Andi stated his opinion that .ini settings should indeed be consistent.

Meanwhile Ilia noticed that Georg Richter, working quietly away at ext/mysqli, had managed to break the extension's compilation - not generally a big issue, but it happened during Andi's fifteenth-or-so attempt to create the beta. While Andi awaited Georg's response, Rob Richards arrived on the scene with a patch for ext/dom that he'd hoped to squeeze into beta 2. Andi allowed him that, and Ilia subsequently made a temporary fix in ext/mysqli to enable him to finally roll the release.

Short version: A bit of a push.

PAT: Wanted - FastCGI patch

Jani reviewed and committed the usual bunch of NetWare fixes for Kamesh but, in a surprise move, Derick suddenly started working with Kamesh when Jani wasn't around. A grateful Kamesh addressed his email to Derick for the rest of the week.

Tony submitted a patch to fix bug 29983 (unable to set default_charset using ini_set()), which Ilia initially reviewed. Andi also looked at it, and asked him to wait for the beta release and wider peer review.

One Jochen Hansper sent in a patch, initially against PHP 4_3 branch (!), to support the cookie attribute httpOnly in ext/session. His idea included a new php.ini setting, session.cookie_httponly. By the time Jochen got around to writing his patch against CVS HEAD, it had grown to include support for httpOnly cookies in setcookie() and setrawcookie(), but Nuno was quick to say that it had been decided some weeks ago not to add further parameters to these functions, and Andi backed this observation. Jochen patiently rewrote his patch to overload those functions with an array as the third parameter, as suggested, but several people explained that the original plan had been to have an array with named keys, at which point Jochen appears to have given up on the project.

Meanwhile, Jani committed a patch from an unnamed person, adding session.hash_bits_per_character support in ext/session.

Andrew Prendergast, who undertook a thorough investigation of memory fragmentation in FastCGI with a fair amount of support from the PHP development team behind the scenes, chased up on the patch he produced a couple of weeks ago. The patch proposes implementations of two new environmental variables, PHP_FCGI_MAX_RAM_MB (allowing the user to limit the size of an FCGI process) and PHP_FCGI_MAX_RAM_INCREASE (allowing a maximum percentage of RAM usage increase compared to that being used at the start of the process). If either limit is exceeded, the process will be restarted following script execution (the important part). Andrew reported that this allows a high setting for PHP_FCGI_MAX_REQUESTS, which in turn gives a big performance boost in terms of requests per second.

It's not in PAT because the attachment keeps failing to reach the internals list; Andrew, please could you either put your patch online somewhere and mail a link to internals@, or contact me directly so I can upload it here. It'd be kinda nice to see it :)

Short version: Nothing new for the PAT directory this week.