View Issue Details

IDProjectCategoryView StatusLast Update
0004262OXID eShop (all versions)4.07. Source code, Testpublic2013-05-29 09:55
ReporterWBL_BjoernLange Assigned To 
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Target Version4.8.0_5.1.0_beta1Fixed in Version4.7.6 / 5.0.6 
Summary0004262: Change in oxSession::getBasket leads to sideeffect in oxutilsobject::_makeSafeModuleClassParents
DescriptionYou changed

// 4.6.0
//0001746
            oxNew('oxbasket');
            oxNew('oxbasketitem');

            $oBasket = ( $sBasket && ( $oBasket = unserialize( $sBasket ) ) ) ? $oBasket : null;

            //0003908
            if ( !$oBasket || !($oBasket instanceof oxbasket)) {
                $oBasket = oxNew('oxbasket');
            }


to

            oxNew('oxbasketitem');

            $oBasket = ( $sBasket && ( $oBasket = unserialize( $sBasket ) ) ) ? $oBasket : null;

            //0003908
            $oEmptyBasket = oxNew('oxbasket');
            if ( !$oBasket || ( get_class($oBasket) !== get_class($oEmptyBasket) ) ) {
                $oBasket = $oEmptyBasket;
            }


this leads to different call orders.
OXID 4.6.0 loads the basket modules through oxNew first.
OXID 4.6.2 loads the basket module through the autoloader first and leads to oxutilsobject::_disableModule-Call in some ways. For example, if you use advanced techniques like a special autoloader for loading your basket module, the module file tries to call oxbasket_parent, which leads to a nested call to oxAutoload for oxbasket. And this nested call trys to load the basket module again

//including original file
                if ( file_exists( $sParentPath ) ) {
                    include_once $sParentPath;
                } elseif ( !class_exists( $sModuleClass ) ) { // <-- here


which will fail, because this nested call is not bubbling back to the special autoloader, which was loaded through the functions.php before oxAutoload was registered.


I would say

         
            oxNew('oxbasketitem');
            $oEmptyBasket = oxNew('oxbasket');

            $oBasket = ( $sBasket && ( $oBasket = unserialize( $sBasket ) ) ) ? $oBasket : null;

            //0003908
            //$oEmptyBasket = oxNew('oxbasket');
            if ( !$oBasket || ( get_class($oBasket) !== get_class($oEmptyBasket) ) ) {
                $oBasket = $oEmptyBasket;
            }


does the same, but is a much better approach, because the oxid framework has the chance to instantiate the basket module like every other class in the oxid framework through oxNew first, and not through an autoloader.
TagsBasket, Session
ThemeBoth
BrowserAll
PHP Versionany
Database Versionany

Activities

WBL_BjoernLange

2012-07-17 00:33

reporter   ~0007103

Last edited: 2012-10-03 21:59

And the call "$oEmptyBasket = oxNew('oxbasket');" from oxid 4.6.2 after the unserializing works not "correctly", if the unserializing leads to an autoloader-call, which disables a module even when it exists. And so "get_class($oBasket) !== get_class($oEmptyBasket)" got a small flaw.

Update: From the quote above,
//0001746
oxNew('oxbasketitem');
            $oEmptyBasket = oxNew('oxbasket'); // <--- 

was done by myself as a core fix but even in the original form of 4.6 the bug was there.

WBL_BjoernLange

2012-10-03 22:15

reporter   ~0007545

Is there something new for this ticket?

Linas Kukulskis

2013-05-29 09:55

reporter   ~0008728

resolved with contribution