[Routing] allow comma and other reserved chars without special meaing to not be encoded in the query and fragment

Tobion · Tobion · commit 76f6c97416ac · 2019-04-16T17:05:04.000+02:00
diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php
@@ -27,6 +27,20 @@
  */
 class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInterface
 {
+    private const QUERY_FRAGMENT_DECODED = [
+        // RFC 3986 explicitly allows those in the query/fragment to reference other URIs unencoded
+        '%2F' => '/',
+        '%3F' => '?',
+        // reserved chars that have no special meaning for HTTP URIs in a query or fragment
+        // this excludes esp. "&", "=" and also "+" because PHP would treat it as a space (form-encoded)
+        '%40' => '@',
+        '%3A' => ':',
+        '%21' => '!',
+        '%3B' => ';',
+        '%2C' => ',',
+        '%2A' => '*',
+    ];
+
     protected $routes;
     protected $context;
 
@@ -275,13 +289,11 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa
         }
 
         if ($extra && $query = http_build_query($extra, '', '&', PHP_QUERY_RFC3986)) {
-            // "/" and "?" can be left decoded for better user experience, see
-            // http://tools.ietf.org/html/rfc3986#section-3.4
-            $url .= '?'.strtr($query, ['%2F' => '/']);
+            $url .= '?'.strtr($query, self::QUERY_FRAGMENT_DECODED);
         }
 
         if ('' !== $fragment) {
-            $url .= '#'.strtr(rawurlencode($fragment), ['%2F' => '/', '%3F' => '?']);
+            $url .= '#'.strtr(rawurlencode($fragment), self::QUERY_FRAGMENT_DECODED);
         }
 
         return $url;
diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php
@@ -337,7 +337,7 @@ public function testUrlEncoding()
     {
         $expectedPath = '/app.php/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id'
             .'/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id'
-            .'?query=%40%3A%5B%5D/%28%29%2A%27%22%20%2B%2C%3B-._~%26%24%3C%3E%7C%7B%7D%25%5C%5E%60%21%3Ffoo%3Dbar%23id';
+            .'?query=@:%5B%5D/%28%29*%27%22%20%2B,;-._~%26%24%3C%3E%7C%7B%7D%25%5C%5E%60!?foo%3Dbar%23id';
 
         // This tests the encoding of reserved characters that are used for delimiting of URI components (defined in RFC 3986)
         // and other special ASCII chars. These chars are tested as static text path, variable path and query param.

Original file line number	Diff line number	Diff line change
`@@ -337,7 +337,7 @@ public function testUrlEncoding()`
`337`	`337`	`{`
`338`	`338`	`$expectedPath = '/app.php/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E\|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id'`
`339`	`339`	`.'/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E\|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id'`
`340`		`- .'?query=%40%3A%5B%5D/%28%29%2A%27%22%20%2B%2C%3B-._~%26%24%3C%3E%7C%7B%7D%25%5C%5E%60%21%3Ffoo%3Dbar%23id';`
	`340`	`+ .'?query=@:%5B%5D/%28%29*%27%22%20%2B,;-._~%26%24%3C%3E%7C%7B%7D%25%5C%5E%60!?foo%3Dbar%23id';`
`341`	`341`
`342`	`342`	`// This tests the encoding of reserved characters that are used for delimiting of URI components (defined in RFC 3986)`
`343`	`343`	`// and other special ASCII chars. These chars are tested as static text path, variable path and query param.`