Fixing bug #18556 (was: Complete case-sensitivity in PHP)

2012/4/22 C.Koycan5koy@gmail.com

But, I did not start this thread to discuss such bug fix, because:

1. It does not take a genius to figure it out, and should take minutes to
   implement for someone experienced in the internals. Given the 10 year
   span
   and dozens of comments/complaints on the bug's entry, it's hard to say
   this
   issue went unnoticed. So I had to conclude that such fix has quietly been
   overruled for performance and/or other undisclosed reasons.

Why does it matter if a solution is simple?

It doesn't matter, you've misunderstood.

You've misunderstood me. While you may have set out with the goal of
discussing making PHP completely case-sensitive, that doesn't preclude
others from suggesting fixes for the specific bug you mention. Indeed, some
of the first e-mails were around the bug, and not just in the context of
case-sensitive PHP.

I didn't introduce the custom case conversion solution as a
counter-argument to case-sensitive PHP, and I wasn't asking for feedback on
that solution in the context of case-sensitive PHP; I was asking for
reasons why it wouldn't be a suitable solution for the bug. The only place
case-sensitive PHP enters into it was your statement that:

As the recent comments on that page indicate, there's not a deterministic

way to resolve this issue, apart from eliminating tolower() calls for
function/class names during lookup. Hence totally case-sensitive PHP.

My proposition shows this is isn't entirely true, and branches off from the
original discussion at that point. I'm focusing on fixing the bug, which is
a smaller issue than case-sensitivity. Discussion of case-sensitivity can
continue without regard to the custom conversion solution. As such, I've
changed the subject of this e-mail.

Furthermore, going back to your original e-mail, you explicitly stated it
was about the bug, making case sensitivity subordinate to it.

