Zend Framework

Can you tell me whether or not we also should fix Zend_Cache::_isReadable ?
It seems Zend_Cache has same problem.

I would say so, since it is basically the same code.

I think ZF-2701 has same trouble like as this.

Resolved in SVN r2925

Reopening, since SVN r9295 should be reverted because fopen() is directed to utilize the include_path, whereas file_exists() does not use the include_path.

Ya, r9295 breaks the PluginLoader: exception 'Zend_Loader_PluginLoader_Exception' with message 'Plugin by name Word_CamelCaseToDash was not found in the registry.'

Please, revert and see ZF-3170 as well

I have some more Zend_Loader tests to commit as well

Thank you for mailing me and reverting, Darby.

testing new code now

I think the function will be as following.
Today, I tested with Zend_Loader_PluginLoader.
And I will try to test with other components.
I will be happy If you tell me any ideas

public static function isReadable($filename)
{
    if (is_readable($filename)) {
        return true;
    }

    $paths = explode(PATH_SEPARATOR, get_include_path());

    foreach ($paths as $path) {
        if (is_readable($path . DIRECTORY_SEPARATOR . $filename)) {
            return true;
        }
    }

    return false;
}

I believe that SplFileObject supports opening a file in the include path and it throws an exception (that can be caught if the file is not found.

Ｆｉｘｅｄ in SVN r9451. In SVN r6780, fopen method was used instead of is_readable method. But it causes some problems. So, I fix by using is_readable.

To:Emil Ivanov
Thanks your comment. But Unfortunately, I can not use filename in openFile method of SplFileObject.

Pardon me if I'm posting in the wrong place, but after updating to SVN r9451 I noticed breakage related to changes in Loader.php's isReadable() method.

Symptoms: My rendered views no longer worked as if the files template files were not being read. (Note, I use a Smarty/ViewRenderer solution)

I made a patch in my local copy by placing your suggested is_readable($filename) check above the directory path traversing.

I see you had posted on May 7th the inclusion of the is_readable($filename), I believe it should be added for consideration.

<code>
public static function isReadable($filename)
{
// If the directory methods above don't work, try the filename directly
if (is_readable($filename)) { return true; }

$dirs = explode(PATH_SEPARATOR, get_include_path());
$dirs = array_merge($dirs, array('.'));

foreach ($dirs as $dir) {
if (is_readable($dir . DIRECTORY_SEPARATOR . $filename)) { return true; }
}

return false;
}
</code>

Thanks,

Hello, Basil. You means is_readable( "./$filename") does not work?
If so, I will add is_readable($filename) and remove array_merge($dirs, array('.')).

Satoru Yoshida: "Thanks your comment. But Unfortunately, I can not use filename in openFile method of SplFileObject."

Do not use openFile, use $f = new SplFileObject($filename, $mode, $useIncludePath);

Correct Satoru, the is_readable(./$filename) does not work. Adding the is_readable($filename) and removing array_merge($dirs, array('.')) would be good. Thanks!

Thank you for advice and responces, Emil Ivanov and Basil Guevarra .

Buy the way I hear ZF Dev Team members works on this matter.
I will wait their to expect good results.

I saw the same problem too, just like the other problem in a previous topic.

I think isReadable should also search in the default root, not using an includepath, so I did as follows:

/**
     * Returns TRUE if the $filename is readable, or FALSE otherwise.
     * This function uses the PHP include_path, where PHP's is_readable()
     * does not.
     *
     * @param string   $filename
     * @return boolean
     */
    public static function isReadable($filename)
    {
    	$arrPaths = explode(PATH_SEPARATOR, get_include_path() );
    	
    	$blnReturn = false;
    	
    	if(  file_exists( $filename ) &&  $fh = @fopen( $filename, 'r') ){
    		@fclose($fh);
    		$blnReturn = true;
    	}else{
	    	
    		if( substr($filename, 0, 2) === './' ) $filename = substr($filename, 2);
	    	
    		foreach($arrPaths as $path){
	    		if( substr($path, -1, 1) !== '/' && substr($path, -1, 1) !== '\\'  ) $path .= '/';
	    		
	    		if ( file_exists( $path . $filename ) &&  $fh = @fopen($path . $filename, 'r')) {
		            @fclose($fh);
	    			$blnReturn = true;
		        }
	    		
	    	}
    	}
    	
    	

        return $blnReturn;
    }

Two notes: First, any solution that involves looping over paths in userland code will be rejected. These solutions have a tremendous performance impact, which is why the fopen() solution was chosen (the third argument to this function allows it search on the include_path – but doing so in C is magnitudes faster than doing so in userland PHP).

Second, the fopen() call also has the error suppression operator; you will only see these errors in your error log. This is acceptable when it comes to E_STRICT standards – the spirit of this rule refers to the messages that will be displayed when display_errors is on. This is one reason why error suppression is discouraged – it should be used only for very, very good reasons. This is one such case, to our thinking.

Hi, Matthew.

I thank you that you explain why fopen() should be selected than other ideas.

I think this issue should be closed as 'Won' t fix' , do you think?

Hi, All.

I wondered why fopen() with at mark happens.
Because I did not find any warning in my tests with ver.1.6.1, 1.6.2, 1.5.3 and etc.

I understand what causes the warning, by talking about Simon on ZF-2900.
I recommend to look at that issue to solve this issue.

"First, any solution that involves looping over paths in userland code will be rejected. These solutions have a tremendous performance impact, which is why the fopen() solution was chosen"

According to my tests, your fopen-based method is always 5-30% slower than my "looping over paths in userland code" method.

Here's my test code:
first add the new method to Zend/Loader.php:
public static function _isReadable($filename)
{
if (is_readable($filename)) return true;
foreach (explode(PATH_SEPARATOR, get_include_path()) as $path)

return false;
}

Now run test:
<?php
set_include_path(implode(PATH_SEPARATOR, array('/path1', '/path2', '/path3', '/path4', '/path5', '/path6')) . PATH_SEPARATOR . get_include_path());

require_once '/ZF/library/Zend/Loader.php';

$start = microtime(true);
for ($i = 0, $count = 1000; $i < $count; $i++)
{
Zend_Loader::_isReadable("nonexistingFile$i");
Zend_Loader::_isReadable('Var_Dump.php');
}
$stop = microtime(true);
echo 'Using new method: ' . ($stop - $start) . ' seconds<br />';

$start = microtime(true);
for ($i = 0, $count = 1000; $i < $count; $i++)
{
Zend_Loader::isReadable("nonexistingFile$i");
Zend_Loader::isReadable('Var_Dump.php');
}
$stop = microtime(true);
echo 'Using old fopen-method: ' . ($stop - $start) . ' seconds<br />';

?>

Var_Dump.php is part of PEAR, and was just added to test for existing files in include_path (assuming PEAR path is part of include_path).

You could try commenting out the call to set_include_path(), or calls to both methods using nonexisting file or existing file, just to check that the resulting times are always similar when given similar input. The two methods also provide the same actual results, of course.

Could you please change this now, or do you need more proof?

Sorry, there was a bug in my method - empty input would return true, as is_readable would only check against the directories in include_path.
Adding the following line to the beginning of the method fixes the problem.
if ($filename === null || $filename === false || $filename === '' || !is_scalar($filename)) return false;

This change will not effect performance (noticeably anyway).

Reopening as requested in a comment in ZF-2900 , I'm curious...

Actually, I asked that this issue was un-duplicated, ie. remove the duplicate tag for this issue. I also mentioned that I would post a new bug instead, implying that re-opening this one would not be necessary.

You may close this again unless there were other reasons than my comment in ZF-2900.

If you think it is not duplicated issue, I would be out of ability to close this.
change into unassign.

Removed link to ZF-2900.

Closed issue again as the mentioned code is not reproducable and already fixed within the actual release.