Bug #46449

Routes should urldecode their parts

Added by Adrian Föder over 8 years ago. Updated over 7 years ago.

Status:
Resolved
Priority:
Should have
Category:
MVC - Routing
Start date:
2013-03-20
Due date:
% Done:

100%

Estimated time:
PHP Version:
Has patch:
No
Complexity:

Description

requesting the following Flow project URI, for example,

http://example.org/acme.package/foo%62ar/create

results in a resolved controllerName of foo%62ar while it should of course be foobar.

This is incorrect by reading; and especially problematic in sub-namespaced controllers like even in nativ Neos' Backend\BackendController (however in this case it's circumvented by a custom route).
Such a controller would be

http://example.org/acme.package/accouting%5Caccount/create

intended to result in an controllerName of accounting\account.

#1

Updated by Adrian Föder over 8 years ago

..the question is whether to decode the whole query string pretty early, making

/foo/bar/this%2Fthat/baz

into

/foo/bar/this/that/baz

which could be unwanted, for example in this case:

/acme.package/books/The%20Wizard%20of%20Oz%2FWonderful%20Wizard%20of%20Oz

this is surely intended to result the title representation

The Wizard of Oz/Wonderful wizard of Oz

so I think the decoding should take place "later" on the value of the route part.

#2

Updated by Bastian Waidelich about 8 years ago

  • Status changed from New to Needs Feedback
  • Priority changed from Must have to Should have

I'm not so sure about this one. We should definitely not decode the complete request path (btw. you mix up query string with request path, for the query string it might be different) because that would prevent you from having dynamic RouteParts containing a slash for instance.
I wonder in what cases you end up with the urlencoded version? I suppose this is a problem with "Object routing". The IdentityRoutePart-Handler should probably de/encode the part!

#3

Updated by Gerrit Code Review about 8 years ago

  • Status changed from Needs Feedback to Under Review

Patch set 1 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/23915

#4

Updated by Bastian Waidelich almost 8 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
#5

Updated by Gerrit Code Review almost 8 years ago

  • Status changed from Resolved to Under Review

Patch set 1 for branch 2.0 has been pushed to the review server.
It is available at https://review.typo3.org/23939

#6

Updated by Gerrit Code Review almost 8 years ago

Patch set 2 for branch 2.0 has been pushed to the review server.
It is available at https://review.typo3.org/23939

#7

Updated by Bastian Waidelich almost 8 years ago

  • Assignee set to Bastian Waidelich
#8

Updated by Bastian Waidelich over 7 years ago

  • Status changed from Under Review to Resolved
  • Target version set to 2.0.1

Also available in: Atom PDF