[PATCH] zend-multibyte unicode detection vs. __halt_compiler()

php.internals

LAUPRETRE François (P)

19 years ago
Hi, Here is a patch I am submitting to fix bug #42396 (PHP 5). The problem: when PHP is configured with the '--enable-zend-multibyte' option, it tries to autodetect unicode-encoded scripts. Then, if a script contains null bytes after an __halt_compiler() directive, it will be considered as UTF-16 or 32, and the execution typically results in a lot of '?' garbage. In practice, it makes PHK and PHAR incompatible with the zend-multibyte feature. The only workaround was to turn off the (undocumented) 'detect_unicode' flag. But it is not a real solution, as people may want to use unicode detection along with PHK/PHAR packages, and there's no logical reason to keep them incompatible. The patch I am submitting assumes that a document encoded in UTF-8, UTF-16, or UTF-32 cannot contain a sequence of four 0xff bytes. So, it adds a small detection loop before scanning the script for null bytes. If a sequence of 4 0xff is found, the unicode detection is aborted and the script is considered as non unicode, whatever other binary data it can contain. Of course, this detection happens after looking for a byte-order mark. Now, I can modify the PHK_Creator tool to set 4 0xff bytes after the __halt_compiler() directive, which makes the generated PHK archives compatible with zend-multibyte. The same for PHAR. It would be better if we could scan the script for null bytes only up to the __halt_compiler() directive, but I suspect it to be impossible as it is not yet compiled... Regards Francois --- zend_multibyte.c.old 2007-01-01 10:35:46.000000000 +0100 +++ zend_multibyte.c 2007-08-23 17:22:24.000000000 +0200 @@ -1035,6 +1035,7 @@ zend_encoding *script_encoding = NULL; int bom_size; char *script; + unsigned char *p,*p_end; if (LANG_SCNG(script_org_size) < sizeof(BOM_UTF32_LE)-1) { return NULL; @@ -1069,6 +1070,18 @@ return script_encoding; } + /* Search for four 0xff bytes - if found, script cannot be unicode */ + + p=(unsigned char *)LANG_SCNG(script_org); + p_end=(p+LANG_SCNG(script_org_size)-3); + while (p < p_end) { + if ( ((* p) ==(unsigned char)0x0ff) + && ((*(p+1))==(unsigned char)0x0ff) + && ((*(p+2))==(unsigned char)0x0ff) + && ((*(p+3))==(unsigned char)0x0ff)) return NULL; + p++; + } + /* script contains NULL bytes -> auto-detection */ if (memchr(LANG_SCNG(script_org), 0, LANG_SCNG(script_org_size))) { /* make best effort if BOM is missing */

Rui Hirokawa

19 years ago
Hi, IMHO, #42396 is not a bug, but it is the specification. The normal script doesn't contain a null byte if it is not encoded in Unicode. It is understandable the addition of a unique byte seqence '0xFFFFFFFF' detection to support PHAR/PHK, but it is a change to add a new feature. Rui On Thu, 23 Aug 2007 18:58:52 +0200 LAUPRETRE Fran輟is (P) <francois.laupretre@ratp.fr> wrote:
> Hi, > > Here is a patch I am submitting to fix bug #42396 (PHP 5). > > The problem: when PHP is configured with the '--enable-zend-multibyte' option, it tries to autodetect unicode-encoded scripts. Then, if a script contains null bytes after an __halt_compiler() directive, it will be considered as UTF-16 or 32, and the execution typically results in a lot of '?' garbage. In practice, it makes PHK and PHAR incompatible with the zend-multibyte feature. > > The only workaround was to turn off the (undocumented) 'detect_unicode' flag. But it is not a real solution, as people may want to use unicode detection along with PHK/PHAR packages, and there's no logical reason to keep them incompatible. > > The patch I am submitting assumes that a document encoded in UTF-8, UTF-16, or UTF-32 cannot contain a sequence of four 0xff bytes. So, it adds a small detection loop before scanning the script for null bytes. If a sequence of 4 0xff is found, the unicode detection is aborted and the script is considered as non unicode, whatever other binary data it can contain. Of course, this detection happens after looking for a byte-order mark. > > Now, I can modify the PHK_Creator tool to set 4 0xff bytes after the __halt_compiler() directive, which makes the generated PHK archives compatible with zend-multibyte. The same for PHAR. > > It would be better if we could scan the script for null bytes only up to the __halt_compiler() directive, but I suspect it to be impossible as it is not yet compiled... > > Regards > > Francois > > --- zend_multibyte.c.old 2007-01-01 10:35:46.000000000 +0100 > +++ zend_multibyte.c 2007-08-23 17:22:24.000000000 +0200 > @@ -1035,6 +1035,7 @@ > zend_encoding *script_encoding = NULL; > int bom_size; > char *script; > + unsigned char *p,*p_end; > > if (LANG_SCNG(script_org_size) < sizeof(BOM_UTF32_LE)-1) { > return NULL; > @@ -1069,6 +1070,18 @@ > return script_encoding; > } > > + /* Search for four 0xff bytes - if found, script cannot be unicode */ > + > + p=(unsigned char *)LANG_SCNG(script_org); > + p_end=(p+LANG_SCNG(script_org_size)-3); > + while (p < p_end) { > + if ( ((* p) ==(unsigned char)0x0ff) > + && ((*(p+1))==(unsigned char)0x0ff) > + && ((*(p+2))==(unsigned char)0x0ff) > + && ((*(p+3))==(unsigned char)0x0ff)) return NULL; > + p++; > + } > + > /* script contains NULL bytes -> auto-detection */ > if (memchr(LANG_SCNG(script_org), 0, LANG_SCNG(script_org_size))) { > /* make best effort if BOM is missing */ >
-- Rui Hirokawa <rui_hirokawa@ybb.ne.jp>

