Re: Silenced include(_once) calls

On 3/2/06, Alan Knowles <alan@akbkhome.com> wrote: > I think this also need to be balanced against the concept, > a) is it a setup/programming error > b) is it a result of user-input > > If b) is likely, then this approach is correct, > if a) is likely, then just using include_once, and displaying the error > is probably more usefull. > As long as you're saying that using include_once should be used without @ that's fine. My point is that @ is never to be used with include (or require). I don't really care what people do otherwise, I was just offering solution. > Regards > Alan > > > Justin Patrin wrote: > > >A user reported a problem with DB_DataObject_FormBuilder today that > >caused me to go back and look at the factory code in the create() > >function. Just yesterday I checked a new factory method into Text_Wiki > >so I had a fresh perspective on the factory algorithm. I traced the > >user's problem to a silenced include_once that I was erroneously using > >to check for file existance as well as include a file. > > > >Since I had just fixed this I decided to see how many other files in > >PEAR silence include calls. I was quite shocked to find that there are > >over 100 silenced include calls in PEAR code. > >http://pear.reversefold.com/include_once_silenced.txt > > > >I would like to make the case for this to be disallowed from PEAR code > >as part of the PEAR CS. > > > >The problem is that include_once has side effects and silencing the > >call causes lots of lots development time when things happen to go > >wrong. Consider the following error cases: > > > >* The file to include cannot be found. > >This is generally the case that these silenced calls are meant to deal > >with and it deals with it fine. The call returns false and the code > >can raise an error. > > > >* The include file is found but has a parse error > >The script dies on the spot without any output. This is strange as > >normally PHP will output when a parse error happens. This causes a > >major problem as the developer has to trace into all the code to find > >out what the problem is. Once it is found that the include_once is > >where it dies, removing the @ gives the developer the real error. > > > >* The file to include exists and has a require or require_once which > >cannot be found > >See above. The script will die but not output anything. This also > >applies recursively as required scripts could in turn require other > >scripts. > > > >* The file to include exists and has an include or require which > >includes a script with parse error > >Again, see above. The script will die but not output anything. Also a > >recursive problem. > > > >* The file to include parses fine but has a "return false;" or similar > >at the end > >include returns false even though the code was included, parsed, and > >run fine. This is not likely to ever be the case in actual PEAR code, > >but it could be the case in some maliciously crafted code or in > >someone's badly written code for their own use. > > > >It should be clear from these cases that checking for the return value > >of include is not a correct check for the includability of a file. In > >addition, it has many bad side-effects which cause developers to lose > >time debugging when they shouldn't have to. > > > >There are several alternatives to silencing include: > >1) Leave it as-is but without @. This will cause PHP to display errors > >when files can't be found, but this is in general something that the > >developer wanted to see anyway. Remember that production websites > >should normally have error_reporting turned off and output to a file > >so that the user doesn't see it. > > > >2) Split the include_path and check file_exists and is_readable for > >the file for each path. This is a solution which adds some extra > >cycles to the include process but allows the application to gracefully > >return an error when a file is not includable without a PHP error > >getting triggered. > > > >3) Use @fopen($file, 'r', true). This is faster than #2 and still > >checks the include_path. This also does a check for readability in one > >call. This does use @, but in this case it has no horrible > >side-effects, such as the script dying. There are a few side-effects > >in that a registered PHP error handler will catch errors from this, > >but that is not likely to hurt an application in any way. This > >solution also allows an application to gracefully return an error > >without PHP errors being displayed. > > > >-- > >Justin Patrin > > > > > > > > -- > PEAR Development Mailing List (http://pear.php.net/) > To unsubscribe, visit: http://www.php.net/unsub.php > > -- Justin Patrin