This post is about bug #18556
(https://bugs.php.net/bug.php?**id=18556https://bugs.php.net/bug.php?id=18556)

which is a decade old.

I hope you can see why others might take the bug to be the context for
case-sensitivity, rather than the other way around.

And that's what makes me curious and confused about why this bug still

exists. See, I'm drawing a conclusion with what little information I have,
and stating the reasonings it's based on (first two statements).
Overall, that and the item following it were an explanation of "why I'm
suggesting a major feature change in solution to a specific bug", although
noone directly asked me to.

In other words, you jumped to a conclusion. I wasn't asking about possible
reasons why custom conversion hasn't been accepted as the solution to this
bug. Neither was I asking why you didn't suggest it. I was (and still am)
asking for explicit, justifiable reasons as to whether or not it's a
suitable solution to the bug.

If it's already been rejected privately, it's time to bring the reasons
into the open (which is why I asked). If not, it should be considered
publicly.

A comment dated 2002-09-26 on bug's page states the bug is fixed. The next
comment dated 2006-02-17 states it reappeared.
I don't know who did what 10, 6 years ago but it's been revoked. Why?
That was the main reason I deemed this bug not fixable, hence suggest
other ways to resolve.

I don't know either, but I'm not about to disregard potential fixes if
they haven't been publicly discussed. The regression could just as easily
have been a mistake. From looking at the original fix (revision 97040,
http://svn.php.net/viewvc?view=revision&revision=97040, authored by iliaa)
and the bug comments, something along the lines of what I'm suggesting has
been suggested and even implemented before, but there's no real discussion
of it. The original fix (zend_str_tolower_nlc) assumed ASCII, which isn't
entirely suitable as there are uppercase characters that it doesn't
convert, which suggests yet another reason for the regression, namely that
using zend_str_tolower would convert the characters that
zend_str_tolower_nlc missed.

As for the real reason why the bug reappeared, we can continue on in our
historical examination. Revision 99001 (
http://svn.php.net/viewvc?view=revision&revision=99001, also authored
by iliaa) replaced zend_str_tolower with zend_str_tolower_nlc, making all
internal Zend case conversion use ASCII. iliaa had this to say about the
change (http://news.php.net/php.zend-engine.cvs/478):

It appears that there no reason to keep both zend_str_tolower_nlc and

zend_str_tolower. zend_str_tolower_nlc can be safely renamed to
zend_str_tolower. The places it is used in, do not appear to depend on
locale. For people who do need it there is an alternative php function
php_strtolower, which they can use, which does respect the locale. So, if
there are no objections I'll prepare a patch that will change
zend_str_tolower_nlc to zend_str_tolower.

Revision 128057 (http://svn.php.net/viewvc?view=revision&revision=128057,
authored by sterling) adds zend_str_tolower for use in
fast_call_user_function, which makes use of tolower rather than a custom
conversion. Revision 128060 (
http://svn.php.net/viewvc?view=revision&revision=128060, same author) then
changes zend_str_tolower to use tolower instead of its custom ASCII-based
conversion. The commit message is: "make this faster and sexier". Within
these revisions, zend_lookup_class is case sensitive. This change, in
combination with 99001, mask the reason for the custom conversion.

Introduction of zend_tolower and use of tolower_l was introduced by
revision 224372 (http://svn.php.net/viewvc?view=revision&revision=224372,
authored by stas (hi, Stas!)). The commit message is: "Improve
tolower()-related functions on Windows and VC2005 by caching locale and
using tolower_l function."

There are plenty of other edits to Zend functions affecting case handling
(look over the commit messages listed in
http://svn.php.net/viewvc/php/php-src/trunk/Zend/zend_operators.c?view=log&pathrev=225000)
that make similar tweaks involving case conversion and the character
encoding. What are we to conclude from all this? That the custom conversion
was a bug fix was lost as the file was edited and different people worked
on it. In other words, the fix was not lost due to a conscious decision
made by anyone, but rather the typical reason for regression (in the
original sense of the word): there's too much for anyone to keep all of it
in mind at once, so someone can easily re-introduce a bug without being
aware of it.

I trust this demonstrates that "there must be an undisclosed reason" isn't
a justifiable reason not to implement my proposed solution.

The abstract property that makes a locale problematic is obvious. I

was looking for specific locales, as they need to be identified for a
complete solution.

I'm not locale expert. Given the public complaints/bugs we can, in
practice, assume this affects Turkish and Azerbaijani only. (I don't know
about Kurdish)

Kurdish is mentioned by Mike and Tokul in the comments for the bug. I
could easily have come to the same conclusion, but I want an answer from
someone who knows without needing to make any assumptions. Are there any
locale experts (or someone willing to put in the leg-work) reading this
with a conclusive answer to my question about problematic locales?

I've been experimenting with bare-bones PHP I've built from pristine
sources so far. Don't you think you should do the same, in dealing with
such a bug?

My personal system is a BSD derivative; the Turkish locales on these use
latin rather than Turkish case conversion (and installing a proper Turkish
locale is a mess), so I've been testing on another system. I've been
hesitant to use its resources too heavily for professional reasons. Running
a small PHP script is one thing; though time and space required for a PHP
build isn't large on modern systems, I can't justify doing so since it's
not directly related to site operations.

Try to var_dump the setLocale and see if it return the specified locale or
just 'false'. If false try the following:

setlocale(LC_ALL, 'tr_TR.UTF-8');

I had previously tested the locale by using "setlower('I')", as it tests
both that the locale exists and uses Turkish-langage case conversion. The
systems where I tested C.Koy's script passed the "setlower" test. Turned
out to be the Zend optimizer that prevented the error. With it not loaded,
the example script failed with a "Fatal error: Call to undefined function
IJK()" error message.

Here's a breakdown:

In both PHP 5.2 and 5.3, calling a function before defining it results in a
dynamic call (INIT_FCALL_BY_NAME+DO_FCALL_BY_NAME). Here's the PHP 5.2 dump
of C.Koy's example:

line # * op fetch ext return
operands

 2     0  >   FETCH_CONSTANT                                   ~0

'LC_CTYPE'
1 SEND_VAL
~0
2 SEND_VAL
'tr_TR'
3 DO_FCALL 2
'setlocale'
3 4 INIT_FCALL_BY_NAME
'IJK'
5 DO_FCALL_BY_NAME 0
4 6 NOP
5 7 > RETURN 1
8* > ZEND_HANDLE_EXCEPTION

Here's the 5.3 dump:
line # * op fetch ext return
operands

 2     0  >   EXT_STMT
       1      EXT_FCALL_BEGIN
       2      SEND_VAL                                                 2
       3      SEND_VAL

'tr_TR'
4 DO_FCALL 2
'setlocale'
5 EXT_FCALL_END
3 6 EXT_STMT
7 INIT_FCALL_BY_NAME
'ijk', 'IJK'
8 EXT_FCALL_BEGIN
9 DO_FCALL_BY_NAME 0
10 EXT_FCALL_END
4 11 EXT_STMT
12 NOP
5 13 > RETURN 1

From line 7 in the 5.3 dump, we see 5.3 converts the function name to
lowercase during compilation, but 5.2 doesn't. Examining the source
confirms this: you can see the lowercase conversion in 5.3's
zend_do_begin_dynamic_function_call on lines 1659 (for namespaced calls)
and 1683 (for non-namespaced calls) of zend_compile.c (
http://svn.php.net/viewvc/php/php-src/branches/PHP_5_3_10/Zend/zend_compile.c?revision=323023&view=markup#l1683),
while there's no such conversion in the same function in 5.2 (
http://svn.php.net/viewvc/php/php-src/branches/PHP_5_2/Zend/zend_compile.c?view=markup&pathrev=302150#l1450
).

5.3 only performs case conversion if the function name is a CONST
expression, which is why defining the function after calling it works but
calling a function with a variable name breaks. Correspondingly, the
ZEND_INIT_FCALL_BY_NAME_SPEC_HANDLER (in zend_vm_execute.h) uses the
first operand (which is already lowercased), while the other
INIT_FCALL_BY_NAME opcode handlers (ZEND_INIT_FCALL_BY_NAME_SPEC_HANDLER)
use the second, non-lowercased operand.

The 5.2 INIT_FCALL_BY_NAME opcode handlers only ever use the second,
un-lowercased operand.

So, what does this mean for fixing the bug? Not so much when the function
or class is stored in a variable, since these can't be converted to
lowercase at compile time without converting all variables, which is too
wasteful of both time and space (as both the unconverted and converted
strings would need to be stored). For object instantiation,
zend_do_begin_new_object gets the class name ultimately from the
namespace_name rule. zend_do_begin_new_object could then take the resulting
znode and create a second, lowercased copy, storing it as the second
operand. ZEND_NEW_SPEC_HANDLER would then be altered to use the second
operand (if not UNUSED) to instantiate the object. This certainly seems a
valid alternative to a lowercasing version of the namespace_name rule; it's
not as far reaching, which may be good (in that it has less impact) and bad
(in that there may be other instances of this bug that it won't fix).

However, neither the dual-operand solution nor lc_namespace_name will fix
the bug when the identifier is stored in a variable. That requires fixing
the run-time portion of PHP, in particular zend_fetch_class (or
zend_do_begin_class_member_function_call, zend_do_begin_new_object and
likely others) and the INIT_FCALL_BY_NAME handlers.

I get the feeling that there are still other cases yet to be discovered
where this bug surfaces.