LAUPRETRE François (P)

18 years ago
Hi,
> From: Rui Hirokawa > > IMHO, #42396 is not a bug, but it is the specification. > The normal script doesn't contain a null byte if it is not > encoded in Unicode. > > It is understandable the addition of a unique byte seqence > '0xFFFFFFFF' detection to support PHAR/PHK, > but it is a change to add a new feature.
Sorry to insist but, since __halt_compiler() was introduced, your assertion is not true any more. Actually, it depends on what you consider as 'the script' : if you just consider the data from the beginning of the file to the __halt_compiler() directive, that's right: if this data contains a null byte, it is unicode. But the current unicode detection is not aware of the __halt_compiler() directive, and it scans the whole file. So, your assertion is wrong: it is perfectly legitimate to have a non-unicode script contain null bytes (if they are after an __halt_compiler() directive). So, it is a bug and not a feature request. This side effect was not identified when __halt_compiler() was added. The obvious solution is to decide that a non-unicode script cannot contain null bytes, even after an __halt_compiler(). It would just require three lines in the PHP doc. But that would introduce a severe limitation and, in practice, would make the __halt_compiler() feature almost useless. The solution I am proposing is not very elegant but it is the only one I found which does not make __halt_compiler() and multibyte incompatible. As __halt_compiler() was introduced recently, and as, afaict, the only software to use it are PHAR and PHK, I consider it as acceptable, if not perfect. Greg, Marcus, do you have a better idea ? I considered that unicode detection is done before __halt_compiler() can be detected, do you confirm ? Regards Francois

Greg Beaver

18 years ago
LAUPRETRE François (P) wrote:
> Hi, > >> From: Rui Hirokawa >> >> IMHO, #42396 is not a bug, but it is the specification. The normal >> script doesn't contain a null byte if it is not encoded in Unicode. >> >> >> It is understandable the addition of a unique byte seqence >> '0xFFFFFFFF' detection to support PHAR/PHK, but it is a change to >> add a new feature. > > Sorry to insist but, since __halt_compiler() was introduced, your > assertion is not true any more. > > Actually, it depends on what you consider as 'the script' : if you > just consider the data from the beginning of the file to the > __halt_compiler() directive, that's right: if this data contains a > null byte, it is unicode. > > But the current unicode detection is not aware of the > __halt_compiler() directive, and it scans the whole file. So, your > assertion is wrong: it is perfectly legitimate to have a non-unicode > script contain null bytes (if they are after an __halt_compiler() > directive). So, it is a bug and not a feature request. This side > effect was not identified when __halt_compiler() was added. > > The obvious solution is to decide that a non-unicode script cannot > contain null bytes, even after an __halt_compiler(). It would just > require three lines in the PHP doc. But that would introduce a severe > limitation and, in practice, would make the __halt_compiler() feature > almost useless. > > The solution I am proposing is not very elegant but it is the only > one I found which does not make __halt_compiler() and multibyte > incompatible. As __halt_compiler() was introduced recently, and as, > afaict, the only software to use it are PHAR and PHK, I consider it > as acceptable, if not perfect. > > Greg, Marcus, do you have a better idea ? I considered that unicode > detection is done before __halt_compiler() can be detected, do you > confirm ?
unicode detection in mb_string is in fact done before __halt_compiler(). I don't think there is a solution to this problem without changes to PHP. Fortunately, PHP 6 introduces usage of declare (please correct me if I'm wrong) that allows declaration of the file's encoding, which would remove the guesswork. I think the best thing in this case is to recommend that multibyte auto-detection be disabled, and wait for PHP 6 which provides a proper solution to the unicode encoding issue. Greg

