8000 merged branch arnaud-lb/apache-dumper (PR #5792) · symfony/symfony@a088722 · GitHub
[go: up one dir, main page]
Skip to content

Commit a088722

Browse files
fabpotfabpot
committed
merged branch arnaud-lb/apache-dumper (PR #5792)
This PR was merged into the master branch. Commits ------- c7a8f7a [Routing] fixed possible parameters conflict in apache url matcher Discussion ---------- [Routing] fixed possible parameters conflict in apache url matcher Bug fix: yes Feature addition: no Backwards compatibility break: no (as long as rewrite rules are generated after upgrading) Symfony2 tests pass: yes - This fixes a conflict in route parameters: The rewrite rules currently pass route informations through environment variables: `_ROUTING_DEFAULT_x`: passes the default value of parameter x `_ROUTING__allow_x`: passes the information that method x was allowed for this route `_ROUTING_x`: passes the value of parameter x The problem is that naming a route parameter `DEFAULT_*` or `_allow_*` would not behave as expected. I fixed this by namespacing all environment variables; e.g. parameters are in `_ROUTING_param_*`, defaults in `_ROUTING_default_*`, etc. - The PR fixes a second issue: sometimes the variables are prefixed with multiple REDIRECT_. This PR handles this case by ignoring them all. - This also improves performance a little: Matching a route with two parameters and two default parameters 100K times: (`$_SERVER` was copied from a real request, so with many non `_ROUTING_` variables) master: 6.6s this branch: 4.7s --------------------------------------------------------------------------- by fabpot at 2012-10-27T13:37:24Z Any news on this PR? Is it mergeable? --------------------------------------------------------------------------- by arnaud-lb at 2012-10-27T14:50:08Z There is an issue with default parameter values, I can't find how to fix that in a simple way. Before this PR, default values are never used (if a parameter is an optional not present in the url, the parameter's value is the empty string); after this PR, when a parameter is present and empty (e.g. a requirement like `.*`), its value is set to its default value. --------------------------------------------------------------------------- by Tobion at 2012-10-29T01:36:08Z The problem is, it's not consistent with the default php matcher. So one cannot safely exchange it with the apache matcher because it behaves differently under some (special) circumstances. --------------------------------------------------------------------------- by fabpot at 2012-11-05T08:05:54Z We need to move forward as I want to merge the hostname support in the routing ASAP to have plenty of time for feedback before the 2.2 release. Does it sound reasonable to merge this PR as is an open a ticket about the remaining issue (which should not occur that often anyways)? --------------------------------------------------------------------------- by arnaud-lb at 2012-11-05T09:22:02Z @fabpot it sounds reasonable to me. Also, I've the hostname support branch is currently rebased so that it can be merged without this one. --------------------------------------------------------------------------- by Tobion at 2012-11-11T21:50:20Z Btw, does the ApacheMatcherDumper handle the _scheme requirement? It doesn't look like it. This would be another bug. Anyway, we can probably merge this PR and open new issues for the remaining bugs.
2 parents 0876a19 + c7a8f7a commit a088722

File tree

5 files changed

+181
-120
lines changed

5 files changed

+181
-120
lines changed

‎src/Symfony/Component/Routing/Matcher/ApacheUrlMatcher.php

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
* ApacheUrlMatcher matches URL based on Apache mod_rewrite matching (see ApacheMatcherDumper).
1818
*
1919
* @author Fabien Potencier <fabien@symfony.com>
20+
* @author Arnaud Le Blanc <arnaud.lb@gmail.com>
2021
*/
2122
class ApacheUrlMatcher extends UrlMatcher
2223
{
@@ -36,36 +37,52 @@ public function match($pathinfo)
3637
$parameters = array();
3738
$defaults = array();
3839
$allow = array();
39-
$match = false;
40+
$route = null;
4041

4142
foreach ($_SERVER as $key => $value) {
4243
$name = $key;
4344

44-
if (0 === strpos($name, 'REDIRECT_')) {
45-
$name = substr($name, 9);
45+
// skip non-routing variables
46+
// this improves performance when $_SERVER contains many usual
47+
// variables like HTTP_*, DOCUMENT_ROOT, REQUEST_URI, ...
48+
if (false === strpos($name, '_ROUTING_')) {
49+
continue;
4650
}
4751

48-
if (0 === strpos($name, '_ROUTING_DEFAULTS_')) {
49-
$name = substr($name, 18);
50-
$defaults[$name] = $value;
51-
} elseif (0 === strpos($name, '_ROUTING_')) {
52+
while (0 === strpos($name, 'REDIRECT_')) {
5253
$name = substr($name, 9);
53-
if ('_route' == $name) {
54-
$match = true;
55-
$parameters[$name] = $value;
56-
} elseif (0 === strpos($name, '_allow_')) {
57-
$allow[] = substr($name, 7);
58-
} else {
54+
}
55+
56+
// expect _ROUTING_<type>_<name>
57+
// or _ROUTING_<type>
58+
59+
if (0 !== strpos($name, '_ROUTING_')) {
60+
continue;
61+
}
62+
if (false !== $pos = strpos($name, '_', 9)) {
63+
$type = substr($name, 9, $pos-9);
64+
$name = substr($name, $pos+1);
65+
} else {
66+
$type = substr($name, 9);
67+
}
68+
69+
if ('param' === $type) {
70+
if ('' !== $value) {
5971
$parameters[$name] = $value;
6072
}
61-
} else {
62-
continue;
73+
} elseif ('default' === $type) {
74+
$defaults[$name] = $value;
75+
} elseif ('route' === $type) {
76+
$route = $value;
77+
} elseif ('allow' === $type) {
78+
$allow[] = $name;
6379
}
6480

6581
unset($_SERVER[$key]);
6682
}
6783

68-
if ($match) {
84+
if (null !== $route) {
85+
$parameters['_route'] = $route;
6986
return $this->mergeDefaults($parameters, $defaults);
7087
} elseif (0 < count($allow)) {
7188
throw new MethodNotAllowedException($allow);

‎src/Symfony/Component/Routing/Matcher/Dumper/ApacheMatcherDumper.php

Lines changed: 98 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\Routing\Matcher\Dumper;
1313

14+
use Symfony\Component\Routing\Route;
15+
1416
/**
1517
* Dumps a set of Apache mod_rewrite rules.
1618
*
@@ -46,88 +48,116 @@ public function dump(array $options = array())
4648
$methodVars = array();
4749

4850
foreach ($this->getRoutes()->all() as $name => $route) {
49-
$compiledRoute = $route->compile();
50-
51-
// prepare the apache regex
52-
$regex = $compiledRoute->getRegex();
53-
$delimiter = $regex[0];
54-
$regexPatternEnd = strrpos($regex, $delimiter);
55-
if (strlen($regex) < 2 || 0 === $regexPatternEnd) {
56-
throw new \LogicException('The "%s" route regex "%s" is invalid', $name, $regex);
57-
}
58-
$regex = preg_replace('/\?<.+?>/', '', substr($regex, 1, $regexPatternEnd - 1));
59-
$regex = '^'.self::escape(preg_quote($options['base_uri']).substr($regex, 1), ' ', '\\');
60-
61-
$methods = array();
62-
if ($req = $route->getRequirement('_method')) {
63-
$methods = explode('|', strtoupper($req));
64-
// GET and HEAD are equivalent
65-
if (in_array('GET', $methods) && !in_array('HEAD', $methods)) {
66-
$methods[] = 'HEAD';
67-
}
51+
$rules[] = $this->dumpRoute($name, $route, $options);
52+
$methodVars = array_merge($methodVars, $this->getRouteMethods($route));
53+
}
54+
55+
if (0 < count($methodVars)) {
56+
$rule = array('# 405 Method Not Allowed');
57+
$methodVars = array_values(array_unique($methodVars));
58+
foreach ($methodVars as $i => $methodVar) {
59+
$rule[] = sprintf('RewriteCond %%{_ROUTING_allow_%s} !-z%s', $methodVar, isset($methodVars[$i + 1]) ? ' [OR]' : '');
6860
}
61+
$rule[] = sprintf('RewriteRule .* %s [QSA,L]', $options['script_name']);
6962

70-
$hasTrailingSlash = (!$methods || in_array('HEAD', $methods)) && '/$' === substr($regex, -2) && '^/$' !== $regex;
63+
$rules[] = implode("\n", $rule);
64+
}
7165

72-
$variables = array('E=_ROUTING__route:'.$name);
73-
foreach ($compiledRoute->getVariables() as $i => $variable) {
74-
$variables[] = 'E=_ROUTING_'.$variable.':%'.($i + 1);
75-
}
76-
foreach ($route->getDefaults() as $key => $value) {
77-
$variables[] = 'E=_ROUTING_DEFAULTS_'.$key.':'.strtr($value, array(
78-
':' => '\\:',
79-
'=' => '\\=',
80-
'\\' => '\\\\',
81-
' ' => '\\ ',
82-
));
83-
}
84-
$variables = implode(',', $variables);
85-
86-
$rule = array("# $name");
87-
88-
// method mismatch
89-
if ($req = $route->getRequirement('_method')) {
90-
$methods = explode('|', strtoupper($req));
91-
// GET and HEAD are equivalent
92-
if (in_array('GET', $methods) && !in_array('HEAD', $methods)) {
93-
$methods[] = 'HEAD';
94-
}
95-
$allow = array();
96-
foreach ($methods as $method) {
97-
$methodVars[] = $method;
98-
$allow[] = 'E=_ROUTING__allow_'.$method.':1';
99-
}
100-
101-
$rule[] = "RewriteCond %{REQUEST_URI} $regex";
102-
$rule[] = sprintf("RewriteCond %%{REQUEST_METHOD} !^(%s)$ [NC]", implode('|', $methods));
103-
$rule[] = sprintf('RewriteRule .* - [S=%d,%s]', $hasTrailingSlash ? 2 : 1, implode(',', $allow));
104-
}
66+
return implode("\n\n", $rules)."\n";
67+
}
68+
69+
private function dumpRoute($name, $route, array $options)
70+
{
71+
$compiledRoute = $route->compile();
72+
73+
// prepare the apache regex
74+
$regex = $this->regexToApacheRegex($compiledRoute->getRegex());
75+
$regex = '^'.self::escape(preg_quote($options['base_uri']).substr($regex, 1), ' ', '\\');
76+
77+
$methods = $this->getRouteMethods($route);
78+
79+
$hasTrailingSlash = (!$methods || in_array('HEAD', $methods)) && '/$' === substr($regex, -2) && '^/$' !== $regex;
80+
81+
$variables = array('E=_ROUTING_route:'.$name);
82+
foreach ($compiledRoute->getVariables() as $i => $variable) {
83+
$variables[] = 'E=_ROUTING_param_'.$variable.':%'.($i + 1);
84+
}
85+
foreach ($route->getDefaults() as $key => $value) {
86+
$variables[] = 'E=_ROUTING_default_'.$key.':'.strtr($value, array(
87+
':' => '\\:',
88+
'=' => '\\=',
89+
'\\' => '\\\\',
90+
' ' => '\\ ',
91+
));
92+
}
93+
$variables = implode(',', $variables);
94+
95+
$rule = array("# $name");
10596

106-
// redirect with trailing slash appended
107-
if ($hasTrailingSlash) {
108-
$rule[] = 'RewriteCond %{REQUEST_URI} '.substr($regex, 0, -2).'$';
109-
$rule[] = 'RewriteRule .* $0/ [QSA,L,R=301]';
97+
// method mismatch
98+
if (0 < count($methods)) {
99+
$allow = array();
100+
foreach ($methods as $method) {
101+
$methodVars[] = $method;
102+
$allow[] = 'E=_ROUTING_allow_'.$method.':1';
110103
}
111104

112-
// the main rule
113105
$rule[] = "RewriteCond %{REQUEST_URI} $regex";
114-
$rule[] = "RewriteRule .* {$options['script_name']} [QSA,L,$variables]";
106+
$rule[] = sprintf("RewriteCond %%{REQUEST_METHOD} !^(%s)$ [NC]", implode('|', $methods));
107+
$rule[] = sprintf('RewriteRule .* - [S=%d,%s]', $hasTrailingSlash ? 2 : 1, implode(',', $allow));
108+
}
115109

116-
$rules[] = implode("\n", $rule);
110+
// redirect with trailing slash appended
111+
if ($hasTrailingSlash) {
112+
$rule[] = 'RewriteCond %{REQUEST_URI} '.substr($regex, 0, -2).'$';
113+
$rule[] = 'RewriteRule .* $0/ [QSA,L,R=301]';
117114
}
118115

119-
if (0 < count($methodVars)) {
120-
$rule = array('# 405 Method Not Allowed');
121-
$methodVars = array_values(array_unique($methodVars));
122-
foreach ($methodVars as $i => $methodVar) {
123-
$rule[] = sprintf('RewriteCond %%{_ROUTING__allow_%s} !-z%s', $methodVar, isset($methodVars[$i + 1]) ? ' [OR]' : '');
116+
// the main rule
117+
$rule[] = "RewriteCond %{REQUEST_URI} $regex";
118+
$rule[] = "RewriteRule .* {$options['script_name']} [QSA,L,$variables]";
119+
120+
return implode("\n", $rule);
121+
}
122+
123+
/**
124+
* Returns methods allowed for a route
125+
*
126+
* @param Route $route The route
127+
*
128+
* @return array The methods
129+
*/
130+
private function getRouteMethods(Route $route)
131+
{
132+
$methods = array();
133+
if ($req = $route->getRequirement('_method')) {
134+
$methods = explode('|', strtoupper($req));
135+
// GET and HEAD are equivalent
136+
if (in_array('GET', $methods) && !in_array('HEAD', $methods)) {
137+
$methods[] = 'HEAD';
124138
}
125-
$rule[] = sprintf('RewriteRule .* %s [QSA,L]', $options['script_name']);
139+
}
126140

127-
$rules[] = implode("\n", $rule);
141+
return $methods;
142+
}
143+
144+
/**
145+
* Converts a regex to make it suitable for mod_rewrite
146+
*
147+
* @param string $regex The regex
148+
*
149+
* @return string The converted regex
150+
*/
151+
private function regexToApacheRegex($regex)
152+
{
153+
$delimiter = $regex[0];
154+
$regexPatternEnd = strrpos($regex, $delimiter);
155+
if (strlen($regex) < 2 || 0 === $regexPatternEnd) {
156+
throw new \LogicException('The "%s" route regex "%s" is invalid', $name, $regex);
128157
}
158+
$regex = preg_replace('/\?<.+?>/', '', substr($regex, 1, $regexPatternEnd - 1));
129159

130-
return implode("\n\n", $rules)."\n";
160+
return $regex;
131161
}
132162

133163
/**

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.apache

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,72 +4,72 @@ RewriteRule .* - [QSA,L]
44

55
# foo
66
RewriteCond %{REQUEST_URI} ^/foo/(baz|symfony)$
7-
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:foo,E=_ROUTING_bar:%1,E=_ROUTING_DEFAULTS_def:test]
7+
RewriteRule .* app.php [QSA,L,E=_ROUTING_route:foo,E=_ROUTING_param_bar:%1,E=_ROUTING_default_def:test]
88

99
# foobar
1010
RewriteCond %{REQUEST_URI} ^/foo(?:/([^/]++))?$
11-
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:foobar,E=_ROUTING_bar:%1,E=_ROUTING_DEFAULTS_bar:toto]
11+
RewriteRule .* app.php [QSA,L,E=_ROUTING_route:foobar,E=_ROUTING_param_bar:%1,E=_ROUTING_default_bar:toto]
1212

1313
# bar
1414
RewriteCond %{REQUEST_URI} ^/bar/([^/]++)$
1515
RewriteCond %{REQUEST_METHOD} !^(GET|HEAD)$ [NC]
16-
RewriteRule .* - [S=1,E=_ROUTING__allow_GET:1,E=_ROUTING__allow_HEAD:1]
16+
RewriteRule .* - [S=1,E=_ROUTING_allow_GET:1,E=_ROUTING_allow_HEAD:1]
1717
RewriteCond %{REQUEST_URI} ^/bar/([^/]++)$
18-
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:bar,E=_ROUTING_foo:%1]
18+
RewriteRule .* app.php [QSA,L,E=_ROUTING_route:bar,E=_ROUTING_param_foo:%1]
1919

2020
# baragain
2121
RewriteCond %{REQUEST_URI} ^/baragain/([^/]++)$
2222
RewriteCond %{REQUEST_METHOD} !^(GET|POST|HEAD)$ [NC]
23-
RewriteRule .* - [S=1,E=_ROUTING__allow_GET:1,E=_ROUTING__allow_POST:1,E=_ROUTING__allow_HEAD:1]
23+
RewriteRule .* - [S=1,E=_ROUTING_allow_GET:1,E=_ROUTING_allow_POST:1,E=_ROUTING_allow_HEAD:1]
2424
RewriteCond %{REQUEST_URI} ^/baragain/([^/]++)$
25-
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baragain,E=_ROUTING_foo:%1]
25+
RewriteRule .* app.php [QSA,L,E=_ROUTING_route:baragain,E=_ROUTING_param_foo:%1]
2626

2727
# baz
2828
RewriteCond %{REQUEST_URI} ^/test/baz$
29-
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz]
29+
RewriteRule .* app.php [QSA,L,E=_ROUTING_route:baz]
3030

3131
# baz2
3232
RewriteCond %{REQUEST_URI} ^/test/baz\.html$
33-
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz2]
33+
RewriteRule .* app.php [QSA,L,E=_ROUTING_route:baz2]
3434

3535
# baz3
3636
RewriteCond %{REQUEST_URI} ^/test/baz3$
3737
RewriteRule .* $0/ [QSA,L,R=301]
3838
RewriteCond %{REQUEST_URI} ^/test/baz3/$
39-
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz3]
39+
RewriteRule .* app.php [QSA,L,E=_ROUTING_route:baz3]
4040

4141
# baz4
4242
RewriteCond %{REQUEST_URI} ^/test/([^/]++)$
4343
RewriteRule .* $0/ [QSA,L,R=301]
4444
RewriteCond %{REQUEST_URI} ^/test/([^/]++)/$
45-
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz4,E=_ROUTING_foo:%1]
45+
RewriteRule .* app.php [QSA,L,E=_ROUTING_route:baz4,E=_ROUTING_param_foo:%1]
4646

4747
# baz5
4848
RewriteCond %{REQUEST_URI} ^/test/([^/]++)/$
4949
RewriteCond %{REQUEST_METHOD} !^(GET|HEAD)$ [NC]
50-
RewriteRule .* - [S=2,E=_ROUTING__allow_GET:1,E=_ROUTING__allow_HEAD:1]
50+
RewriteRule .* - [S=2,E=_ROUTING_allow_GET:1,E=_ROUTING_allow_HEAD:1]
5151
RewriteCond %{REQUEST_URI} ^/test/([^/]++)$
5252
RewriteRule .* $0/ [QSA,L,R=301]
5353
RewriteCond %{REQUEST_URI} ^/test/([^/]++)/$
54-
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz5,E=_ROUTING_foo:%1]
54+
RewriteRule .* app.php [QSA,L,E=_ROUTING_route:baz5,E=_ROUTING_param_foo:%1]
5555

5656
# baz5unsafe
5757
RewriteCond %{REQUEST_URI} ^/testunsafe/([^/]++)/$
5858
RewriteCond %{REQUEST_METHOD} !^(POST)$ [NC]
59-
RewriteRule .* - [S=1,E=_ROUTING__allow_POST:1]
59+
RewriteRule .* - [S=1,E=_ROUTING_allow_POST:1]
6060
RewriteCond %{REQUEST_URI} ^/testunsafe/([^/]++)/$
61-
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz5unsafe,E=_ROUTING_foo:%1]
61+
RewriteRule .* app.php [QSA,L,E=_ROUTING_route:baz5unsafe,E=_ROUTING_param_foo:%1]
6262

6363
# baz6
6464
RewriteCond %{REQUEST_URI} ^/test/baz$
65-
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz6,E=_ROUTING_DEFAULTS_foo:bar\ baz]
65+
RewriteRule .* app.php [QSA,L,E=_ROUTING_route:baz6,E=_ROUTING_default_foo:bar\ baz]
6666

6767
# baz7
6868
RewriteCond %{REQUEST_URI} ^/te\ st/baz$
69-
RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz7]
69+
RewriteRule .* app.php [QSA,L,E=_ROUTING_route:baz7]
7070

7171
# 405 Method Not Allowed
72-
RewriteCond %{_ROUTING__allow_GET} !-z [OR]
73-
RewriteCond %{_ROUTING__allow_HEAD} !-z [OR]
74-
RewriteCond %{_ROUTING__allow_POST} !-z
72+
RewriteCond %{_ROUTING_allow_GET} !-z [OR]
73+
RewriteCond %{_ROUTING_allow_HEAD} !-z [OR]
74+
RewriteCond %{_ROUTING_allow_POST} !-z
7575
RewriteRule .* app.php [QSA,L]

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.apache

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@ RewriteRule .* - [QSA,L]
44

55
# foo
66
RewriteCond %{REQUEST_URI} ^/foo$
7-
RewriteRule .* ap\ p_d\ ev.php [QSA,L,E=_ROUTING__route:foo]
7+
RewriteRule .* ap\ p_d\ ev.php [QSA,L,E=_ROUTING_route:foo]

0 commit comments

Comments
 (0)
0