[Routing] Request methods always return a raw path, fix the matcher to decode only once

sq
This commit is contained in:
Victor Berchet 2012-03-19 15:59:52 +01:00
parent d17ba0e147
commit 55014a6841
14 changed files with 92 additions and 25 deletions

View File

@ -329,6 +329,10 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c
* Request::getClientIp() method doesn't take a parameter anymore but bases
itself on the trustProxy parameter.
* Added isMethod() to Request object.
* [BC BREAK] The methods `getPathInfo()`, `getBaseUrl()` and `getBasePath()` of
a `Request` now all return a raw value (vs a urldecoded value before). Any call
to one of these methods must be checked and wrapped in a `rawurldecode()` if
needed.
### HttpKernel
@ -356,6 +360,9 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c
* added a TraceableUrlMatcher
* added the possibility to define options, default values and requirements for placeholders in prefix, including imported routes
* added RouterInterface::getRouteCollection
* [BC BREAK] the UrlMatcher urldecodes the route parameters only once, they were
decoded twice before. Note that the `urldecode()` calls have been change for a
single `rawurldecode()` in order to support `+` for input paths.
### Security

View File

@ -130,7 +130,10 @@ class Configuration implements ConfigurationInterface
->performNoDeepMerging()
->children()
->scalarNode('ip')->end()
->scalarNode('path')->end()
->scalarNode('path')
->setInfo('use the urldecoded format')
->setExample('^/path to resource/')
->end()
->scalarNode('service')->end()
->end()
->end()

View File

@ -154,7 +154,11 @@ class MainConfiguration implements ConfigurationInterface
->prototype('array')
->children()
->scalarNode('requires_channel')->defaultNull()->end()
->scalarNode('path')->defaultNull()->end()
->scalarNode('path')
->defaultNull()
->setInfo('use the urldecoded format')
->setExample('^/path to resource/')
->end()
->scalarNode('host')->defaultNull()->end()
->scalarNode('ip')->defaultNull()->end()
->arrayNode('methods')

View File

