Ilia, I think there is a problem with your latest fixes on the 4_3 branch.
Stuff that used to work is now broken. Stuff like this:
main.php:
include 'inc1.inc';
inc1.inc:
@include 'inc2.inc';
If inc2.inc does not exist we now get an error similar to:
Warning: main(./lang/serendipity_lang_.inc.php): failed to open stream: No
such file or directory in /var/www/s9y/serendipity_lang.inc.php on line 5
Warning: main(): Failed opening './inc2.inc' for
inclusion (include_path='.:/usr/local/lib/php') on line {the include line #}
Fatal error: Parse error inside included file. in
/var/www/htdocs/inc1.inc on line {the include line #}
Remember that it is ok for an include to not find the file. We issue a
warning and move on. It should in no way be treated as a fatal error.
-Rasmus
Remember that it is ok for an include to not find the file. We issue a
warning and move on. It should in no way be treated as a fatal error.
But it should on a parse error in an include file, not?
Derick
Remember that it is ok for an include to not find the file. We issue a
warning and move on. It should in no way be treated as a fatal error.But it should on a parse error in an include file, not?
Yes, but a failed include is not a parse error.
-Rasmus
Remember that it is ok for an include to not find the file. We issue a
warning and move on. It should in no way be treated as a fatal error.But it should on a parse error in an include file, not?
Yes, but a failed include is not a parse error.
I know, I was just making sure while fixing it :)
Derick
It seems like the only way to distinguish between a parse error and a
non-existant file for regular include() is by doing a zend_stream_open() upon
failure to determine if the file is avaliable. If it is, then we return a
parse error and if it does not we continue execution. This does add a small
overhead for failed includes, but IMHO if a non-existant files are being
included performance is not a big consideration.
That said, if there is much opposition to the approach I would be happy to
revert the code to the previous state.
Ilia
P.S. Suggested 'fix' is attached.
Ilia, I think there is a problem with your latest fixes on the 4_3 branch.
Stuff that used to work is now broken. Stuff like this:main.php:
include 'inc1.inc';
inc1.inc:
@include 'inc2.inc';
If inc2.inc does not exist we now get an error similar to:
Warning: main(./lang/serendipity_lang_.inc.php): failed to open stream: No
such file or directory in /var/www/s9y/serendipity_lang.inc.php on line 5Warning: main(): Failed opening './inc2.inc' for
inclusion (include_path='.:/usr/local/lib/php') on line {the include line
#}Fatal error: Parse error inside included file. in
/var/www/htdocs/inc1.inc on line {the include line #}Remember that it is ok for an include to not find the file. We issue a
warning and move on. It should in no way be treated as a fatal error.-Rasmus
Alle 15:59, venerdì 30 gennaio 2004, Ilia Alshanetsky ha scritto:
+ if (can_open != SUCCESS) {
I didn't try to apply/compile the patch, I just read it, so perhaps I'm
wrong, but... shouldn't that != be a == ?
--
Cesare D'Amico - boss (@t) cesaredamico (dot) com
http://www.cesaredamico.com ~ http://www.phpday.it
The best way to accelerate a Windows machine is at 9.81 m/s^2
It seems like the only way to distinguish between a parse error and a
non-existant file for regular include() is by doing a zend_stream_open() upon
failure to determine if the file is avaliable. If it is, then we return a
parse error and if it does not we continue execution. This does add a small
overhead for failed includes, but IMHO if a non-existant files are being
included performance is not a big consideration.
Granted, I haven't looked closely at the code, but PHP knows that failure
to open an include file is an E_WARNING not an E_ERROR. An E_WARNING
should never cause script execution to terminate. If we already know it
is non-fatal, why is the extra check needed?
-Rasmus
Granted, I haven't looked closely at the code, but PHP knows that failure
to open an include file is anE_WARNINGnot an E_ERROR. AnE_WARNING
should never cause script execution to terminate. If we already know it
is non-fatal, why is the extra check needed?
A parse error is an interesting beast because unlike other fatal errors it is
not handled by PHP's error handler inside main.c. It is not done there since
that would leak the stdio stream opened in ZE. Inside the regular
require/include we have very little data to see what happened, we only know
what op_array is NULL. Whether this happend due to a parse error or a not
found file is unknown. So there are 3 possibilities @ this point:
-> included file was not found
-> included file was found, but has a parse error
-> required file was found, but has a parse error
(if requires was was not found, ZE would've already bailed out).
Since we do not know which of the 3 occured, although I suppose the require
situation can be optimized to:
if (!new_op_array && ... == ZEND_REQUIRE)
For include situation we need to see if the error is due to the file not being
there, in which case we can go forward with execution, or if the problem was
due to a parse error, in which case we need to stop. The easiest way I saw of
doing it (without modifying too much existing code) was to perform a
zend_stream_open(), which would check if the file is avaliable.
Ilia
For include situation we need to see if the error is due to the file not being
there, in which case we can go forward with execution, or if the problem was
due to a parse error, in which case we need to stop. The easiest way I saw of
doing it (without modifying too much existing code) was to perform a
zend_stream_open(), which would check if the file is avaliable.
Since this only happens on an error condition I guess it is ok. Just
seems like we should be able to set some kind of hint in Zend to pass
this information back up.
-Rasmus
Here is the final revision of the patch for the situation, if there are no
last minute objections, it'll go in the CVS and we'll proceed with RC2.
Ilia
For include situation we need to see if the error is due to the file not
being there, in which case we can go forward with execution, or if the
problem was due to a parse error, in which case we need to stop. The
easiest way I saw of doing it (without modifying too much existing code)
was to perform a zend_stream_open(), which would check if the file is
avaliable.Since this only happens on an error condition I guess it is ok. Just
seems like we should be able to set some kind of hint in Zend to pass
this information back up.-Rasmus
Here is the final revision of the patch for the situation, if there are no
last minute objections, it'll go in the CVS and we'll proceed with RC2.
Uh, that is a ZE2 patch. We are talking about PHP 4.3.5 RC2 here.
-Rasmus
Here is the ZE1 revision.
Ilia
Here is the final revision of the patch for the situation, if there are
no last minute objections, it'll go in the CVS and we'll proceed with
RC2.Uh, that is a ZE2 patch. We are talking about PHP 4.3.5 RC2 here.
-Rasmus
zend_error(E_ERROR, "Parse error inside included file.");
Is there any chance to include the name of the included file here?
--
- Martin Martin Jansen
http://martinjansen.com/
The oirignal parse error messages not only tells you the filename but also the
line where the parse error occurs. The 2nd error (the one you referring to)
will tell you on which line the 'broken' file is being included.
Ex:
Parse error: parse error in /home/rei/PHP_CVS/php4/BUILD/inc2.inc on line 3
Fatal error: Parse error inside included file.
in /home/rei/PHP_CVS/php4/BUILD/inc1.php on line 2
Ilia
zend_error(E_ERROR, "Parse error inside included file.");Is there any chance to include the name of the included file here?
> + zend_bool can_open = zend_stream_open(inc_filename->value.str.val, &file_handle TSRMLS_CC);
> + zend_file_handle_dtor(&file_handle);
> +
wouldn't a stat be better suited here?
-sterling
> + /* file open succeeded but still no op-array, likely parse error */
> + if (can_open == SUCCESS) {
> +fatal_error:
> + zend_error(E_ERROR, "Parse error inside included file.");
> + }
> + }
> }
> break;
> case ZEND_EVAL: {
>
> --
>
>
--
"Reductionists like to take things apart. The rest of us are
just trying to get it together."
- Larry Wall, Programming Perl, 3rd Edition
stat wouldn't work for URL includes.
- zend_file_handle file_handle;
- zend_bool can_open = zend_stream_open(inc_filename->value.str.val,
&file_handle TSRMLS_CC);- zend_file_handle_dtor(&file_handle);
wouldn't a stat be better suited here?
I think I missed out on the original problem. What does the fix, fix? Isn't
a parse error automatically a fatal error?
If you revert your patch what won't work?
Andi
At 09:59 AM 1/30/2004 -0500, Ilia Alshanetsky wrote:
It seems like the only way to distinguish between a parse error and a
non-existant file for regular include() is by doing a zend_stream_open() upon
failure to determine if the file is avaliable. If it is, then we return a
parse error and if it does not we continue execution. This does add a small
overhead for failed includes, but IMHO if a non-existant files are being
included performance is not a big consideration.That said, if there is much opposition to the approach I would be happy to
revert the code to the previous state.Ilia
P.S. Suggested 'fix' is attached.
Ilia, I think there is a problem with your latest fixes on the 4_3 branch.
Stuff that used to work is now broken. Stuff like this:main.php:
include 'inc1.inc';
inc1.inc:
@include 'inc2.inc';
If inc2.inc does not exist we now get an error similar to:
Warning: main(./lang/serendipity_lang_.inc.php): failed to open stream: No
such file or directory in /var/www/s9y/serendipity_lang.inc.php on line 5Warning: main(): Failed opening './inc2.inc' for
inclusion (include_path='.:/usr/local/lib/php') on line {the include line
#}Fatal error: Parse error inside included file. in
/var/www/htdocs/inc1.inc on line {the include line #}Remember that it is ok for an include to not find the file. We issue a
warning and move on. It should in no way be treated as a fatal error.-Rasmus
The basic problem is as follows. If you have a parse error inside an included
or required file, the execution stops just for that file and continues for
the main script. The result is that normally a fatal (parse) error becomes a
warning. Consequently, it may result in undefined behavior since whatever
code was inside the included file was not entirely parsed and executed.
Ilia
I think I missed out on the original problem. What does the fix, fix? Isn't
a parse error automatically a fatal error?
If you revert your patch what won't work?
AndiAt 09:59 AM 1/30/2004 -0500, Ilia Alshanetsky wrote:
It seems like the only way to distinguish between a parse error and a
non-existant file for regular include() is by doing a zend_stream_open()
upon failure to determine if the file is avaliable. If it is, then we
return a parse error and if it does not we continue execution. This does
add a small overhead for failed includes, but IMHO if a non-existant
files are being included performance is not a big consideration.That said, if there is much opposition to the approach I would be happy to
revert the code to the previous state.Ilia
P.S. Suggested 'fix' is attached.
Ilia, I think there is a problem with your latest fixes on the 4_3
branch. Stuff that used to work is now broken. Stuff like this:main.php:
include 'inc1.inc';
inc1.inc:
@include 'inc2.inc';
If inc2.inc does not exist we now get an error similar to:
Warning: main(./lang/serendipity_lang_.inc.php): failed to open stream:
No such file or directory in /var/www/s9y/serendipity_lang.inc.php on
line 5Warning: main(): Failed opening './inc2.inc' for
inclusion (include_path='.:/usr/local/lib/php') on line {the include
line #}Fatal error: Parse error inside included file. in
/var/www/htdocs/inc1.inc on line {the include line #}Remember that it is ok for an include to not find the file. We issue a
warning and move on. It should in no way be treated as a fatal error.-Rasmus