Unable to multiload libraries (eg. sessions) - FIX attached

[Darkless012]

Hi, due to bug in custom loader you can't load 2 session libraries with different names and configs. One a session is loaded, another with different name can't.

File: fuel/application/third_party/fuel/Loader.php

line 192: -- if (isset($this->_ci_classes[$class]) AND $_alias = $this->_ci_classes[$class]) line 192: ++ if (isset($this->_base_classes[$class]) AND $_alias = $this->_base_classes[$class])

And actually its a bug in MX too...

[daylightstudio]

Thanks for the report. Could you provide an example of what you were experiencing so we can test the fix out?

[Darkless012]

I was too rash. My fix only masked another bug. I reviewed the code again. I'll state both example and posiible bug+fix.

The example: (Will end up with Error)

(for this example you have to set up form_sessions table in database: http://codeigniter.com/user_guide/libraries/sessions.html) Saving session to database

private $session_config = array( 'sess_cookie_name' => 'form', 'sess_expiration' => 7200, 'sess_expire_on_close' => TRUE, 'sess_encrypt_cookie' => TRUE, 'sess_use_database' => TRUE, 'sess_table_name' => "form_sessions" ); $this->CI->load->library("session"); $this->CI->load->library('session', $this->session_config, "form_session");

(Line 134): $this->CI->form_session->set_userdata("test","test");

Fatal error: Call to a member function set_userdata() on a non-object in /home/(...path...)/libraries/Multiform.php on line 134

Reviewed bug in Loader (Line 192):

if (isset($this->_ci_classes[$class]) AND $_alias = $this->_ci_classes[$class])

This line tries to test if $class is set and ASSIGN something to alias. Which is (by my opinion) wrong.

This code shoud test the class, and its alias derived from $object_name var passed to method. eg. use of "==" instead of "=":

FIX:

if (isset($this->_ci_classes[$class]) AND $_alias == $this->_ci_classes[$class])

but there is no var $_alias so far, so it is needed to push line 195:

($_alias = strtolower($object_name)) OR $_alias = strtolower($class);

above the line 192.

Hope this will help and fix the Issue.

[daylightstudio]

So you are proposing something like the following at line 191?

...
$class = end(explode('/', $library));

($_alias = strtolower($object_name)) OR $_alias = strtolower($class);

if (isset($this->_ci_classes[$class]) AND $_alias = $this->_ci_classes[$class])
    return CI::$APP->$_alias;
...

[Darkless012]

You've forgot the double == in if statement, the root of all troubles.

... $class = end(explode('/', $library));

($_alias = strtolower($object_name)) OR $_alias = strtolower($class);

if (isset($this->_ci_classes[$class]) AND $_alias == $this->_ci_classes[$class]) return CI::$APP->$_alias; ...

[TeckniX]

I would actually modify it to be as follow, otherwise $class is trying to load with uppercase characters, when $this->_ci_classes has all classes as lowercase

$class = strtolower(end(explode('/', $library)));

        if (isset($this->_ci_classes[$class]) AND $_alias == $this->_ci_classes[$class])
            return CI::$APP->$_alias;

[daylightstudio]

I will look into adding this.

[Darkless012]

Hi, please fix it also in 1.0. It drives me mad to correct same bug across multiple versions.

They have also pull request for the fix in their MX library...

[daylightstudio]

This fix has been pushed to 1.0: https://github.com/daylightstudio/FUEL-CMS/blob/1.0/fuel/modules/fuel/core/Loader.php

[Darkless012]

Sorry, but no it wasn't. Lines 193-196 should be:

($_alias = strtolower($object_name)) OR $_alias = strtolower($class);

if (isset($this->_ci_classes[$class]) AND $_alias == $this->_ci_classes[$class])
    return CI::$APP->$_alias;

Or they have similar fix on bitbucket (though I think their pull request fix is not fully functional) https://bitbucket.org/wiredesignz/codeigniter-modular-extensions-hmvc/pull-request/6/allowing-the-same-library-class-to-be/diff

[daylightstudio]

The reference to the strtolower has been added but your are right that the other changes are missing. You mind posting your lines from the beginning of the method to line 200 or so? Thanks.

[Darkless012]

Loader.php - fixed

/** Load a module library **/
public function library($library, $params = NULL, $object_name = NULL, $module = NULL) {
    if (!isset($module)) $module = $this->_module; // FUEL

    if (is_array($library)) return $this->libraries($library);      

    $class = strtolower(end(explode('/', $library))); //probably useless fix because it will run strtolower on next line

    ($_alias = strtolower($object_name)) OR $_alias = strtolower($class); // this line is swaped 

    if (isset($this->_ci_classes[$_alias]) AND $_alias == $this->_ci_classes[$_alias]) // alias change
        return CI::$APP->$_alias;

    list($path, $_library) = Modules::find($library, $module, 'libraries/');

    /* load library config file  */
    if ($params == NULL) {
        ... (line 201 at 1.0 github)

And I will also try to explain why I think the patch is correct. The previous version ot this if statement,

if (isset($this->_ci_classes[$class]) AND $_alias = $this->_ci_classes[$class])
    return CI::$APP->$_alias;

does: If there is a $class in _ci_classes (class is already loaded), assign this class to $_alias and return it. So if you initiate same library with another alias. It will crash or so.

This line should(IMHO) limit loading same class twice, but also limits loading aliases. So first try to get alias (object_name or class) then check if the resolved alias is loaded. (If it is then just return)