Allow disabling urldecoding before route parsing. - #16110
Conversation
86e0501 to
76536e1
Compare
| $expected = [ | ||
| 'controller' => 'Products', | ||
| 'action' => 'test', | ||
| 'pass' => ['xx/yy'], |
There was a problem hiding this comment.
So an unmatched passed parameter will always be decoded once while a matched one will not?
There was a problem hiding this comment.
That feels off. We could re-urlencode the pass values, or disable the urldecoding there as well.
There was a problem hiding this comment.
This decoding is due to the rawurldecode() in _parseArgs() which is called only for the part that matches the trailing * in the route template. As you can see in next test, no decoding is done for the value matching the named placeholder {slug}.
Also as per the issue #15269, he expects the decoded value xx/yy and not xx%2Fyy in passed args.
So maybe this special casing for * is intentional? Or perhaps this decoding should be done for ** instead of *?
There was a problem hiding this comment.
So maybe this special casing for * is intentional? Or perhaps this decoding should be done for ** instead of *?
I think the ** intentionally doesn't decode as it is intended to preserve as much as possible of the path. I think having the * use the route's urldecode setting instead.
There was a problem hiding this comment.
I think having the * use the route's urldecode setting instead.
Okay, I will update accordingly.
5fe50cb to
1b585e7
Compare
Currently path prefix matching does not treat %2f as /, and `Route` has an option for it. In the past we've made changes in this area (#18050, #16110) to make urldecoding optional, and to intentionally decode urlencoding in path segments to support non-ascii applications. We got a report on the security list for a potential issue where *if* an application enforced authorization within path prefixed scopes, and had fallback routes enabled, then one could potentially bypass the scoped middleware and hit the fallback routes which inconsistently handle %2f. These changes align the behavior of urldecoding between RouteCollection and Route with a new shared internal function. I thought a function was better than exposing a static method on a public class. Thanks to Rotem Reiss for reporting this issue.
Currently path prefix matching does not treat %2f as /, and `Route` has an option for it. In the past we've made changes in this area (#18050, #16110) to make urldecoding optional, and to intentionally decode urlencoding in path segments to support non-ascii applications. We got a report on the security list for a potential issue where *if* an application enforced authorization within path prefixed scopes, and had fallback routes enabled, then one could potentially bypass the scoped middleware and hit the fallback routes which inconsistently handle %2f. These changes align the behavior of urldecoding between RouteCollection and Route with a new shared internal function. I thought a function was better than exposing a static method on a public class. Thanks to Rotem Reiss for reporting this issue.
Currently path prefix matching does not treat %2f as /, and `Route` has an option for it. In the past we've made changes in this area (#18050, #16110) to make urldecoding optional, and to intentionally decode urlencoding in path segments to support non-ascii applications. We got a report on the security list for a potential issue where *if* an application enforced authorization within path prefixed scopes, and had fallback routes enabled, then one could potentially bypass the scoped middleware and hit the fallback routes which inconsistently handle %2f. These changes align the behavior of urldecoding between RouteCollection and Route with a new shared internal function. I thought a function was better than exposing a static method on a public class. Thanks to Rotem Reiss for reporting this issue.
Handle %2f in routing more consistently Currently path prefix matching does not treat %2f as /, and `Route` has an option for it. In the past we've made changes in this area (#18050, #16110) to make urldecoding optional, and to intentionally decode urlencoding in path segments to support non-ascii applications. We got a report on the security list for a potential issue where *if* an application enforced authorization within path prefixed scopes, and had fallback routes enabled, then one could potentially bypass the scoped middleware and hit the fallback routes which inconsistently handle %2f. These changes align the behavior of urldecoding between RouteCollection and Route with a new shared internal function. I thought a function was better than exposing a static method on a public class. Thanks to Rotem Reiss for reporting this issue.
Currently path prefix matching does not treat %2f as /, and `Route` has an option for it. In the past we've made changes in this area (#18050, #16110) to make urldecoding optional, and to intentionally decode urlencoding in path segments to support non-ascii applications. We got a report on the security list for a potential issue where *if* an application enforced authorization within path prefixed scopes, and had fallback routes enabled, then one could potentially bypass the scoped middleware and hit the fallback routes which inconsistently handle %2f. These changes align the behavior of urldecoding between RouteCollection and Route with a new shared internal function. I thought a function was better than exposing a static method on a public class. Thanks to Rotem Reiss for reporting this issue.
Currently path prefix matching does not treat %2f as /, and `Route` has an option for it. In the past we've made changes in this area (#18050, #16110) to make urldecoding optional, and to intentionally decode urlencoding in path segments to support non-ascii applications. We got a report on the security list for a potential issue where *if* an application enforced authorization within path prefixed scopes, and had fallback routes enabled, then one could potentially bypass the scoped middleware and hit the fallback routes which inconsistently handle %2f. These changes align the behavior of urldecoding between RouteCollection and Route with a new shared internal function. I thought a function was better than exposing a static method on a public class. Thanks to Rotem Reiss for reporting this issue.
Handle %2f in routing more consistently Currently path prefix matching does not treat %2f as /, and `Route` has an option for it. In the past we've made changes in this area (#18050, #16110) to make urldecoding optional, and to intentionally decode urlencoding in path segments to support non-ascii applications. We got a report on the security list for a potential issue where *if* an application enforced authorization within path prefixed scopes, and had fallback routes enabled, then one could potentially bypass the scoped middleware and hit the fallback routes which inconsistently handle %2f. These changes align the behavior of urldecoding between RouteCollection and Route with a new shared internal function. I thought a function was better than exposing a static method on a public class. Thanks to Rotem Reiss for reporting this issue.
Closes #15269