On February 01, 2004 12:30 pm, Andi Gutmans wrote:
> I understand from the comments here that your patch wasn't very successful.

Original patch was incomplete, therefor I made 2 seperate patches (1 for php5 
& 1 for php4) that augment the behaviour and fix the problem originally 
reported by Rasmus.

> Any chance you can:
> a) Revert the patch.
> b) Send me a short reproducing example of the combination which doesn't
> currently work? I tried to include a file which has a parse error and it
> did stop execution right away (which it should). I guess I didn't quite
> understand which combination doesn't work.

main.php
<?php
include("a.php");
echo "Hello World\n";
?>

a.php
<?php
$a = '
?>

The old behaviour would print the parse error message, BUT also the "Hello 
World" string indicating the execution of the main script was not stopped. My 
argument, which others seem to share is that it should've stopped. If you 
disagree with this, then I'll revert the patch, otherwise the additions (see 
attached) should be applied, which would properly handle file not found 
situation of include().


Ilia

>
> Thanks,
>
> Andi
>
> At 02:04 AM 2/1/2004 -0500, Ilia Alshanetsky wrote:
> >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
> >
> >On January 31, 2004 04:17 pm, you wrote:
> > > 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.
> > > >
> > > >On January 30, 2004 05:29 am, Rasmus Lerdorf wrote:
> > > > > 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
> > > >
> > > >--
> > > >PHP Internals - PHP Runtime Development Mailing List
> > > >To unsubscribe, visit: http://www.php.net/unsub.php
Index: zend_execute.c
===================================================================
RCS file: /repository/Zend/Attic/zend_execute.c,v
retrieving revision 1.316.2.28
diff -u -3 -p -r1.316.2.28 zend_execute.c
--- zend_execute.c      30 Jan 2004 02:22:31 -0000      1.316.2.28
+++ zend_execute.c      30 Jan 2004 18:37:21 -0000
@@ -2152,7 +2152,24 @@ send_by_ref:
                                                case ZEND_REQUIRE:
                                                        new_op_array = 
compile_filename(EX(opline)->op2.u.constant.value.lval, inc_filename TSRMLS_CC);
                                                        if (!new_op_array) {
-                                                               zend_error(E_ERROR, 
"Parse error inside included file.");
+                                                               /* small optimization 
for require() */
+                                                               if (EX(opline)->opcode 
== ZEND_REQUIRE) {
+                                                                       goto 
fatal_error;
+                                                               } else {
+                                                                       /* This check 
is needed to ensure that included file has a parse error 
+                                                                        * and we are 
no dealing with a non-existant include, which is not considered
+                                                                        * to be a 
fatal error.
+                                                                        */
+                                                                       
zend_file_handle file_handle = {0};
+                                                                       int can_open = 
(zend_open(inc_filename->value.str.val, &file_handle) == SUCCESS && 
ZEND_IS_VALID_FILE_HANDLE(&file_handle));
+                                                                       
zend_file_handle_dtor(&file_handle);
+
+                                                                       /* file open 
succeeded but still no op-array, likely parse error */
+                                                                       if (can_open) {
+fatal_error:
+                                                                               
zend_error(E_ERROR, "Parse error inside included file.");
+                                                                       }
+                                                               }
                                                        }
                                                        break;
                                                case ZEND_EVAL: {
Index: zend_execute.c
===================================================================
RCS file: /repository/ZendEngine2/zend_execute.c,v
retrieving revision 1.592
diff -u -3 -p -r1.592 zend_execute.c
--- zend_execute.c      30 Jan 2004 02:22:17 -0000      1.592
+++ zend_execute.c      30 Jan 2004 18:16:32 -0000
@@ -3390,7 +3390,24 @@ int zend_include_or_eval_handler(ZEND_OP
                case ZEND_REQUIRE:
                        new_op_array = 
compile_filename(EX(opline)->op2.u.constant.value.lval, inc_filename TSRMLS_CC);
                        if (!new_op_array) {
-                               zend_error(E_ERROR, "Parse error inside included 
file.");
+                               /* small optimization for require() */
+                               if (EX(opline)->op2.u.constant.value.lval == 
ZEND_REQUIRE) {
+                                       goto fatal_error;
+                               } else {
+                                       /* This check is needed to ensure that 
included file has a parse error 
+                                        * and we are no dealing with a non-existant 
include, which is not considered
+                                        * to be a fatal error.
+                                        */
+                                       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);
+
+                                       /* 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: {

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to