Zend Weekly Summaries Issue #242

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 "http://bugs.php.net/33420">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:

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 "http://bugs.php.net/15854">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.