Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ Changelog

* **2016-01-09**: When ChainRouter::match is used with a RequestMatcher, the
Request is now properly rebuilt from the RequestContext if that was set on
the ChainRouter.
the ChainRouter, and http://localhost is used otherwise to avoid issues with
paths starting with a double forward slash.
* **2014-09-29**: ChainRouter does not require a RouterInterface, as a
RequestMatcher and UrlGenerator is fine too. Fixed chain router interface to
not force a RouterInterface.
Expand Down
50 changes: 20 additions & 30 deletions ChainRouter.php
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,9 @@ protected function sortRouters()
*
* Note: You should use matchRequest if you can.
*/
public function match($url)
public function match($pathinfo)
{
return $this->doMatch($url);
return $this->doMatch($pathinfo);
}

/**
Expand All @@ -162,14 +162,14 @@ public function matchRequest(Request $request)
* At least the url must be provided, if a request is additionally provided
* the request takes precedence.
*
* @param string $url
* @param string $pathinfo
* @param Request $request
*
* @return array An array of parameters
*
* @throws ResourceNotFoundException If no router matched.
*/
private function doMatch($url, Request $request = null)
private function doMatch($pathinfo, Request $request = null)
{
$methodNotAllowed = null;

Expand All @@ -180,14 +180,14 @@ private function doMatch($url, Request $request = null)
// matching requests is more powerful than matching URLs only, so try that first
if ($router instanceof RequestMatcherInterface) {
if (empty($requestForMatching)) {
$requestForMatching = $this->rebuildRequest($url);
$requestForMatching = $this->rebuildRequest($pathinfo);
}

return $router->matchRequest($requestForMatching);
}

// every router implements the match method
return $router->match($url);
return $router->match($pathinfo);
} catch (ResourceNotFoundException $e) {
if ($this->logger) {
$this->logger->debug('Router '.get_class($router).' was not able to match, message "'.$e->getMessage().'"');
Expand All @@ -203,7 +203,7 @@ private function doMatch($url, Request $request = null)

$info = $request
? "this request\n$request"
: "url '$url'";
: "url '$pathinfo'";
throw $methodNotAllowed ?: new ResourceNotFoundException("None of the routers in the chain matched $info");
}

Expand Down Expand Up @@ -255,42 +255,32 @@ public function generate($name, $parameters = array(), $absolute = UrlGeneratorI
*
* If the request context is not set, this simply returns the request object built from $uri.
*
* @param string $uri
* @param string $pathinfo
*
* @return Request
*/
private function rebuildRequest($uri)
private function rebuildRequest($pathinfo)
{
if (!$this->context) {
return Request::create($uri);
return Request::create('http://localhost'.$pathinfo);
}

$uri = $pathinfo;

$server = array();
if ($this->context->getHost()) {
$server['SERVER_NAME'] = $this->context->getHost();
$server['HTTP_HOST'] = $this->context->getHost();
}
if ($this->context->getBaseUrl()) {
$uri = $this->context->getBaseUrl().$uri;
$uri = $this->context->getBaseUrl().$pathinfo;
$server['SCRIPT_FILENAME'] = $this->context->getBaseUrl();
$server['PHP_SELF'] = $this->context->getBaseUrl();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the base url server variables are probably the only ones that you need to deal with as done here because that cannot be deducated from the URI alone.

}
if ('https' === $this->context->getScheme()) {
$server['HTTPS'] = 'on';
$server['SERVER_PORT'] = $this->context->getHttpsPort();
if (443 !== $this->context->getHttpsPort()) {
// this is parsed from the host by symfony request
// https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Request.php#L971
$server['HTTP_HOST'] .= ':'.$this->context->getHttpsPort();
}
} else {
$server['SERVER_PORT'] = $this->context->getHttpPort();
if (80 !== $this->context->getHttpPort()) {
// this is parsed from the host by symfony request
// https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Request.php#L971
$server['HTTP_HOST'] .= ':'.$this->context->getHttpPort();
}
$host = $this->context->getHost() ?: 'localhost';
if ('https' === $this->context->getScheme() && 443 !== $this->context->getHttpsPort()) {
$host .= ':'.$this->context->getHttpsPort();
}
if ('http' === $this->context->getScheme() && 80 !== $this->context->getHttpPort()) {
$host .= ':'.$this->context->getHttpPort();
}
$uri = $this->context->getScheme().'://'.$host.$uri.$this->context->getQueryString();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not work. it's missing the ? before the query string. this is why i linked the exmaple implementation https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Request.php#L1067

Copy link
Member Author

@dbu dbu Jan 21, 2016 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


return Request::create($uri, $this->context->getMethod(), $this->context->getParameters(), array(), array(), $server);
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/Routing/ChainRouterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ public function provideBaseUrl()
*/
public function testMatchWithRequestMatchersAndContext($baseUrl)
{
$url = '/test';
$url = '//test';

list($low) = $this->createRouterMocks();

Expand Down