Early Magento Session Instantiation is Harmful

This week we’re going to look at a system bug deep in Magento’s core code. I only recently discovered this myself, and will be reviewing and fixing my extensions soon.

In short: If you use a session object too early in Magento’s request lifecycle, you can break session storage.

Magento and PHP Session Cookies

If you take a look at the cookies set in a stock Magento system, you’ll see one named adminhtml, and another named frontend

This assumes you’ve browsed to a frontend page, and to a backend admin page. Theses cookies contain a seemingly incomprehensible string value like

77uvup7bpn6lnjt84ukl0vjmq1
l717sjjoudqehj46sdpl2fnp72

This is your PHP session ID. HTTP is a stateless protocol — the way web systems store state is to set a small cookie in the browser with a unique ID, and then use this unique ID to lookup session values in a separate system. In Magento, this data is stored in either the var/session folder, or the core_session table, (depending on your choices during installation). It’s also possible to store sessions in a separate data store like redis.

The name of the cookie is important. When Magento initializes its sessions, it sets a custom name for the session with the PHP session_name function. This function sets the name of the cookie PHP will use to look for the session_id. Magento sets this with the value for the current “area” (adminhtml or frontend). The important method invocation is here

#File: app/code/core/Mage/Core/Controller/Varien/Action.php
public function preDispatch()
{
    //...
    $session = Mage::getSingleton('core/session', array('name' => $this->_sessionNamespace))->start();
    //...        
}

Notice that Magento instantiates the core/session singleton with a name parameter. This parameter is the area, and set on each concrete controller class.

#File: app/code/core/Mage/Adminhtml/Controller/Action.php
class Mage_Adminhml_Controller_Action extends Mage_Core_Controller_Varien_Action
{
    //...    
    const SESSION_NAMESPACE = 'adminhtml';
    //...
    protected $_sessionNamespace = self::SESSION_NAMESPACE
    //...
}

#File: app/code/core/Mage/Core/Controller/Front/Action.php
class Mage_Core_Controller_Front_Action extends Mage_Core_Controller_Varien_Action    
{
    //...    
    const SESSION_NAMESPACE = 'frontend';
    //...
    protected $_sessionNamespace = self::SESSION_NAMESPACE
    //...
}

The _sessionNamespace value is eventually used in the call to session_name

#File: app/code/core/Mage/Core/Model/Session/Abstract/Varien.php    
//...
public function start($sessionName=null)
{
    if (!empty($sessionName)) {
        $this->setSessionName($sessionName);
    }
}
//...
public function setSessionName($name)
{
    session_name($name);
    return $this;
}

So, by creating an abstraction layer on top of PHP’s session system, Magento allows different session values to be used and set for the different Magento application areas. The backend admin uses a different set of session variables from the frontend.

While the design and intent of this system is solid, there is a flaw in its implementation.

Early Session Instantiation

Magento doesn’t know which area it’s handling until the preDispatch method. Therefore, Magento can’t instantiate its session object until the preDispatch method. However, there are Magento system events that fire before pre-dispatch. If a Magento client programmer attempts to use a session in one of these events, some weird stuff can happen.

Consider this module. It sets up an observer method

<controller_front_init_before>
    <observers>
        <sessionareabug_example>
            <type>singleton</type>
            <class>Pulsestorm_Sessionareabug_Model_Observer</class>
            <method>earlySession</method>
        </sessionareabug_example>
    </observers>
</controller_front_init_before>

and this observer method instantiates a core/session object.

public function earlySession($observer)
{
    if(strpos(implode($_SERVER), 'pulsestorm_sessionareabug/index/special') !== false)
    {
        Mage::getSingleton('core/session');
    }
}

If you load the pulsestorm_sessionareabug/index/special URL (i.e. instantiate a session before preDispatch) in your system and take a look at your cookie values

You’ll see a cookie named PHPSESSID. When Magento’s session code can’t find an area name, the session_name function never gets called, and PHP is reading from the default session.

This is clearly a bug.

Consequences

The consequences of this bug aren’t necessarily disastrous — but they are subtle, and are likely the root cause of many “the behavior only happens intermittently” problems reported with Magento systems.

If you’re instantiating the session object early on all requests, this means your backend admin and frontend cart are reading from the same session. If one area attempts to set a session variable with the same name and namespace, the older value will be clobbered.

Equally frustrating is when sessions are instantiated early, and conditionally. When this happens, sometimes the session ID will be PHPSESSID. Other times it will be named for the area. The end result will look like Magento hasn’t saved a session value because it’s reading or writing to the wrong session.

A real world example? If you’ve ever installed an extension and had to clear sessions to log into the admin, there’s a good chance this bug is to blame.

Fixes

Unfortunately, there’s no clean fix here. If you’re an extension author, you should avoid using sessions in events fired prior to Magento instantiating the first core/session singleton — or if you do you’ll need to ensure a proper area value is set when you’re instantiating the session.

The obvious events to avoid are

controller_front_init_before
controller_front_init_routers
adminhtml_controller_action_predispatch_start

These are the one time events that fire before Magento calls the preDispatch method. However, you also need to be careful in the following events.

resource_get_tablename
core_collection_abstract_load_before
core_collection_abstract_load_after
model_load_after
core_abstract_load_after

These events fire numerous times during a Magento page request, and fire both before and after the preDispatch method. Be cautious using sessions in these event observers — I recommend explicitly checking that the system sessions have started before continuing with your observer code

public function myObserverMethod($observer)
{
    if(!session_id())
    {
        return;
    }

    $session = Mage::getModel('core/session');
}

Unfortunately, if you’re a Magento system owner (merchant, marketer, etc.) — there’s not a good general solution for you other than “stop using that extension”. If you suspect your system is having trouble related to this bug, dropping in a temporary code pool override for the Mage_Core_Model_Session_Abstract_Varien class with the following additional logging

#File: app/code/local/Mage/Core/Model/Session/Abstract/Varien.php
//...
public function start($sessionName=null)
{
    Mage::Log('Starting session without name', Zend_Log::WARN);
    //other logging to pinpiont problematic requests
    //...rest of original method here...
}    

will let you track this down. A code pool override is necessary because a rewrite based solution would require you to rewrite every session object in the system — including unknown session objects from custom modules.

However, ensuring the session always has a proper name/area will be dependent on circumstances individual to each system. This is a classic case of “The application is relying on broken system functionality” where fixing the system may break the application.

I hope eBay will address this in Magento 2, but it seems like something we’ll be stuck with in Magento 1 through its end of life period.