Rui Hirokawa

18 years ago
Hi, The declare() semantic like, declare(encoding="Shift_JIS"); is already supported by mbstring since PHP 4.3. 1.set detect_unicode = Off 2.adding declare(encoding="encoding_name") on the first line of script can be your solution ? Rui On Fri, 07 Sep 2007 14:40:14 -0500 Greg Beaver <greg@chiaraquartet.net> wrote:
> LAUPRETRE Fran輟is (P) wrote: > > Hi, > > > >> From: Rui Hirokawa > >> > >> IMHO, #42396 is not a bug, but it is the specification. The normal > >> script doesn't contain a null byte if it is not encoded in Unicode. > >> > >> > >> It is understandable the addition of a unique byte seqence > >> '0xFFFFFFFF' detection to support PHAR/PHK, but it is a change to > >> add a new feature. > > > > Sorry to insist but, since __halt_compiler() was introduced, your > > assertion is not true any more. > > > > Actually, it depends on what you consider as 'the script' : if you > > just consider the data from the beginning of the file to the > > __halt_compiler() directive, that's right: if this data contains a > > null byte, it is unicode. > > > > But the current unicode detection is not aware of the > > __halt_compiler() directive, and it scans the whole file. So, your > > assertion is wrong: it is perfectly legitimate to have a non-unicode > > script contain null bytes (if they are after an __halt_compiler() > > directive). So, it is a bug and not a feature request. This side > > effect was not identified when __halt_compiler() was added. > > > > The obvious solution is to decide that a non-unicode script cannot > > contain null bytes, even after an __halt_compiler(). It would just > > require three lines in the PHP doc. But that would introduce a severe > > limitation and, in practice, would make the __halt_compiler() feature > > almost useless. > > > > The solution I am proposing is not very elegant but it is the only > > one I found which does not make __halt_compiler() and multibyte > > incompatible. As __halt_compiler() was introduced recently, and as, > > afaict, the only software to use it are PHAR and PHK, I consider it > > as acceptable, if not perfect. > > > > Greg, Marcus, do you have a better idea ? I considered that unicode > > detection is done before __halt_compiler() can be detected, do you > > confirm ? > > unicode detection in mb_string is in fact done before __halt_compiler(). > I don't think there is a solution to this problem without changes to PHP. > > Fortunately, PHP 6 introduces usage of declare (please correct me if I'm > wrong) that allows declaration of the file's encoding, which would > remove the guesswork. > > I think the best thing in this case is to recommend that multibyte > auto-detection be disabled, and wait for PHP 6 which provides a proper > solution to the unicode encoding issue. > > Greg >
-- Rui Hirokawa <rui_hirokawa@ybb.ne.jp>

LAUPRETRE François (P)

18 years ago
> From: Rui Hirokawa [mailto:rui_hirokawa@ybb.ne.jp] > > 1.set detect_unicode = Off > 2.adding declare(encoding="encoding_name") on the first line > of script can be your solution ?
The 'declare' statement is not a solution because unicode detection is done before it is seen. So, PHK and PHAR remain incompatible with zend_multibyte. Can you confirm that unicode detection does not exist in PHP 6 (keeping only 'declare' statements) ? As a workaround in PHP 5, would it be possible to add a 'detect_unicode=off' line in php.ini-dist and php.ini-recommended ? Regards Francois

Rui Hirokawa

18 years ago
Changing 'detect_unicode' disabled as default is a possible solution. But, it might cause some backward incompatibility issues. If no one gives any objection within a coupled of days, I will commit the change ('detect_unicode' will be disabled by default). Rui On Thu, 27 Sep 2007 18:37:57 +0200 LAUPRETRE François (P) <francois.laupretre@ratp.fr> wrote:
> > From: Rui Hirokawa [mailto:rui_hirokawa@ybb.ne.jp] > > > > 1.set detect_unicode = Off > > 2.adding declare(encoding="encoding_name") on the first line > > of script can be your solution ? > > The 'declare' statement is not a solution because unicode detection is done before it is seen. So, PHK and PHAR remain incompatible with zend_multibyte. > > Can you confirm that unicode detection does not exist in PHP 6 (keeping only 'declare' statements) ? > > As a workaround in PHP 5, would it be possible to add a 'detect_unicode=off' line in php.ini-dist and php.ini-recommended ? > > Regards > > Francois >
-- Rui Hirokawa <rui_hirokawa@ybb.ne.jp>