Fixed Route Prefix Class Not Validated

[Chris D]

This seems a little inconsistent and actually caused a bit of a bother because it took so long to work out there was a spelling mistake in the directory structure.

When creating a Route Prefix, the class isn't validated so it can be prone to mistakes that could be easily caught.

Failing that, it might also be a good idea or an alternative to say why a certain route cannot be found e.g. instead of:

Route {route} could not be found.

Maybe:

Route {route} could not be found. Invalid class.

 

[Liam W]

This always annoyed me, it's caught me out more than once. I never reported it because things that just request a class aren't checked throughout - I'm sure there's somewhere other than the route prefix class, although I'm probably wrong...

Liam

 

[Chris D]

You're right. Looking at the code it is potentially deliberate. It may also be not so easy to catch, overall.

I just did a test with class_exists and that seems to return true even if the filename is spelt incorrectly, of course. So the solution might not be so simple.

 

[Liam W]

You're right. Looking at the code it is potentially deliberate. It may also be not so easy to catch, overall.

I just did a test with class_exists and that seems to return true even if the filename is spelt incorrectly, of course. So the solution might not be so simple.

In theory, class exists will only return true if the file with the class is included... So, how would it return true if it isn't spelt correctly? The file would have to have been included.

The main issue would be with misspellings - however, the same issue happens with class and method checks. Especially on Windows, due to it's case insensitivity.

The main system uses class exists to check the class before checking the method. I'm sure, thinking about it, that this could be used here?

 

[Chris D]

Yeah it's ok. I was being stupid.

Basically, this code should be sufficient:

if ($this->isChanged('route_class'))
{
    if (!XenForo_Application::autoload($this->get('route_class')))
    {
        $this->error(new XenForo_Phrase('route_class_must_be_valid', array('class' => $this->get('route_class'))), 'route_class');
        return;
    }
}

The autoload function is a more robust way of checking a class exists. But certainly class_exists works too.

 

[Mike]

class_exists() is fine. I've changed this for 1.4 on the off chance that there is an add-on that will now fail to install correctly because of this check.