Bug in rewriterouter

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug in rewriterouter

Kevin McArthur-2
Theres a small bug in the rewrite router when a route isnt found.
 
        /**
         * Find the matching route
         */
        foreach ($this->_routes as $route) {
            if ($params = $route->match($path)) {
                $controller = $params['controller'];
                $action     = $params['action'];
                break;
            }
        }
 
        $actionObj = new Zend_Controller_Dispatcher_Token($controller, $action, $params);
 
 
If the foreach fails to locate a router then $controller, $action and $params will be unset and will throw errors.
 
Kevin

smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Bug in rewriterouter

Martel
Kevin McArthur wrote:

> If the foreach fails to locate a router then $controller, $action and
> $params will be unset and will throw errors.

That's because the default route has been commented out in the process of moving it
out of the proposals to the incubator.

I guess it was removed because noRoute action was never called. The quick and dirty
fix is to set a very generic route and/or add defaults for those variables just
before the foreach loop:

$controller = 'index';
$action = 'noRoute';

And a default like this one will suffice:

$router->addRoute('default', ':controller/:action', array('action' => 'index'));

Try adding this as a first because the routes are matched in a reversed order. So the
most generic ones should always be pushed to the front.

And one more thing - Jayson is working on specification of the routing process at the
moment, so I would expect a revised class in a couple of days.

Thanks for your input.

> Kevin

--
Martel Valgoerad aka Michal Minicki | [hidden email] | http://aie.pl/martel.asc
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
"Idleness is not doing nothing. Idleness is being free to do anything." --
Floyd Dell
Reply | Threaded
Open this post in threaded view
|

Re: Bug in rewriterouter

Kevin McArthur-2
Martel,

What version of RewriteRouter are you referring to with that addRoute call

$router->addRoute('default', ':controller/:action', array('action' =>
'index'));

The svn copy in the incubator has the following definition

    public function addRoute($map, $params = array(), $reqs = array())
    {
        array_unshift($this->_routes, new Zend_Controller_Router_Route($map,
$params, $reqs));
    }

Which appears to take your param2 and 3, and an array of $reqs (what reqs
does im not sure)

The following appears to work on the incubator version

$router->addRoute(':controller/:action', array('controller'=>'index',
'action' => 'index'));

Kevin

----- Original Message -----
From: "Martel Valgoerad" <[hidden email]>
To: <[hidden email]>
Sent: Tuesday, May 30, 2006 11:59 AM
Subject: Re: [fw-general] Bug in rewriterouter


> Kevin McArthur wrote:
>
>> If the foreach fails to locate a router then $controller, $action and
>> $params will be unset and will throw errors.
>
> That's because the default route has been commented out in the process of
> moving it out of the proposals to the incubator.
>
> I guess it was removed because noRoute action was never called. The quick
> and dirty fix is to set a very generic route and/or add defaults for those
> variables just before the foreach loop:
>
> $controller = 'index';
> $action = 'noRoute';
>
> And a default like this one will suffice:
>
> $router->addRoute('default', ':controller/:action', array('action' =>
> 'index'));
>
> Try adding this as a first because the routes are matched in a reversed
> order. So the most generic ones should always be pushed to the front.
>
> And one more thing - Jayson is working on specification of the routing
> process at the moment, so I would expect a revised class in a couple of
> days.
>
> Thanks for your input.
>
>> Kevin
>
> --
> Martel Valgoerad aka Michal Minicki | [hidden email] |
> http://aie.pl/martel.asc
> =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
> "Idleness is not doing nothing. Idleness is being free to do anything." --
> Floyd Dell

smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Bug in rewriterouter

Kevin McArthur-2
In reply to this post by Martel
I think i've sourced another small bug in the RewriteRouter.

http://url.com/test/abc%34asdf/def

$router->addRoute('test/:type/:something', array('controller' => 'test',
'action' => 'index'));

This causes the router to try to noroute action (when theres a urlencoded
value in the url for a parameter)

In fact the url should appear as

http://url.com/test/abc4asdf/def

to the router, and the value should properly end up in the type parameter as
'abc4asdf' as it does if its not encoded.

Kevin

smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Bug in rewriterouter

