Zend Framework

Bug in PluginLoader breaks two Zend_View UnitTests

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.7 Preview Release
  • Fix Version/s: 1.10.2
  • Component/s: Zend_Loader
  • Labels:
    None
  • Fix Version Priority:
    Must Have

Description

The two view tests that it breaks are

ViewTests.php -> testGetHelperPath() and testGetFilterPath()

The problem comes in with this code in the PluginLoader load() Method

if (class_exists($className, false)) {
    $found = true;
    break;
}

With the break it takes it out of the loop and never creates the $loadFile variable which sets the path to the class.

If you comment out this if statement everything is fine..

Attachments

  1. Text File
    PluginLoader.php.patch
    23/Oct/08 7:27 PM
    2 kB
    Jon Whitcraft
  2. Text File
    zf4697.r21037.patch
    12/Feb/10 10:40 AM
    2 kB
    Michael Rehbein

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Jon Whitcraft added a comment -

Attached is the patch to fix this. If you want me to fix it then I will.

Show
Jon Whitcraft added a comment - Attached is the patch to fix this. If you want me to fix it then I will.
Hide
Permalink
Alexander Veremyev added a comment -

BTW It produces one unit tests error and one unit tests failure now:

8) testGetHelperPath(Zend_ViewTest)
strpos(): Empty delimiter

12) testGetFilterPath(Zend_ViewTest)
Failed asserting that two strings are equal.
expected string </home/cawa/ZendFramework/svn/framework/trunk/tests/Zend/View/_stubs/FilterDir1/Foo.php>
difference <??????????????????????????????????????????????????????????????????????????????????????>
got string <>

Show
Alexander Veremyev added a comment - BTW It produces one unit tests error and one unit tests failure now:
8) testGetHelperPath(Zend_ViewTest) strpos(): Empty delimiter
12) testGetFilterPath(Zend_ViewTest) Failed asserting that two strings are equal. expected string </home/cawa/ZendFramework/svn/framework/trunk/tests/Zend/View/_stubs/FilterDir1/Foo.php> difference <??????????????????????????????????????????????????????????????????????????????????????> got string <>
Hide
Permalink
Matthew Weier O'Phinney added a comment -

This appears to be resolved in current trunk.

Show
Matthew Weier O'Phinney added a comment - This appears to be resolved in current trunk.
Hide
Permalink
Jon Whitcraft added a comment -

OK I found how this bug is being produced.

If you call a helper by doing $mhc = new My_Helpers_Custom() before calling it from the helperBroker it produces the error.

I'm going to setup a test page and submit it for testing.

Show
Jon Whitcraft added a comment - OK I found how this bug is being produced. If you call a helper by doing $mhc = new My_Helpers_Custom() before calling it from the helperBroker it produces the error. I'm going to setup a test page and submit it for testing.
Hide
Permalink
Jon Whitcraft added a comment -

Update to remove the Fix Version since this is not fixed.

Show
Jon Whitcraft added a comment - Update to remove the Fix Version since this is not fixed.
Hide
Permalink
Matthew Weier O'Phinney added a comment -

All unit tests for Zend_View are running fine currently. What OS, PHP version, and ZF version are you seeing the issue on?

Show
Matthew Weier O'Phinney added a comment - All unit tests for Zend_View are running fine currently. What OS, PHP version, and ZF version are you seeing the issue on?
Hide
Permalink
Jon Whitcraft added a comment -

Matthew,

This issue was submitted almost a year ago and I don't think it's valid any more. I forget what version of PHP I ran this on but it was for the 1.7.0 PR release.

I think this can be closed an a Non-Issue.

Show
Jon Whitcraft added a comment - Matthew, This issue was submitted almost a year ago and I don't think it's valid any more. I forget what version of PHP I ran this on but it was for the 1.7.0 PR release. I think this can be closed an a Non-Issue.
Hide
Permalink
Michael Rehbein added a comment -

Found a test case that shows the bug.
Also created a patch to correct it.
Will attach patch next.

Show
Michael Rehbein added a comment - Found a test case that shows the bug. Also created a patch to correct it. Will attach patch next.
Hide
Permalink
Michael Rehbein added a comment -

Patch:

  • adds a unit test for this issue
  • uses reflection to get which path the class was loaded from if it is already loaded
Show
Michael Rehbein added a comment - Patch:
  • adds a unit test for this issue
  • uses reflection to get which path the class was loaded from if it is already loaded
Hide
Permalink
Matthew Weier O'Phinney added a comment -

I'm reluctant to use this patch, as Reflection introduces substantial overhead – and with something used as widely as the PluginLoader, this will have a lot of impact. In looking at your test case and patch, I discovered that we already have the code you describe: in getClassPath(). Since this is a method only used on-demand (not in every request), it makes sense to simply leverage this. Once I looked at that, I realized that I found the flaw: it was in how the $loadFile was detected and cached. I simply removed the logic to cache that information, and subsequently all tests, including your new one, pass.

Show
Matthew Weier O'Phinney added a comment - I'm reluctant to use this patch, as Reflection introduces substantial overhead – and with something used as widely as the PluginLoader, this will have a lot of impact. In looking at your test case and patch, I discovered that we already have the code you describe: in getClassPath(). Since this is a method only used on-demand (not in every request), it makes sense to simply leverage this. Once I looked at that, I realized that I found the flaw: it was in how the $loadFile was detected and cached. I simply removed the logic to cache that information, and subsequently all tests, including your new one, pass.
Hide
Permalink
Matthew Weier O'Phinney added a comment -

Patch applied (with revisions) to trunk and 1.10 release branch.

Show
Matthew Weier O'Phinney added a comment - Patch applied (with revisions) to trunk and 1.10 release branch.
Hide
Permalink
Hendri Smit added a comment -

The changes made to the PluginLoader make my application crash Apache.
I can't really point my finger on it but if add a simple isset() there is no problem :S

Zend_Loader_PluginLoader
...
        if ($this->_useStaticRegistry) {
            self::$_staticLoadedPlugins[$this->_useStaticRegistry][$name]     = $className;
            isset($className);
        } else {
            $this->_loadedPlugins[$name]     = $className;
        }
...

Any thoughts?

Show
Hendri Smit added a comment - The changes made to the PluginLoader make my application crash Apache. I can't really point my finger on it but if add a simple isset() there is no problem :S
Zend_Loader_PluginLoader
...
        if ($this->_useStaticRegistry) {
            self::$_staticLoadedPlugins[$this->_useStaticRegistry][$name]     = $className;
            isset($className);
        } else {
            $this->_loadedPlugins[$name]     = $className;
        }
...
Any thoughts?
Hide
Permalink
Matthew Weier O'Phinney added a comment -

@Hendri Actually, no thoughts. The lines you quote were never changed. I actually simply modified the load() method not to store the class filename; instead, it's only retrieved on-demand when you use getClassFile().

Can you provide a reproduce case that demonstrates the behavior? The ZF site itself is using 1.10.2, and we're not experiencing any crashes at this time.

Show
Matthew Weier O'Phinney added a comment - @Hendri Actually, no thoughts. The lines you quote were never changed. I actually simply modified the load() method not to store the class filename; instead, it's only retrieved on-demand when you use getClassFile(). Can you provide a reproduce case that demonstrates the behavior? The ZF site itself is using 1.10.2, and we're not experiencing any crashes at this time.
Hide
Permalink
Hendri Smit added a comment -

@Matthew There was probably something wrong with my Apache installation

Show
Hendri Smit added a comment - @Matthew There was probably something wrong with my Apache installation

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Zend Framework. Try JIRA - bug tracking software for your team.