Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[Routing] support scheme requirement without redirectable dumped matcher #26304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 28, 2018
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
83 changes: 37 additions & 46 deletions 83 src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ class PhpMatcherDumper extends MatcherDumper
{
private $expressionLanguage;
private $signalingException;
private $supportsRedirections;

/**
* @var ExpressionFunctionProviderInterface[]
Expand Down Expand Up @@ -57,7 +56,7 @@ public function dump(array $options = array())

// trailing slash support is only enabled if we know how to redirect the user
$interfaces = class_implements($options['base_class']);
$this->supportsRedirections = isset($interfaces[RedirectableUrlMatcherInterface::class]);
$supportsRedirections = isset($interfaces[RedirectableUrlMatcherInterface::class]);

return <<<EOF
<?php
Expand All @@ -77,7 +76,7 @@ public function __construct(RequestContext \$context)
\$this->context = \$context;
}

{$this->generateMatchMethod()}
{$this->generateMatchMethod($supportsRedirections)}
}

EOF;
Expand All @@ -91,7 +90,7 @@ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterfac
/**
* Generates the code for the match method implementing UrlMatcherInterface.
*/
private function generateMatchMethod(): string
private function generateMatchMethod(bool $supportsRedirections): string
{
// Group hosts by same-suffix, re-order when possible
$matchHost = false;
Expand All @@ -111,7 +110,7 @@ private function generateMatchMethod(): string

$code = <<<EOF
{
\$allow = array();
\$allow = \$allowSchemes = array();
\$pathinfo = rawurldecode(\$rawPathinfo);
\$context = \$this->context;
\$requestMethod = \$canonicalMethod = \$context->getMethod();
Expand All @@ -124,25 +123,44 @@ private function generateMatchMethod(): string

EOF;

if ($this->supportsRedirections) {
if ($supportsRedirections) {
return <<<'EOF'
public function match($pathinfo)
{
$allow = array();
if ($ret = $this->doMatch($pathinfo, $allow)) {
$allow = $allowSchemes = array();
if ($ret = $this->doMatch($pathinfo, $allow, $allowSchemes)) {
return $ret;
}
if ('/' !== $pathinfo && in_array($this->context->getMethod(), array('HEAD', 'GET'), true)) {
if ($allow) {
throw new MethodNotAllowedException(array_keys($allow));
}
if (!in_array($this->context->getMethod(), array('HEAD', 'GET'), true)) {
// no-op
} elseif ($allowSchemes) {
redirect_scheme:
$scheme = $this->context->getScheme();
$this->context->setScheme(key($allowSchemes));
try {
if ($ret = $this->doMatch($pathinfo)) {
return $this->redirect($pathinfo, $ret['_route'], $this->context->getScheme()) + $ret;
}
} finally {
$this->context->setScheme($scheme);
}
} elseif ('/' !== $pathinfo) {
$pathinfo = '/' !== $pathinfo[-1] ? $pathinfo.'/' : substr($pathinfo, 0, -1);
if ($ret = $this->doMatch($pathinfo)) {
if ($ret = $this->doMatch($pathinfo, $allow, $allowSchemes)) {
return $this->redirect($pathinfo, $ret['_route']) + $ret;
}
if ($allowSchemes) {
goto redirect_scheme;
}
}

throw $allow ? new MethodNotAllowedException(array_keys($allow)) : new ResourceNotFoundException();
throw new ResourceNotFoundException();
}

private function doMatch(string $rawPathinfo, array &$allow = array()): ?array
private function doMatch(string $rawPathinfo, array &$allow = array(), array &$allowSchemes = array()): ?array

EOF
.$code."\n return null;\n }";
Expand Down Expand Up @@ -238,9 +256,6 @@ private function compileStaticRoutes(array $staticRoutes, bool $matchHost): stri
}

if (!$route->getCondition()) {
if (!$this->supportsRedirections && $route->getSchemes()) {
throw new \LogicException('The "schemes" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
}
$default .= sprintf(
"%s => array(%s, %s, %s, %s),\n",
self::export($url),
Expand Down Expand Up @@ -535,8 +550,8 @@ private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
} else {
$code = '';
}
if ($this->supportsRedirections) {
$code .= <<<EOF

$code .= <<<EOF

\$hasRequiredScheme = !\$requiredSchemes || isset(\$requiredSchemes[\$context->getScheme()]);
if (\$requiredMethods && !isset(\$requiredMethods[\$canonicalMethod]) && !isset(\$requiredMethods[\$requestMethod])) {
Expand All @@ -546,28 +561,13 @@ private function compileSwitchDefault(bool $hasVars, bool $matchHost): string
break;
}
if (!\$hasRequiredScheme) {
if ('GET' !== \$canonicalMethod) {
break;
}

return \$this->redirect(\$rawPathinfo, \$ret['_route'], key(\$requiredSchemes)) + \$ret;
}

return \$ret;

EOF;
} else {
$code .= <<<EOF

if (\$requiredMethods && !isset(\$requiredMethods[\$canonicalMethod]) && !isset(\$requiredMethods[\$requestMethod])) {
\$allow += \$requiredMethods;
\$allowSchemes += \$requiredSchemes;
break;
}

return \$ret;

EOF;
}

return $code;
}
Expand Down Expand Up @@ -647,9 +647,6 @@ private function compileRoute(Route $route, string $name, bool $checkHost): stri
}

if ($schemes = $route->getSchemes()) {
if (!$this->supportsRedirections) {
throw new \LogicException('The "schemes" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
}
$schemes = self::export(array_flip($schemes));
if ($methods) {
$code .= <<<EOF
Expand All @@ -662,11 +659,8 @@ private function compileRoute(Route $route, string $name, bool $checkHost): stri
goto $gotoname;
}
if (!\$hasRequiredScheme) {
if ('GET' !== \$canonicalMethod) {
goto $gotoname;
}

return \$this->redirect(\$rawPathinfo, '$name', key(\$requiredSchemes)) + \$ret;
\$allowSchemes += \$requiredSchemes;
goto $gotoname;
}


Expand All @@ -675,11 +669,8 @@ private function compileRoute(Route $route, string $name, bool $checkHost): stri
$code .= <<<EOF
\$requiredSchemes = $schemes;
if (!isset(\$requiredSchemes[\$context->getScheme()])) {
if ('GET' !== \$canonicalMethod) {
goto $gotoname;
}

return \$this->redirect(\$rawPathinfo, '$name', key(\$requiredSchemes)) + \$ret;
\$allowSchemes += \$requiredSchemes;
goto $gotoname;
}


Expand Down
54 changes: 27 additions & 27 deletions 54 src/Symfony/Component/Routing/Matcher/RedirectableUrlMatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@

namespace Symfony\Component\Routing\Matcher;

use Symfony\Component\Routing\Exception\ExceptionInterface;
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
use Symfony\Component\Routing\Route;

/**
* @author Fabien Potencier <fabien@symfony.com>
Expand All @@ -27,38 +27,38 @@ public function match($pathinfo)
try {
return parent::match($pathinfo);
} catch (ResourceNotFoundException $e) {
if ('/' === $pathinfo || !\in_array($this->context->getMethod(), array('HEAD', 'GET'), true)) {
if (!\in_array($this->context->getMethod(), array('HEAD', 'GET'), true)) {
throw $e;
}

try {
$pathinfo = '/' !== $pathinfo[-1] ? $pathinfo.'/' : substr($pathinfo, 0, -1);
$ret = parent::match($pathinfo);
if ($this->allowSchemes) {
redirect_scheme:
$scheme = $this->context->getScheme();
$this->context->setScheme(current($this->allowSchemes));
try {
$ret = parent::match($pathinfo);

return $this->redirect($pathinfo, $ret['_route'] ?? null) + $ret;
} catch (ResourceNotFoundException $e2) {
return $this->redirect($pathinfo, $ret['_route'] ?? null, $this->context->getScheme()) + $ret;
} catch (ExceptionInterface $e2) {
throw $e;
} finally {
$this->context->setScheme($scheme);
}
} elseif ('/' === $pathinfo) {
throw $e;
}
}
}
} else {
try {
$pathinfo = '/' !== $pathinfo[-1] ? $pathinfo.'/' : substr($pathinfo, 0, -1);
$ret = parent::match($pathinfo);

/**
* {@inheritdoc}
*/
protected function handleRouteRequirements($pathinfo, $name, Route $route)
{
// expression condition
if ($route->getCondition() && !$this->getExpressionLanguage()->evaluate($route->getCondition(), array('context' => $this->context, 'request' => $this->request ?: $this->createRequest($pathinfo)))) {
return array(self::REQUIREMENT_MISMATCH, null);
}

// check HTTP scheme requirement
$scheme = $this->context->getScheme();
$schemes = $route->getSchemes();
if ($schemes && !$route->hasScheme($scheme)) {
return array(self::ROUTE_MATCH, $this->redirect($pathinfo, $name, current($schemes)));
return $this->redirect($pathinfo, $ret['_route'] ?? null) + $ret;
} catch (ExceptionInterface $e2) {
if ($this->allowSchemes) {
goto redirect_scheme;
}
throw $e;
}
}
}

return array(self::REQUIREMENT_MATCH, null);
}
}
30 changes: 22 additions & 8 deletions 30 src/Symfony/Component/Routing/Matcher/UrlMatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,19 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface
const ROUTE_MATCH = 2;

protected $context;

/**
* Collects HTTP methods that would be allowed for the request.
*/
protected $allow = array();

/**
* Collects URI schemes that would be allowed for the request.
*
* @internal
*/
protected $allowSchemes = array();

protected $routes;
protected $request;
protected $expressionLanguage;
Expand Down Expand Up @@ -70,7 +82,7 @@ public function getContext()
*/
public function match($pathinfo)
{
$this->allow = array();
$this->allow = $this->allowSchemes = array();

if ($ret = $this->matchCollection(rawurldecode($pathinfo), $this->routes)) {
return $ret;
Expand Down Expand Up @@ -141,22 +153,28 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
continue;
}

// check HTTP method requirement
$hasRequiredScheme = !$route->getSchemes() || $route->hasScheme($this->context->getScheme());
if ($requiredMethods = $route->getMethods()) {
// HEAD and GET are equivalent as per RFC
if ('HEAD' === $method = $this->context->getMethod()) {
$method = 'GET';
}

if (!in_array($method, $requiredMethods)) {
if (self::REQUIREMENT_MATCH === $status[0]) {
if ($hasRequiredScheme) {
$this->allow = array_merge($this->allow, $requiredMethods);
}

continue;
}
}

if (!$hasRequiredScheme) {
$this->allowSchemes = array_merge($this->allowSchemes, $route->getSchemes());

continue;
}

return $this->getAttributes($route, $name, array_replace($matches, $hostMatches, isset($status[1]) ? $status[1] : array()));
}
}
Expand Down Expand Up @@ -197,11 +215,7 @@ protected function handleRouteRequirements($pathinfo, $name, Route $route)
return array(self::REQUIREMENT_MISMATCH, null);
}

// check HTTP scheme requirement
$scheme = $this->context->getScheme();
$status = $route->getSchemes() && !$route->hasScheme($scheme) ? self::REQUIREMENT_MISMATCH : self::REQUIREMENT_MATCH;

return array($status, null);
return array(self::REQUIREMENT_MATCH, null);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public function __construct(RequestContext $context)

public function match($rawPathinfo)
{
$allow = array();
$allow = $allowSchemes = array();
$pathinfo = rawurldecode($rawPathinfo);
$context = $this->context;
$requestMethod = $canonicalMethod = $context->getMethod();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public function __construct(RequestContext $context)

public function match($rawPathinfo)
{
$allow = array();
$allow = $allowSchemes = array();
$pathinfo = rawurldecode($rawPathinfo);
$context = $this->context;
$requestMethod = $canonicalMethod = $context->getMethod();
Expand Down Expand Up @@ -64,8 +64,15 @@ public function match($rawPathinfo)
}
}

$hasRequiredScheme = !$requiredSchemes || isset($requiredSchemes[$context->getScheme()]);
if ($requiredMethods && !isset($requiredMethods[$canonicalMethod]) && !isset($requiredMethods[$requestMethod])) {
$allow += $requiredMethods;
if ($hasRequiredScheme) {
$allow += $requiredMethods;
}
break;
}
if (!$hasRequiredScheme) {
$allowSchemes += $requiredSchemes;
break;
}

Expand Down Expand Up @@ -209,8 +216,15 @@ public function match($rawPathinfo)
}
}

$hasRequiredScheme = !$requiredSchemes || isset($requiredSchemes[$context->getScheme()]);
if ($requiredMethods && !isset($requiredMethods[$canonicalMethod]) && !isset($requiredMethods[$requestMethod])) {
$allow += $requiredMethods;
if ($hasRequiredScheme) {
$allow += $requiredMethods;
}
break;
}
if (!$hasRequiredScheme) {
$allowSchemes += $requiredSchemes;
break;
}

Expand Down
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.