Bug behind before/after="Mage_Adminhtml" in Magento

Magento Standard Router BugSince Magento CE 1.3.0 version, creating Magento admin controller required you to specify before="Mage_Adminhtml" or after="Mage_Adminhtml" attribute for your node at admin/routers/adminhtml/args/modules XML Path, depending on type of behavior you want to achieve. Like me, you probably have doubts on why this is necessary, and what is the background of this requirement? Simple answer would be that without it, it simply doesn't work. More precisely, Magento hits you right in the schnoz with 404 after you try accessing admin controller in question, if you don't set one of these attributes at your node. In this article I'll do my best to explain bug in Magento standard router causing this behavior, one that somehow managed to get packaged with last 25+ Magento CE versions without being noticed.

Yesterday my team came to conclusion that Enterprise_Rma module contains a bug, because code snippet registering it's admin controller has missing before/after attribute at relevant node in config.xml, what causes 404 in Magento admin. Here's an excerpt from config.xml:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
<!-- app/code/core/Enterprise/Rma/etc/config.xml -->
<config>
 
    <!-- Other XML -->
 
    <admin>
        <routers>
            <adminhtml>
                <args>
                    <modules>
                        <enterprise_rma>Enterprise_Rma_Adminhtml</enterprise_rma>
                    </modules>
                </args>
            </adminhtml>
        </routers>
    </admin>
 
    <!-- Other XML -->
 
</config>

This was rather logical conclusion, because after changing previous code snippet to something like following, Magento Enterprise_Rma admin controllers worked flawlessly:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
<!-- app/code/core/Enterprise/Rma/etc/config.xml -->
<config>
 
    <!-- Other XML -->
 
    <admin>
        <routers>
            <adminhtml>
                <args>
                    <modules>
                        <enterprise_rma before="Mage_Adminhtml">Enterprise_Rma_Adminhtml</enterprise_rma>
                    </modules>
                </args>
            </adminhtml>
        </routers>
    </admin>
 
    <!-- Other XML -->
 
</config>

The only thing special about Magento admin controllers is that they share single front name owned by Mage_Adminhtml and named "admin". Under condition that controller name is unique, it seemed natural for this to work correctly without the before or after attributes added, even though I knew it never did. I've taken a quick look at the code in Magento standard router (admin router simply extends standard router a little), specifically code in charge of collecting routes as outlined in the following code snippet:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
<?php
 
class Mage_Core_Controller_Varien_Router_Standard extends Mage_Core_Controller_Varien_Router_Abstract
{
 
    // Other methods
 
    public function collectRoutes($configArea, $useRouterName)
    {
        $routers = array();
        $routersConfigNode = Mage::getConfig()->getNode($configArea.'/routers');
        if($routersConfigNode) {
            $routers = $routersConfigNode->children();
        }
        foreach ($routers as $routerName=>$routerConfig) {
            $use = (string)$routerConfig->use;
            if ($use == $useRouterName) {
                $modules = array((string)$routerConfig->args->module);
                if ($routerConfig->args->modules) {
                    foreach ($routerConfig->args->modules->children() as $customModule) {
                        if ($customModule) {
                            if ($before = $customModule->getAttribute('before')) {
                                $position = array_search($before, $modules);
                                if ($position === false) {
                                    $position = 0;
                                }
                                array_splice($modules, $position, 0, (string)$customModule);
                            } elseif ($after = $customModule->getAttribute('after')) {
                                $position = array_search($after, $modules);
                                if ($position === false) {
                                    $position = count($modules);
                                }
                                array_splice($modules, $position+1, 0, (string)$customModule);
                            } else {
                                $modules[] = (string)$customModule;
                            }
                        }
                    }
                }
 
                $frontName = (string)$routerConfig->args->frontName;
                $this->addModule($frontName, $modules, $routerName);
            }
        }
    }
 
    // Other methods
}

As you can see, Magento simply collects all modules with registered admin controller into an array, and uses this information later to match correct controller. Before and after attributes are used to provide simple sorting ability, so we could, for example, make Magento load our controller before some other controller (controller rewrites).

So first we have if/elseif code blocks handling case when before and after attributes are added to relevant XML node, and else block handling case where these attributes are not present. That else block doing casting of $customModule (an instance of Varien_Simplexml_Element extending SimpleXMLElement) into string and adding this string to modules array, confirms that before and after attributes are not required by standard router architecture.

If this is the case, then why doesn't admin controller work without specifying before or after attributes? Answer to this question is that there is a bug at line 21 of previous code listing where Magento uses if with condition being $customModule, in order to check is node empty. Unfortunately casting SimpleXMLElement into boolean results with false if node doesn't have any attributes, and this is what's causing controller not to be taken into consideration, if it doesn't have before or after nodes. Actually, node can have any other attribute in order to work, so following also fixes suspected Enterprise_Rma bug outlined at the beginning of this article:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
<!-- app/code/core/Enterprise/Rma/etc/config.xml -->
<config>
 
    <!-- Other XML -->
 
    <admin>
        <routers>
            <adminhtml>
                <args>
                    <modules>
                        <enterprise_rma something="whatever">Enterprise_Rma_Adminhtml</enterprise_rma>
                    </modules>
                </args>
            </adminhtml>
        </routers>
    </admin>
 
    <!-- Other XML -->
 
</config>

The most fascinating part of this story is that this bug has been around for 5 years, and survived 25 Magento Community Edition releases. Fortunately it's fixed in Magento 1.9.0.0 and later Magento versions by casting SimpleXMLElement into string, before evaluating it using if clause. Nevertheless, if you intend supporting virtually any other Magento version except latest 1.9.x branch, you will need to keep adding dummy attributes to your Magento module's admin/routers/adminhtml/args/modules XML Path node.

Even though this article might seem like trashing Magento due to this bug, it's actually tribute to Magento complexity and should be taken as such. Again, really fascinating!

DevGenii

E-commerce is a breeze with Magento Certified Developer Plus & Zend Certified PHP Engineer nearby. Get in touch!

Leave a Reply

Your email address will not be published. Required fields are marked *