Pending

User tests: Successful: Unsuccessful:

avatar G3z
G3z
21 Mar 2013

with this modification it is possible to write plugins that overrides most joomla core classes, for example JAccess, with a simple plugins like

<?php
    defined('_JEXEC') or die;

    // include_once JPATH_SITE.'/overrides/access.php';
    JLoader::register('JAccess', JPATH_SITE.'/overrides/access.php');

?>
avatar G3z G3z - open - 21 Mar 2013
avatar nicksavov
nicksavov - comment - 21 Mar 2013

Thanks for coding this, Giacomo!

While we’re transitioning to a new integrated tracker, could you report the issue on our current main tracker at JoomlaCode and cross-reference each with a link to other? Here’s the process for reporting on the other tracker:
http://docs.joomla.org/Filing_bugs_and_issues

Alternatively, let me know if you’d like me to create it for you and I can go ahead and do that.

Thanks in advance and thanks again for coding this, Giacomo!

avatar nicksavov
nicksavov - comment - 22 Mar 2013

Great; thank you, Giacomo! I'm set this to pending on the tracker, since we have a pull request. I asked one of our reviewers to take a look too.

avatar nikosdion
nikosdion - comment - 23 Mar 2013

Without wanting to shoot down Giacommo's contribution, I need to express my concern. This kind of change is dangerous. Let's take a practical example with hypothetical versions. Let's say that developer X creates his own JAccess override based on CMS version 3.1.0. Let's assume that a change is introduced in the core in version 3.2.0. Developers will anticipate this change using an if(version_compare(JVERSION, '3.2.0', 'ge') {....}. If the override is in place, though, both the core and the third party code will be broken.

It only becomes worse when the overridden classes are some not used everywhere in the core, making the bug detection more difficult and much less obvious. Third party developers would receive bug reports they cannot reproduce because they depend on third party code which is broken.

Essentially what this patch does is try to legitimise the biggest faux pas in Joomla! extension development: hacking core. It should not be done, for no reason whatsoever.

What needs to be done is probably add "hooks" (I am using a very loose definition) to allow 3PDs to enhance the functionality of core classes without overriding them completely. For example, JAccess could have something like onBeforeGetActions and onAfterGetActions which call something like plugins – again, this is an example off the top of my head, not necessarily something that you can code the way I purport is possible. But this would be the overall idea. Do not override core classes, provide a way to hook on to them and extend them.

avatar mbabker
mbabker - comment - 23 Mar 2013

I'm confused. Your proposal changes the JLoader::import() method (which is tied to jimport('joomla.filesystem.path); type calls) but what you want to do is override core classes by calling JLoader::register(), which is already possible. Using your previous example, the syntax would be:

<?php
defined('_JEXEC') or die;

// include_once JPATH_SITE.'/overrides/access.php';
JLoader::register('JAccess', JPATH_SITE . '/overrides/access.php', true);

Mark Dexter also has some tips in the Joomla! Programming book (see here for one example) about how to override classes. So, basically, it already is possible if you use the API (JLoader::register(), which would work for autoloaded classes) or this trick by Mark.

Keeping Nicholas' concerns in mind, you really shouldn't be hacking the core code for any reason short of a temporary security fix until a proper update can be made, but since this is something you are trying to do, well, here's how to do it.

avatar G3z
G3z - comment - 24 Mar 2013

maybe a bit of context may help:

i am developping a product based on joomla and i needed ACL based on groups and single users.
to achieve the second part i hacked JAccess to check a custom table where i did save overrides for the user.
i did add one method

public static function userOverrides($userId, $action, $asset = null)

and hacked a core method

public static function check($userId, $action, $asset = null)

to check for overrides before returning a result
I didn't find a better option to have this working on he whole site (site and admin) than hacking jAccess

so there are basically 2 objection to my proposal: 1 technical and 1 "political"

Technical (@mbabker):

I did indeed read about Mark Dexter's article and
the force paramente is true by default so our code is the same.

public static function register($class, $path, $force = true)

point is that this doesn't work since core classes are loaded after plugin classes, and always forced.
my pull request was basically about that (don't force core classes registration).

Political (@nikosdion):

I agree with your concern about future jAccess updates that may be obscured by my jAccess. You can easily resolve that by checking for Joomla version in the plugin and override it or not and also disable the related component if you feel like it.
you suggest to trigger events on each method. which is ok but doesn't solve the problem of possible incopatibilities, but i need to change the result of check i could always create incompatibilities with future versions.
bottom line: i think a Joomla based site is admin's responsibility.

avatar realityking
realityking - comment - 24 Mar 2013

JAccess should be covered by the autoloader (since 2.5 if I recall correctly) so it will never be imported. Are you sure this change is necessary?

avatar G3z
G3z - comment - 24 Mar 2013

here is my test:
edit index.php form line 54

// echo $app;
echo "<pre>";

print_r(JLoader::getClassList());

echo "</pre>"; exit;

This way i get all registered classes with their file path

with my modification i get

...
    [jplugin] => /Users/g3z/Sites/fwrPortal/libraries/joomla/plugin/plugin.php
    [jaccess] => /Users/g3z/Sites/fwrPortal/overrides/access.php
    [jxmlelement] => /Users/g3z/Sites/fwrPortal/libraries/joomla/utilities/xmlelement.php
...

with standard joomla

    [jplugin] => /Users/g3z/Sites/fwrPortal/libraries/joomla/plugin/plugin.php
    [jaccess] => /Users/g3z/Sites/fwrPortal/libraries/joomla/access/access.php
    [jxmlelement] => /Users/g3z/Sites/fwrPortal/libraries/joomla/utilities/xmlelement.php

(libraries/loader.php:141):

// If we are importing a library from the Joomla namespace set the class to autoload.
            if (strpos($path, 'joomla') === 0)
            {
                // Since we are in the Joomla namespace prepend the classname with J.
                $class = 'J' . $class;

                // Only register the class for autoloading if the file exists.
                if (is_file($base . '/' . $path . '.php'))
                {
                    self::$classes[strtolower($class)] = $base . '/' . $path . '.php'; 
                                        // I propose :
                                        // self::register(strtolower($class), $base . '/' . $path . '.php',false);
                    $success = true;
                }
            }
            /*
             * If we are not importing a library from the Joomla namespace directly include the
            * file since we cannot assert the file/folder naming conventions.
            */
            else
            {
                // If the file exists attempt to include it.
                if (is_file($base . '/' . $path . '.php'))
                {
                    $success = (bool) include_once $base . '/' . $path . '.php';
                }
            }

it's clear that joomla classes are loaded into JLoader:classes without any check while if register is used this happens (libraries/loader.php:216):

public static function register($class, $path, $force = true)
    {
        // Sanitize class name.
        $class = strtolower($class);

        // Only attempt to register the class if the name and file exist.
        if (!empty($class) && is_file($path))
        {
            // Register the class with the autoloader if not already registered or the force flag is set.
            if (empty(self::$classes[$class]) || $force)
            {
                self::$classes[$class] = $path;
            }
        }
    }

and the class is registered only if there isn't already one or if forced.

so basically i could use JLoader::register from your plugins but joomla classes are loaded afterward and so they overwrite your stuff. I'm suggesting to load core classes from library as a fallback and not by force.

avatar brianteeman
brianteeman - comment - 13 Oct 2013

The related item on the feature tracker has been closed

Add a Comment

Login with GitHub to post a comment