@ -16,6 +16,14 @@ use Symfony\Component\HttpFoundation\Session\SessionInterface;
/**
* Request represents an HTTP request.
*
* The methods dealing with URL accept / return a raw path (% encoded):
* * getBasePath
* * getBaseUrl
* * getPathInfo
* * getRequestUri
* * getUri
* * getUriForPath
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @api
@ -568,9 +576,10 @@ class Request
*
* * http://localhost/mysite returns an empty string
* * http://localhost/mysite/about returns '/about'
* * htpp://localhost/mysite/enco%20ded returns '/enco%20ded'
* * http://localhost/mysite/about?var=1 returns '/about'
*
* @return string
* @return string The raw path (i.e. not urldecoded)
*
* @api
*/
@ -588,11 +597,12 @@ class Request
*
* Suppose that an index.php file instantiates this request object:
*
* * http://localhost/index.php returns an empty string
* * http://localhost/index.php/page returns an empty string
* * http://localhost/web/index.php return '/web'
* * http://localhost/index.php returns an empty string
* * http://localhost/index.php/page returns an empty string
* * http://localhost/web/index.php returns '/web'
* * http://localhost/we%20b/index.php returns '/we%20b'
*
* @return string
* @return string The raw path (i.e. not urldecoded)
*
* @api
*/
@ -613,7 +623,7 @@ class Request
* This is similar to getBasePath(), except that it also includes the
* script filename (e.g. index.php) if one exists.
*
* @return string
* @return string The raw url (i.e. not urldecoded)
*
* @api
*/
@ -698,7 +708,7 @@ class Request
/**
* Returns the requested URI.
*
* @return string
* @return string The raw URI (i.e. not urldecoded)
*
* @api
*/
@ -1317,7 +1327,7 @@ class Request
}
$basename = basename($baseUrl);
if (empty($basename) || !strpos(urldecode($truncatedRequestUri), $basename)) {
if (empty($basename) || !strpos(rawurldecode($truncatedRequestUri), $basename)) {
// no match whatsoever; set it blank
return '';
}
@ -1385,7 +1395,7 @@ class Request
return $requestUri;
}
return rawurldecode((string) $pathInfo);
return (string) $pathInfo;
}
/**

View File

@ -127,7 +127,7 @@ class RequestMatcher implements RequestMatcherInterface
if (null !== $this->path) {
$path = str_replace('#', '\\#', $this->path);
if (!preg_match('#'.$path.'#', $request->getPathInfo())) {
if (!preg_match('#'.$path.'#', rawurldecode($request->getPathInfo()))) {
return false;
}
}

View File

@ -150,6 +150,14 @@ class RequestMatcherTest extends \PHPUnit_Framework_TestCase
$this->assertFalse($matcher->matches($request));
}
public function testPathWithEncodedCharacters()
{
$matcher = new RequestMatcher();
$request = Request::create('/admin/fo%20o');
$matcher->matchPath('^/admin/fo o*$');
$this->assertTrue($matcher->matches($request));
}
public function testAttributes()
{
$matcher = new RequestMatcher();

View File

@ -315,6 +315,27 @@ class RequestTest extends \PHPUnit_Framework_TestCase
$request->initialize(array(), array(), array(), array(), array(), $server);
$this->assertEquals('http://servername/path/info?query=string', $request->getUri(), '->getUri() with rewrite, default port without HOST_HEADER');
// With encoded characters
$server = array(
'HTTP_HOST' => 'hostname:8080',
'SERVER_NAME' => 'servername',
'SERVER_PORT' => '8080',
'QUERY_STRING' => 'query=string',
'REQUEST_URI' => '/ba%20se/index_dev.php/foo%20bar/in+fo?query=string',
'SCRIPT_NAME' => '/ba se/index_dev.php',
'PATH_TRANSLATED' => 'redirect:/index.php/foo bar/in+fo',
'PHP_SELF' => '/ba se/index_dev.php/path/info',
'SCRIPT_FILENAME' => '/some/where/ba se/index_dev.php',
);
$request->initialize(array(), array(), array(), array(), array(), $server);
$this->assertEquals(
'http://hostname:8080/ba%20se/index_dev.php/foo%20bar/in+fo?query=string',
$request->getUri()
);
}
/**
@ -984,14 +1005,14 @@ class RequestTest extends \PHPUnit_Framework_TestCase
'/home',
),
array(
'/foo%20bar/app.php/home%2Fbaz',
'/foo%20bar/app.php/home%3Dbaz',
array(
'SCRIPT_FILENAME' => '/home/John Doe/public_html/foo bar/app.php',
'SCRIPT_NAME' => '/foo bar/app.php',
'PHP_SELF' => '/foo bar/app.php',
),
'/foo%20bar/app.php',
'/home%2Fbaz',
'/home%3Dbaz',
),
array(
'/foo/bar+baz',

View File

@ -62,7 +62,7 @@ class PhpMatcherDumper extends MatcherDumper
public function match(\$pathinfo)
{
\$allow = array();
\$pathinfo = urldecode(\$pathinfo);
\$pathinfo = rawurldecode(\$pathinfo);
$code

View File

@ -43,8 +43,6 @@ class TraceableUrlMatcher extends UrlMatcher
protected function matchCollection($pathinfo, RouteCollection $routes)
{
$pathinfo = urldecode($pathinfo);
foreach ($routes as $name => $route) {
if ($route instanceof RouteCollection) {
if (!$ret = $this->matchCollection($pathinfo, $route)) {

View File

@ -72,7 +72,14 @@ class UrlMatcher implements UrlMatcherInterface
}
/**
* {@inheritDoc}
* Tries to match a URL with a set of routes.
*
* @param string $pathinfo The path info to be parsed (raw format, i.e. not urldecoded)
*
* @return array An array of parameters
*
* @throws ResourceNotFoundException If the resource could not be found
* @throws MethodNotAllowedException If the resource was found but the request method is not allowed
*
* @api
*/
@ -80,7 +87,7 @@ class UrlMatcher implements UrlMatcherInterface
{
$this->allow = array();
if ($ret = $this->matchCollection(urldecode($pathinfo), $this->routes)) {
if ($ret = $this->matchCollection(rawurldecode($pathinfo), $this->routes)) {
return $ret;
}
@ -177,7 +184,7 @@ class UrlMatcher implements UrlMatcherInterface
$parameters = $defaults;
foreach ($params as $key => $value) {
if (!is_int($key)) {
$parameters[$key] = rawurldecode($value);
$parameters[$key] = $value;
}
}

View File

@ -30,7 +30,7 @@ interface UrlMatcherInterface extends RequestContextAwareInterface
* If the matcher can not find information, it must throw one of the exceptions documented
* below.
*
* @param string $pathinfo The path info to be parsed
* @param string $pathinfo The path info to be parsed (raw format, i.e. not urldecoded)
*
* @return array An array of parameters
*

View File

@ -23,7 +23,7 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher
public function match($pathinfo)
{
$allow = array();
$pathinfo = urldecode($pathinfo);
$pathinfo = rawurldecode($pathinfo);
// foo
if (0 === strpos($pathinfo, '/foo') && preg_match('#^/foo/(?P<bar>baz|symfony)$#xs', $pathinfo, $matches)) {

View File

@ -23,7 +23,7 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
public function match($pathinfo)
{
$allow = array();
$pathinfo = urldecode($pathinfo);
$pathinfo = rawurldecode($pathinfo);
// foo
if (0 === strpos($pathinfo, '/foo') && preg_match('#^/foo/(?P<bar>baz|symfony)$#xs', $pathinfo, $matches)) {

View File

@ -157,8 +157,8 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
$collection->add('foo', new Route('/{foo}/bar', array(), array('foo' => '['.preg_quote($chars).']+')));
$matcher = new UrlMatcher($collection, new RequestContext(), array());
$this->assertEquals(array('_route' => 'foo', 'foo' => $chars), $matcher->match('/'.urlencode($chars).'/bar'));
$this->assertEquals(array('_route' => 'foo', 'foo' => $chars), $matcher->match('/'.strtr($chars, array('%' => '%25', '+' => '%2B')).'/bar'));
$this->assertEquals(array('_route' => 'foo', 'foo' => $chars), $matcher->match('/'.rawurlencode($chars).'/bar'));
$this->assertEquals(array('_route' => 'foo', 'foo' => $chars), $matcher->match('/'.strtr($chars, array('%' => '%25')).'/bar'));
}
public function testMatchWithDotMetacharacterInRequirements()
@ -216,4 +216,13 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
$matcher = new UrlMatcher($coll, new RequestContext());
$matcher->match('/foo');
}
public function testDecodeOnce()
{
$coll = new RouteCollection();
$coll->add('foo', new Route('/foo/{foo}'));
$matcher = new UrlMatcher($coll, new RequestContext());
$this->assertEquals(array('foo' => 'bar%23', '_route' => 'foo'), $matcher->match('/foo/bar%2523'));
}
}