URIs with query strings no longer match in 2.1, matched fine in 2.0.5 #3711

Closed
TwoWholeWorms opened this Issue Feb 7, 2013 · 3 comments

Projects

None yet

3 participants

@TwoWholeWorms
Contributor

Hi,

With ocramius's help I've tracked down a bug on line 139 of Zend\Mvc\Router\Http\Part https://github.com/zendframework/zf2/blob/master/library/Zend/Mvc/Router/Http/Part.php#L139. I'm not sure which version introduced it, but code which worked perfectly on 2.0.5 fails in 2.1.

I have a route for the request URI /dashboard. That route matches correctly at line 139 of Zend\Mvc\Router\Http\Part in 2.1 and line 142 in 2.0.5 https://github.com/zendframework/zf2/blob/release-2.0.5/library/Zend/Mvc/Router/Http/Part.php#L142.

However, if I load /dashboard?foo=bar the route match fails in 2.1 as the additional trim($uri->getQuery()) == "" clause causes the router to try to match another sub route, which fails as there is no more URI to match. Commenting out the trim() clause returns the code to its previous fully working state.

Does anyone know why the extra clause was added? I'm creating a pull request to fix it now.

Cheers,

Ben.

@cybercandyman
Contributor

Can you show me the definition of your route ?
Have you set 'may_terminate' => true in the first part of your Route ?

@TwoWholeWorms
Contributor

@cybercandyman See the pull request for the discussion. We've tracked it down to the new Query routes as intended behaviour, and @weierophinney is trying to work out how to accommodate both behaviours.

@cybercandyman
Contributor

Oh thank you

2013/2/13 syniq notifications@github.com

@cybercandyman https://github.com/cybercandyman See the pull request
for the discussion. We've tracked it down to the new Query routes as
intended behaviour, and @weierophinney https://github.com/weierophinneyis trying to work out how to accommodate both behaviours.


Reply to this email directly or view it on GitHubhttps://github.com/zendframework/zf2/issues/3711#issuecomment-13480009.

@weierophinney weierophinney added a commit to weierophinney/zendframework that referenced this issue Feb 13, 2013
@weierophinney weierophinney [#3711] Fix regression in query string matching
If a route is marked may_terminate, and a query string is present, in
most cases, we should match; the query route is primarily for the
convenience of _generating_ URIs via the assemble() method, and only
matches if a query string is present.

This patch adds tests that do the following:

- If the route is marked may_terminate and a query string is present,
  but there is no Query route child, match and return immediately.
- If the route is marked may_terminate, and a query string is present,
  and a Query route child is present, pass matching on to child routes.
f9fad6e
@ralphschindler ralphschindler added a commit that closed this issue Feb 19, 2013
@ralphschindler ralphschindler Closes #3778
Also closes #3711
Also closes #3652
Also closes #3636

Merge branch 'weierophinney-hotfix/3711'
a3df97c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment