Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

[#3711] Fix regression in query string matching #3778

Closed
wants to merge 1 commit into from

2 participants

@weierophinney

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.

This fixes #3711 and replaces #3712

@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 closed this pull request from a commit
@ralphschindler ralphschindler Closes #3778
Also closes #3711
Also closes #3652
Also closes #3636

Merge branch 'weierophinney-hotfix/3711'
a3df97c
@ralphschindler ralphschindler referenced this pull request from a commit
@ralphschindler ralphschindler Forward #3778
Merge branch 'weierophinney-hotfix/3711' into develop
0823436
@ghost Unknown referenced this pull request from a commit
@ralphschindler ralphschindler Closes #3778
Also closes #3711
Also closes #3652
Also closes #3636

Merge branch 'weierophinney-hotfix/3711'
c30a935
@ghost Unknown referenced this pull request from a commit
@ralphschindler ralphschindler Forward #3778
Merge branch 'weierophinney-hotfix/3711' into develop
36ba5ae
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 13, 2013
  1. @weierophinney

    [#3711] Fix regression in query string matching

    weierophinney authored
    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.
This page is out of date. Refresh to see the latest.
View
22 library/Zend/Mvc/Router/Http/Part.php
@@ -136,8 +136,11 @@ public function match(Request $request, $pathOffset = null)
$uri = $request->getUri();
$pathLength = strlen($uri->getPath());
- if ($this->mayTerminate && $nextOffset === $pathLength && trim($uri->getQuery()) == "") {
- return $match;
+ if ($this->mayTerminate && $nextOffset === $pathLength) {
+ $query = $uri->getQuery();
+ if ('' == trim($query) || !$this->hasQueryChild()) {
+ return $match;
+ }
}
foreach ($this->routes as $name => $route) {
@@ -200,4 +203,19 @@ public function getAssembledParams()
// don't have to return anything here.
return array();
}
+
+ /**
+ * Is one of the child routes a query route?
+ *
+ * @return bool
+ */
+ protected function hasQueryChild()
+ {
+ foreach ($this->routes as $route) {
+ if ($route instanceof Query) {
+ return true;
+ }
+ }
+ return false;
+ }
}
View
89 tests/ZendTest/Mvc/Router/Http/PartTest.php
@@ -13,9 +13,10 @@
use ArrayObject;
use PHPUnit_Framework_TestCase as TestCase;
use Zend\Http\Request as Request;
-use Zend\Stdlib\Request as BaseRequest;
use Zend\Mvc\Router\RoutePluginManager;
use Zend\Mvc\Router\Http\Part;
+use Zend\Stdlib\Parameters;
+use Zend\Stdlib\Request as BaseRequest;
use ZendTest\Mvc\Router\FactoryTester;
class PartTest extends TestCase
@@ -382,4 +383,90 @@ public function testFactoryShouldAcceptTraversableChildRoutes()
$route = Part::factory($options);
$this->assertInstanceOf('Zend\Mvc\Router\Http\Part', $route);
}
+
+ /**
+ * @group 3711
+ */
+ public function testPartRouteMarkedAsMayTerminateCanMatchWhenQueryStringPresent()
+ {
+ $options = array(
+ 'route' => array(
+ 'type' => 'Zend\Mvc\Router\Http\Literal',
+ 'options' => array(
+ 'route' => '/resource',
+ 'defaults' => array(
+ 'controller' => 'ResourceController',
+ 'action' => 'resource',
+ ),
+ ),
+ ),
+ 'route_plugins' => new RoutePluginManager(),
+ 'may_terminate' => true,
+ 'child_routes' => array(
+ 'child' => array(
+ 'type' => 'Zend\Mvc\Router\Http\Literal',
+ 'options' => array(
+ 'route' => '/child',
+ 'defaults' => array(
+ 'action' => 'child',
+ ),
+ ),
+ ),
+ ),
+ );
+
+ $route = Part::factory($options);
+ $request = new Request();
+ $request->setUri('http://example.com/resource?foo=bar');
+ $query = new Parameters(array('foo' => 'bar'));
+ $request->setQuery($query);
+ $query = $request->getQuery();
+
+ $match = $route->match($request);
+ $this->assertInstanceOf('Zend\Mvc\Router\RouteMatch', $match);
+ $this->assertEquals('resource', $match->getParam('action'));
+ }
+
+ /**
+ * @group 3711
+ */
+ public function testPartRouteMarkedAsMayTerminateButWithQueryRouteChildWillMatchChildRoute()
+ {
+ $options = array(
+ 'route' => array(
+ 'type' => 'Zend\Mvc\Router\Http\Literal',
+ 'options' => array(
+ 'route' => '/resource',
+ 'defaults' => array(
+ 'controller' => 'ResourceController',
+ 'action' => 'resource',
+ ),
+ ),
+ ),
+ 'route_plugins' => new RoutePluginManager(),
+ 'may_terminate' => true,
+ 'child_routes' => array(
+ 'query' => array(
+ 'type' => 'Zend\Mvc\Router\Http\Query',
+ 'options' => array(
+ 'defaults' => array(
+ 'query' => 'string',
+ ),
+ ),
+ ),
+ ),
+ );
+
+ $route = Part::factory($options);
+ $request = new Request();
+ $request->setUri('http://example.com/resource?foo=bar');
+ $query = new Parameters(array('foo' => 'bar'));
+ $request->setQuery($query);
+ $query = $request->getQuery();
+
+ $match = $route->match($request);
+ $this->assertInstanceOf('Zend\Mvc\Router\RouteMatch', $match);
+ $this->assertEquals('string', $match->getParam('query'));
+ $this->assertEquals('bar', $match->getParam('foo'));
+ }
}
Something went wrong with that request. Please try again.