ZF-7368: Chain route fails to match with an optional route at the end

Issue Type:
Bug
Created:
2009-07-23T11:21:59.000+0000
Last Updated:
2011-02-17T12:29:00.000+0000
Status:
Resolved
Fix version(s):
  • 1.11.4 (03/Mar/11)
Reporter:
Geoffrey Tran (potatobob)
Assignee:
Benoît Durand (intiilapa)
Tags:
  • Zend_Controller_Router
Related issues:
Attachments:

Description

Chained routes with optional params are not matched

Static route: "blog" route: ":page" - This route has default page variable specified as 1

Should match: - /blog - /blog/ - /blog/1 - /blog/2/

Currently doesn't match: - /blog - /blog/

It does not match this in the chain class due to checking of the separator to ensure it matches what was set internally. The problem is since it's optional, the separator doesn't exist to check it with the array....

At first glance, adding "$separator !== false &&" to the if statement seems to fix it, but I have not run the unit tests.


    public function match($request, $partial = null)
    {
        $path    = trim($request->getPathInfo(), '/');
        $subPath = $path;
        $values  = array();

        foreach ($this->_routes as $key => $route) {
            if ($key > 0 && $matchedPath !== null) {
                $separator = substr($subPath, 0, strlen($this->_separators[$key]));
                
                if ($separator !== false && $separator !== $this->_separators[$key]) {
                    return false;                
                }
                
                $subPath = substr($subPath, strlen($separator));
            }

Comments

Posted by Alexander Mazalov (alex347) on 2009-09-29T07:15:57.000+0000

I have the same problem, but I think the soultion must be something like this: 1) add this line before if statement if ($separator === false) $separator=''; or 2) remove line: $path = trim($request->getPathInfo(), '/');

then it works for my case: I'm using urls like /search/query/ /search/query/page3.html 1) $routerPage = new Zend_Controller_Router_Route_Regex(
'/(page(\d+).html)?', array( ), array( 2 => 'page' ) );

    $routerSearch = new Zend_Controller_Router_Route(  
                                            'search/:query',
                                            array(
                                                'controller' => 'list',
                                                'action' => 'search'
                                            ),
                                            array(
                                                'query' => '[^/]+'
                                            )
                                        );  

    $chainedRouterSearch = $routerSearch->chain($routerPage, '');
    $router->addRoute('search', $chainedRouterSearch);

2) Separator is '/' , removed from '(page(\d+).html)?' $routerPage = new Zend_Controller_Router_Route_Regex(
'(page(\d+).html)?', array( ), array( 2 => 'page' ) );

    $routerSearch = new Zend_Controller_Router_Route(  
                                            'search/:query',
                                            array(
                                                'controller' => 'list',
                                                'action' => 'search'
                                            ),
                                            array(
                                                'query' => '[^/]+'
                                            )
                                        );  

    $chainedRouterSearch = $routerSearch->chain($routerPage, '/');
    $router->addRoute('search', $chainedRouterSearch);

Posted by Alexander Mazalov (alex347) on 2009-09-29T07:28:44.000+0000

I have the same problem, but I think the soultion must be something like this: 1) add this line before if statement if ($separator === false) $separator=''; or 2) remove line: $path = trim($request->getPathInfo(), '/');

then it works for my case: I'm using urls like /search/query/ /search/query/page3.html

1)


        $routerPage = new Zend_Controller_Router_Route_Regex(  
                                                '(/page(\d+)\.html)?',
                                                array(
                                                ),
                                                array(
                                                    2 => 'page'
                                                )
                                            );      
                                            
        $routerSearch = new Zend_Controller_Router_Route(  
                                                'search/:query',
                                                array(
                                                    'controller' => 'list',
                                                    'action' => 'search'
                                                ),
                                                array(
                                                    'query' => '[^/]+'
                                                )
                                            );  

        $chainedRouterSearch = $routerSearch->chain($routerPage, '');
        $router->addRoute('search', $chainedRouterSearch);

2) Separator is '/' , removed from '(page(\d+).html)?'


        $routerPage = new Zend_Controller_Router_Route_Regex(  
                                                '(page(\d+)\.html)?',
                                                array(
                                                ),
                                                array(
                                                    2 => 'page'
                                                )
                                            );      
                                            
        $routerSearch = new Zend_Controller_Router_Route(  
                                                'search/:query',
                                                array(
                                                    'controller' => 'list',
                                                    'action' => 'search'
                                                ),
                                                array(
                                                    'query' => '[^/]+'
                                                )
                                            );  

        $chainedRouterSearch = $routerSearch->chain($routerPage, '/');
        $router->addRoute('search', $chainedRouterSearch);

Posted by Edward Surov (zooh) on 2009-11-11T11:49:45.000+0000

I have the same problem.

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

This is exactly the same problem I have reported in bug ZF-9030 only that deals with a module router at the end. But the problem remains... While creating the patch reported in that bug I actually never realised the problem also occurs with 'normal' optional parameters.

Posted by Kim Blomqvist (kblomqvist) on 2010-10-21T10:20:59.000+0000

I would have assumed this issue to be solved along ZF-7848, ZF-8812 and ZF-9030, but there is still something wrong with matching to the default value. I wrote these two unit test (attachment) which of the first one does not pass:

Zend_Controller_Router_Route_ChainTest::testChainingStaticDynamicMatchToDefaultValue Failed asserting that matches expected

Posted by Matthew Weier O'Phinney (matthew) on 2010-10-21T10:48:23.000+0000

I'll look at this for the 1.11.1 release. Thanks for the unit tests -- those definitely help better isolate the issue!

Posted by Kim Blomqvist (kblomqvist) on 2011-01-22T01:08:42.000+0000

I have located the issue. The attached patch includes both the fix and the tests. I suggest that the maintainer of this component or the reviewer would check the patch if they came up with more elegant solution.

Posted by Ben Scholzen (dasprid) on 2011-02-16T07:04:09.000+0000

Patch accepted, but I have no time right now to merge it, can somebody else take that part?

There is one tiny CS violation in the patch tho, where it says:


             }
             // Empty variable? Replace with the default value.
             elseif ($return[$var] == '' || $return[$var] === null) {

But it should be:


             } elseif ($return[$var] == '' || $return[$var] === null) {
                 // Empty variable? Replace with the default value.

Also, the patch partly contains tabs instead of 4-space groups, that should be fixed as well.

Posted by Benoît Durand (intiilapa) on 2011-02-16T11:09:07.000+0000

@Ben, sure I can.

Posted by Benoît Durand (intiilapa) on 2011-02-17T12:28:54.000+0000

Fixed in r23711