# Warnings in include files suddenly treated as fatal - Thread: https://externals.io/message/7415 - Original archive: https://news-web.php.net/php.internals/7415 - Started by Rasmus Lerdorf on 2004-01-30T10:29:25+00:00 - Messages: 18 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #7415 — Rasmus Lerdorf — 2004-01-30T10:29:25+00:00 URL: https://externals.io/message/7415#7415 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 --- ## #7416 — Derick Rethans — 2004-01-30T10:40:21+00:00 URL: https://externals.io/message/7415#7416 · In reply to: #7415 > 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 --- ## #7417 — Rasmus Lerdorf — 2004-01-30T10:41:13+00:00 URL: https://externals.io/message/7415#7417 · In reply to: #7416 > > > > […] > > But it should on a parse error in an include file, not? Yes, but a failed include is not a parse error. -Rasmus --- ## #7418 — Derick Rethans — 2004-01-30T11:05:14+00:00 URL: https://externals.io/message/7415#7418 · In reply to: #7417 > > > > […] > > Yes, but a failed include is not a parse error. I know, I was just making sure while fixing it :) Derick --- ## #7422 — Ilia Alshanetsky — 2004-01-30T14:59:41+00:00 URL: https://externals.io/message/7415#7422 · In reply to: #7415 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 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 --- ## #7423 — Cesare D'Amico — 2004-01-30T15:22:50+00:00 URL: https://externals.io/message/7415#7423 · In reply to: #7422 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 --- ## #7427 — Rasmus Lerdorf — 2004-01-30T17:03:41+00:00 URL: https://externals.io/message/7415#7427 · In reply to: #7422 > 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 --- ## #7428 — Ilia Alshanetsky — 2004-01-30T17:35:01+00:00 URL: https://externals.io/message/7415#7428 · In reply to: #7427 > 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? 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 --- ## #7429 — Rasmus Lerdorf — 2004-01-30T17:37:24+00:00 URL: https://externals.io/message/7415#7429 · In reply to: #7428 > 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 --- ## #7430 — Ilia Alshanetsky — 2004-01-30T18:17:43+00:00 URL: https://externals.io/message/7415#7430 · In reply to: #7429 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 > > > […] > > 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 --- ## #7431 — Rasmus Lerdorf — 2004-01-30T18:21:29+00:00 URL: https://externals.io/message/7415#7431 · In reply to: #7430 > 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 --- ## #7432 — Ilia Alshanetsky — 2004-01-30T18:37:52+00:00 URL: https://externals.io/message/7415#7432 · In reply to: #7431 Here is the ZE1 revision. Ilia > > > […] > > Uh, that is a ZE2 patch. We are talking about PHP 4.3.5 RC2 here. > > -Rasmus --- ## #7433 — Sterling Hughes — 2004-01-30T18:43:32+00:00 URL: https://externals.io/message/7415#7433 · In reply to: #7430 > + 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? -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 --- ## #7434 — Wez Furlong — 2004-01-30T18:45:44+00:00 URL: https://externals.io/message/7415#7434 · In reply to: #7433 stat wouldn't work for URL includes. > > […] &file_handle TSRMLS_CC); > > […] > > wouldn't a stat be better suited here? --- ## #7435 — Martin Jansen — 2004-01-30T19:14:02+00:00 URL: https://externals.io/message/7415#7435 · In reply to: #7432 > + 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/ --- ## #7436 — Ilia Alshanetsky — 2004-01-30T19:17:02+00:00 URL: https://externals.io/message/7415#7436 · In reply to: #7435 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 > > > […] > > Is there any chance to include the name of the included file here? --- ## #7439 — Andi Gutmans — 2004-01-31T21:17:09+00:00 URL: https://externals.io/message/7415#7439 · In reply to: #7415 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. > > > > […] --- ## #7446 — Ilia Alshanetsky — 2004-02-01T07:04:50+00:00 URL: https://externals.io/message/7415#7446 · In reply to: #7439 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? > Andi > > At 09:59 AM 1/30/2004 -0500, Ilia Alshanetsky wrote: > > […]