Zend_Controller_Dispatcher_Abstract::_formatName() inconsistent with Action name handling

[divinoro]

In normal case, URL is splited to [:module]/[controller]/[action], eg

domain.com/controller/action

In case of using unallowed chars, like "!" or "@", these are removed, so URL like

domain.com/!controller/action

is cleaned and used as

domain.com/controller/action

However, names of view folder is not claned, so entering

domain.com/!controller

will reult in calling "controller" controller and index action, trying to access

views/scripts/!controller/index.phtml

resulting in error 500, instead of 404 not found

Expecting behavior:

Approach should be consistent, so either keep exclamation mark in both (controller and controller folder) of remove unallowed chars also from view folder.

Why it's problem: some pages using #! to address dynamic pages using ajax calls, but crawlerrs are ignoring # and thats resulting in exceptions.

Thank you.
Ivan

[froschdesign]

@divinoro

resulting in error 500

Is this error message from the ZF application or the web server?

[divinoro]

Both are from webserver, but tha's not important :) I think important is unexpected behavior of ZF application. I'm just pointing out, that ZF is inconsistent between controller and action routing in this case..
I'm getting normal exceptions in both cases, of course.

[froschdesign]

Both are from webserver, but tha's not important

The web server can give this error message and a ZF application can give this error message. There's a difference.

I think important is unexpected behavior of ZF application..

Can you provide an unit test for this? Thanks.

[divinoro]

Unfotunately I have no idea how to run unit test or how to create it. Anyway, I tried to point this out, just in case someone will fall in such problems and will investigate couple of hours like me.

[froschdesign]

Here is an unit test:

/**
 * @group GH-440
 * @dataProvider providerControllerNameDoesNotIncludeDisallowedCharacters
 */
public function testControllerNameDoesNotIncludeDisallowedCharacters($controllerName)
{
    $this->request->setControllerName($controllerName)
                  ->setActionName('index');

    $this->helper->setActionController(
        new Bar_IndexController(
            $this->request, $this->response, array()
        )
    );

    $this->assertEquals(
        'index/index.phtml', $this->helper->getViewScript()
    );
}

/**
 * Data provider for testControllerNameDoesNotIncludeDisallowedCharacters
 * @group GH-440
 * @return array
 */
public function providerControllerNameDoesNotIncludeDisallowedCharacters()
{
    return array(
        array('!index'),
        array('@index'),
        array('-index'),
    );
}

[froschdesign]

@akrabat
Do you have an idea?

Maybe this one:

$controller = substr(
    $dispatcher->formatControllerName($request->getControllerName()),
    0,
    -10
);

https://github.com/zendframework/zf1/blob/master/library/Zend/Controller/Action/Helper/ViewRenderer.php#L845

[akrabat]

That looks sane to me.

[secu-ring]

There is a problem with the changed code part when the controller name don't have the word "Controller" as suffix. As I remember in ZF1 the controller names should be "Modulename_ControllerName" without the word "Controller". When I change a controller namer from maybe "Index_Index" to "IndexController" or even "Index_IndexController" the autoloader will not find the class.

The problem doesn't occur in version 1.12.9. When I change the code to the following, it works again.

$controller = $dispatcher->formatControllerName($request->getControllerName());

if (false !== strpos('controller', strtolower($controller))) {
    $controller = substr($controller, 0,-10);
}

Would be nice when You could fix this Problem.

Regards

[froschdesign]

@secu-ring
Do you have overwritten the dispatcher? The standard dispatcher adds always the string "Controller".

[secu-ring]

@froschdesign
Yes, we're doing this. Than it makes sense that it won't work - we have to extend the View Renderer also to override these method.

Thanks for your hint.

[froschdesign]

@secu-ring
I will add a condition.

[froschdesign]

@secu-ring
See 988d293

[weierophinney]

@secu-ring Can you post a note here indicating whether or not the patch from @froschdesign fixes the issue for you?

(Re-opening, as the issue was not resolved to satisfaction previously based on reports.)

[secu-ring]