Martel
In reply to this post by Kevin McArthur-2
Kevin McArthur wrote:

> Martel,
> What version of RewriteRouter are you referring to with that addRoute call

Well, the most recent one I have sent to the list. But you're right, my bad. I
should have sent the example of the one in incubator.

It's a simple change really - you just have to skip the first parameter which
acts as a name of the route in current version:

$router->addRoute(':controller/:action', array('action' =>  'index'));

But you have figured that already :)

> Kevin

--
Martel Valgoerad aka Michal Minicki | [hidden email] | http://aie.pl/martel.asc
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
"Ambition is a poor excuse for not having sense enough to be lazy." -- Edgar
Bergen
Reply | Threaded
Open this post in threaded view
|

Re: Bug in rewriterouter

Martel
In reply to this post by Kevin McArthur-2
Kevin McArthur wrote:

> I think i've sourced another small bug in the RewriteRouter.
> http://url.com/test/abc%34asdf/def
> $router->addRoute('test/:type/:something', array('controller' => 'test',
> 'action' => 'index'));

It's a different problem this time. I'm not sure if it's a router's
responsibility to parse an URL. Actually, I think router should ask a
Http_Request object for a parsed and cleaned path info.

Decision what to do is on the hands of Jayson at the moment, so we have to wait
a little more for it to be resolved. I will add your example to my test suite,
though.

Thanks for catching this one.

> Kevin

--
Martel Valgoerad aka Michal Minicki | [hidden email] | http://aie.pl/martel.asc
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
"A paranoid is someone who knows a little of what's going on." -- William S.
Burroughs
Reply | Threaded
Open this post in threaded view
|

Re: Bug in rewriterouter

Jayson Minard (ZF)
The incoming URL handling probably does belong in a http request object,
this is something I am currently looking at.

--j


On 5/31/06 12:46 AM, "Martel Valgoerad" <[hidden email]> wrote:

> Kevin McArthur wrote:
>
>> I think i've sourced another small bug in the RewriteRouter.
>> http://url.com/test/abc%34asdf/def
>> $router->addRoute('test/:type/:something', array('controller' => 'test',
>> 'action' => 'index'));
>
> It's a different problem this time. I'm not sure if it's a router's
> responsibility to parse an URL. Actually, I think router should ask a
> Http_Request object for a parsed and cleaned path info.
>
> Decision what to do is on the hands of Jayson at the moment, so we have to
> wait
> a little more for it to be resolved. I will add your example to my test suite,
> though.
>
> Thanks for catching this one.
>
>> Kevin


Reply | Threaded
Open this post in threaded view
|

RE: Bug in rewriterouter

Jared Williams-3

Yes, HttpRequest object should have a getUrl() method, returning a url object,
which is passed to the router.

Also the HttpRequest should have a setUrl($url) method so can rewrite the
request.

So for instance the front controller is registered as 404 error handler, can
use the url actually requested ($_SERVER['REDIRECT_URL'] in Apache, or
extracted from querystring in IIS), and not the url of the error page.

Jared
 

> -----Original Message-----
> From: Jayson Minard [mailto:[hidden email]]
> Sent: 31 May 2006 15:33
> To: Martel Valgoerad; [hidden email]
> Subject: Re: [fw-general] Bug in rewriterouter
>
> The incoming URL handling probably does belong in a http
> request object,
> this is something I am currently looking at.
>
> --j
>
>
> On 5/31/06 12:46 AM, "Martel Valgoerad" <[hidden email]> wrote:
>
> > Kevin McArthur wrote:
> >
> >> I think i've sourced another small bug in the RewriteRouter.
> >> http://url.com/test/abc%34asdf/def
> >> $router->addRoute('test/:type/:something',
> array('controller' => 'test',
> >> 'action' => 'index'));
> >
> > It's a different problem this time. I'm not sure if it's a router's
> > responsibility to parse an URL. Actually, I think router
> should ask a
> > Http_Request object for a parsed and cleaned path info.
> >
> > Decision what to do is on the hands of Jayson at the
> moment, so we have to
> > wait
> > a little more for it to be resolved. I will add your
> example to my test suite,
> > though.
> >
> > Thanks for catching this one.
> >
> >> Kevin
>
>