Everything

php

Directoryiterator / preg_match behavior

php.internals

Aaron Wormus

22 years ago

When playing with PHP5 I tried the following code:

foreach (new DirectoryIterator('.') as $file) {
if (preg_match("/^\./", $file)){
continue;
}
print "<a href='$file'>$file</a><br />\n";
}

the result was unexpected, but after an IRC session and reading the
manual all became clear.

My question is if the following code is the expected behaviour? I'm
running E_STRICT and would expect at least a warning...

foreach (new DirectoryIterator('.') as $file) {
echo "<pre>";
var_dump($file);

if (preg_match("/xxx/", $file)){
// Why is this breaking $file??
//    without even a warning
}

var_dump($file);
echo "</pre>";
}

strpos and substr work fine, and I was kindly informed that they do a
convert_to_string_ex() on the object before they do their thing. Would
it be breaking anything if, for consistancy, the same functionality was
put into preg_match?

I'm all sorted out now but thought to send this off incase it was indeed
a bug somewhere.

Thanks

Aaron

Curt Zirzow

22 years ago

* Thus wrote Aaron Wormus:

>
> foreach (new DirectoryIterator('.') as $file) {
>     echo "<pre>";
>     var_dump($file);
>
>     if (preg_match("/xxx/", $file)){
>         // Why is this breaking $file??
> 	//    without even a warning
>     }
>
>     var_dump($file);
>     echo "</pre>";
> }

This probably needs some attention, here are some related issues that
I've came accross trying to find out why the above was happening:


Test 1: ($file holds last type conversion)
<?php
foreach (new DirectoryIterator('.') as $file) {
echo "\n--\n";
var_dump($file);
preg_match("/xxx/", $file);
var_dump($file);
echo "\n--";
}

Output:
Object(DirectoryIterator)#1 (0) {
}
string(1) "."

--
--
string(1) "."
string(1) "."
--
...


Test 2: (working example)
----------
<?php
$d = new DirectoryIterator('.');
foreach ($d as &$file) {
echo "\n--\n";
var_dump($file);
preg_match("/xxx/", $file);
var_dump($file);
echo "\n--";
}

Output:
--
object(DirectoryIterator)#1 (0) {
}
object(DirectoryIterator)#1 (0) {
}
--
--
object(DirectoryIterator)#1 (0) {
}
object(DirectoryIterator)#1 (0) {
}
--

...

Test 3: (completly fails)
----------
<?php
foreach (new DirectoryIterator('.') as &$file) {
echo "\n--\n";
var_dump($file);
preg_match("/xxx/", $file);
var_dump($file);
echo "\n--\n";
}

Output:
PHP Parse error:  syntax error, unexpected T_STRING in test.php on line 2


This appears to be a definate bug somewhere and I cant spot it atm.


Curt
--
First, let me assure you that this is not one of those shady pyramid schemes
you've been hearing about.  No, sir.  Our model is the trapezoid!

Andrei Zmievski

22 years ago

On Thu, 02 Sep 2004, Curt Zirzow wrote:

> Test 1: ($file holds last type conversion)
> <?php
> foreach (new DirectoryIterator('.') as $file) {
>     echo "\n--\n";
>     var_dump($file);
>     preg_match("/xxx/", $file);
>     var_dump($file);
>     echo "\n--";
> }
>
> Output:
> Object(DirectoryIterator)#1 (0) {
> }
> string(1) "."
> --
> --
> string(1) "."
> string(1) "."
> --
> ...

The culprit is this piece of code in zend_parse_arg_impl():

case IS_OBJECT: {
if (Z_OBJ_HANDLER_PP(arg, cast_object)
&& Z_OBJ_HANDLER_PP(arg, cast_object)(*arg, *arg, IS_STRING, 0 TSRMLS_CC) == SUCCESS) {
*pl = Z_STRLEN_PP(arg);
*p = Z_STRVAL_PP(arg);
break;
}

The cast_object handler overwrites the value in *arg and that's why the
original variable changes type. I think Andi made this change on Dec 02,
2003. I can see the rationale behind not using a new var for the
conversion, because then we have to keep track of it and release it
somehow, but changing variable types is not good either. Andi, any
suggestions?

- Andrei

Andi Gutmans

21 years ago

It probably makes sense to add SEPARATE_ZVAL_IF_NOT_REF() to that code.
This would provide the same behavior as for example for long's.
Can you please check out the following patch?

Index: zend_API.c
===================================================================
RCS file: /repository/ZendEngine2/zend_API.c,v
retrieving revision 1.257
diff -u -r1.257 zend_API.c
--- zend_API.c  24 Aug 2004 18:47:18 -0000      1.257
+++ zend_API.c  4 Sep 2004 01:17:19 -0000
@@ -318,6 +318,7 @@
case IS_OBJECT: {
if (Z_OBJ_HANDLER_PP(arg,
cast_object)
&&
Z_OBJ_HANDLER_PP(arg, cast_object)(*arg, *arg, IS_STRING, 0 TSRMLS_CC) ==
SUCCESS) {
+
SEPARATE_ZVAL_IF_NOT_REF(arg);
*pl =
Z_STRLEN_PP(arg);
*p = Z_STRVAL_PP(arg);
break;


At 10:43 AM 9/2/2004 -0700, Andrei Zmievski wrote:

Curt Zirzow

21 years ago

* Thus wrote Andi Gutmans:

> It probably makes sense to add SEPARATE_ZVAL_IF_NOT_REF() to that code.
> This would provide the same behavior as for example for long's.
> Can you please check out the following patch?

Nice! That fixed the $file holding the value between iterations

Test 1: ($file now is reset on iteration)
<?php
foreach (new DirectoryIterator('.') as $file) {
echo "\n--\n";
var_dump($file);
preg_match("/xxx/", $file);
var_dump($file);
echo "--";
}

--
object(DirectoryIterator)#1 (0) {
}
string(1) "."
--
--
object(DirectoryIterator)#1 (0) {
}
string(2) ".."
--


What are the possibilities of thrown E_STRICT or E_WARNING, since
the object gets lost?  Or should this simply be a documented
behaviour? If possible, any pointers where to consider applying them
at?

From what I've seen there are a lot of convert_to_xxx_ex()'s around
and can cause a some of unexpected results, without some sort of
notice.  Of course ideally, an attempt to fetch a __toString()
value would be really nice :)  I'm way over my head at this point.