@weierophinney Yes the patch solved our problem.

[mirow]

This seems to break compatibility with CamelCase controller names?

I have a controller class named "MetadataValidationController", and the view files are in a folder called "metadata-validation". This worked fine before, but resently it stopped working as Zend now looks for the view files in "metadavalidation" folder.

[froschdesign]

I have a controller class named "MetadataValidationController", and the view files are in a folder called "metadata-validation".

This behaviour was never included in any unit tests – really shitty!

This worked fine before, but resently it stopped working as Zend now looks for the view files in "metadavalidation" folder.

Right, because the only word delimiters are "-" and ".".

[ianlayton]

@froschdesign I can't tell from you comment...is this something that will be 'fixed' to go back to the way it has worked in the past or is this something we will need to account for and adjust as a developer?

[froschdesign]

@mirow and @ianlayton
See: #537

Btw. Thanks for reporting! 👍

[akrabat]

Should we roll a new release fairly quickly for this, @froschdesign ?

[froschdesign]

@akrabat
Sure, but I see one more problem: #534
This should be fixed before we roll out the next release.

[bbrala]

Fun stuff, this does change how paths to views are resolved, and breaks current sites since it cannot find the views. Sure its a fix, but the merge breaks like, every site we built. Painfull "fix".

[akrabat]

@bbrala Can you provide an example of what's broken for you?

[bbrala]

Ok, I've looked into this a little, and things started to fall apart in januari, before the code just user $request->getControllerName(); and that was it. I've put the old and new values in a list to show what is happening.

    echo '<br>Old name (pre 40dd702): ' . $request->getControllerName();;

    // Format controller name
    require_once 'Zend/Filter/Word/CamelCaseToDash.php';
    $filter     = new Zend_Filter_Word_CamelCaseToDash();
    $controller = $filter->filter($request->getControllerName());
    echo '<br>After CamelcaseFilter: ' . $controller;
    $controller = $dispatcher->formatControllerName($controller);
    echo '<br>After dispatcherformat: ' . $controller;
    if ('Controller' == substr($controller, -10)) {
        $controller = substr($controller, 0, -10);
    }
    echo '<br>new name: ' . $controller . ' <hr>';

Old name (pre 40dd702): Mainnav
After CamelcaseFilter: Mainnav
After dispatcherformat: MainnavController
new name: Mainnav

Old name (pre 40dd702): tekst
After CamelcaseFilter: tekst
After dispatcherformat: TekstController
new name: Tekst

Old name (pre 40dd702): head-html
After CamelcaseFilter: head-html
After dispatcherformat: HeadHtmlController
new name: HeadHtml

Hope that helps

[froschdesign]

@bbrala

I've put the old and new values in a list to show what is happening.

Sorry, but where is the problem?

/**
 * @group GH-440
 * @dataProvider providerControllerNames
 */
public function testControllerNames($controllerName, $viewScriptDir)
{
   $this->request->setControllerName($controllerName)
                 ->setActionName('index');

   $this->helper->setActionController(
       new Bar_IndexController(
           $this->request, $this->response, array()
       )
   );

   $this->assertEquals(
       sprintf('%s/index.phtml', $viewScriptDir),
       $this->helper->getViewScript()
   );
}

/**
 * Data provider for testControllerNames
 * @group GH-440
 * @return array
 */
public function providerControllerNames()
{
    return array(
        array('Mainnav', 'mainnav'),
        array('tekst', 'tekst'),
        array('head-html', 'head-html'), // strange controller name!
    );
}

Tests passed

Did I miss something?

[bbrala]

That is pretty weird. The problem is the new name is different than the old one, breaking out view script paths. I'll double check this and see if i can expand a bit on my case. Maybe i echo the result to early.

[bbrala]

Ok, i cannot reproduce the problem why this was changed, i will check with the developer who fixed the issue (or was it xD) and see where it broke. It seems to have something to do with something like InschrijvenStap1 first translating into inschrijven-stap1 and now to inschrijven-stap-1. I don't have the today to check it more, but i'll try to make a proper demonstration later.