Issues

ZF-9030: When placing the Route_Module as last in a chain an assertion on seperators in match() might fail

Issue Type: Bug Created: 2010-01-31T04:12:01.000+0000 Last Updated: 2010-10-21T09:34:54.000+0000 Status: Resolved Fix version(s): - 1.11.0 (02/Nov/10)

Reporter: Peter Moolenaar (petermoolenaar) Assignee: Matthew Weier O'Phinney (matthew) Tags: - Zend_Controller

Related issues: - ZF-8812

Attachments: - chainingmodule.patch

Description

When you define a chain with the default Module route as a final route in the chain the route will fail when the url doesn't explicitly contain a controller or module.

The following example will fail:

<pre class="highlight">
    public function testChainingDynamicModuleDefaultModuleMatch()
    {
        $foo = new Zend_Controller_Router_Route(':dynamic');
        $dispatcher = new Zend_Controller_Router_ChainTest_Dispatcher();
        $request = new Zend_Controller_Router_ChainTest_Request('<a href="http://www.zend.com/foo">http://www.zend.com/foo</a>');
        $bar = new Zend_Controller_Router_Route_Module(array(),$dispatcher,$request);
        $chain = $foo->chain($bar);

        $res = $chain->match($request);
        $this->assertEquals('foo', $res['dynamic']);
        $this->assertEquals('defctrl', $res['controller']);
        $this->assertEquals('defact', $res['action']);
        $this->assertEquals('default', $res['module']);
    }

It doesn't matter if the first route is a static route or (in the above case) a dynamic one.

My patch is an adjustment of the assertion that happens in Chain::match(). In my opinion it should check if the last route is or isn't of the Module type since the Module route normally contains defaults for module, controller and action.

I'll attach a patch I've made.

Comments

Posted by Peter Moolenaar (petermoolenaar) on 2010-01-31T04:16:03.000+0000

The patch I've included contains (besides the fix for Chain) also an modification of the unit tests.

Of course, if there is a disagreement on the unit tests, the patch for Chain invalidates right away :D

Posted by Matthew Weier O'Phinney (matthew) on 2010-02-01T05:22:31.000+0000

Assigning to Ben to review.

Posted by Peter Moolenaar (petermoolenaar) on 2010-02-15T01:37:25.000+0000

Already had a chance to look at the unit tests?

I'm running with the patched version for a while now (ok, not in production... but still) and haven't experienced any troubles. Still when thinking about the checks that are being performed (which I added myself...) I feel that it might impose unwanted overhead on the Router. For my setup it is totally unnoticeable but for others it might not be... any thoughts?

Posted by Peter Moolenaar (petermoolenaar) on 2010-03-02T03:47:36.000+0000

I think the issue here is basically identical, and thinking about it in that way renders my solution useless since it only deals with the module route, not any other optional routing params

Posted by Kim Blomqvist (kblomqvist) on 2010-10-21T09:25:29.000+0000

I have run the unit test proposed here under the present version of trunk and it pass as expected, because the ZF-8812 was patched yesterday.

Posted by Matthew Weier O'Phinney (matthew) on 2010-10-21T09:34:50.000+0000

Marking as resolved, based on report by Kim.

Have you found an issue?

See the Overview section for more details.

Copyright

© 2006-2016 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.

Contacts