Curt
--
First, let me assure you that this is not one of those shady pyramid schemes
you've been hearing about.  No, sir.  Our model is the trapezoid!

Andi Gutmans

21 years ago

At 03:46 AM 9/4/2004 +0000, Curt Zirzow wrote:

>* Thus wrote Andi Gutmans:
> > It probably makes sense to add SEPARATE_ZVAL_IF_NOT_REF() to that code.
> > This would provide the same behavior as for example for long's.
> > Can you please check out the following patch?
>
>Nice! That fixed the $file holding the value between iterations
>
>Test 1: ($file now is reset on iteration)
>   <?php
>   foreach (new DirectoryIterator('.') as $file) {
>     echo "\n--\n";
>     var_dump($file);
>     preg_match("/xxx/", $file);
>     var_dump($file);
>     echo "--";
>   }
>--
>object(DirectoryIterator)#1 (0) {
>}
>string(1) "."
>--
>--
>object(DirectoryIterator)#1 (0) {
>}
>string(2) ".."
>--
>
>
>What are the possibilities of thrown E_STRICT or E_WARNING, since
>the object gets lost?  Or should this simply be a documented
>behaviour? If possible, any pointers where to consider applying them
>at?

Didn't quite understand. Why does the object get lost after my fix?

> >From what I've seen there are a lot of convert_to_xxx_ex()'s around
>and can cause a some of unexpected results, without some sort of
>notice.  Of course ideally, an attempt to fetch a __toString()
>value would be really nice :)  I'm way over my head at this point.

convert_to_xx_ex() does not destroy any values (except if they are by
reference).

Andi

Curt Zirzow

21 years ago

* Thus wrote Andi Gutmans:

> At 03:46 AM 9/4/2004 +0000, Curt Zirzow wrote:
> >
> >What are the possibilities of thrown E_STRICT or E_WARNING, since
> >the object gets lost?  Or should this simply be a documented
> >behaviour? If possible, any pointers where to consider applying them
> >at?
>
> Didn't quite understand. Why does the object get lost after my fix?

It was getting lost in the call to preg_match, prior to the patch.

>
> >>From what I've seen there are a lot of convert_to_xxx_ex()'s around
> >and can cause a some of unexpected results, without some sort of
> >notice.  Of course ideally, an attempt to fetch a __toString()
> >value would be really nice :)  I'm way over my head at this point.
>
> convert_to_xx_ex() does not destroy any values (except if they are by
> reference).

Correct.  I might be wrong on blaming it on convert_to_xx_ex(), but
there definately is something wrong when passing builtin classes
to certain functions, user defined classes behave differently:

<?php

class o {
function __toString() {
return 'a string';
}
}

echo "User class:\n";
$o = new o();
preg_match('/x/', $o);  /* warning issued: expected string */
var_dump($o);           /*  object(0) #1 */

echo "\nDirectory Iterator:\n";
$d = new DirectoryIterator('.');
preg_match('/x/', $d);  /* no warning */
var_dump($d);           /* string(1) "." */

echo "\nTidy:\n";
$t = new Tidy();
preg_match('/x/', $t);  /* no warning */
var_dump($t);           /* string(1) "\n" */



Curt

--
First, let me assure you that this is not one of those shady pyramid schemes
you've been hearing about.  No, sir.  Our model is the trapezoid!

Andi Gutmans

21 years ago

Can you please try again (HEAD)?
Let me know if it's OK and I'll merge to PHP_5_0

Andi

At 05:51 PM 9/5/2004 +0000, Curt Zirzow wrote:

Curt Zirzow

21 years ago

* Thus wrote Andi Gutmans:

> Can you please try again (HEAD)?
> Let me know if it's OK and I'll merge to PHP_5_0

Works good now.  Passes the attached test now.

Curt

--
First, let me assure you that this is not one of those shady pyramid schemes
you've been hearing about.  No, sir.  Our model is the trapezoid!

Andi Gutmans

21 years ago

Thanks. I merged it into 5.0.x

At 02:35 AM 9/9/2004 +0000, Curt Zirzow wrote: