Skip to content

Commit

Permalink
[Routing] Route collection prefixes must start with a / and must not …
Browse files Browse the repository at this point in the history
…end with a /
  • Loading branch information
fabpot committed Apr 26, 2011
1 parent b80bb9c commit 98e70f0
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/Symfony/Component/Routing/RouteCollection.php
Expand Up @@ -120,10 +120,18 @@ public function addCollection(RouteCollection $collection, $prefix = '')
*/
public function addPrefix($prefix)
{
// a prefix must not end with a slash
$prefix = rtrim($prefix, '/');

if (!$prefix) {
return;
}

// a prefix must start with a slash
if ('/' !== $prefix[0]) {
$prefix = '/'.$prefix;
}

$this->prefix = $prefix.$this->prefix;

foreach ($this->all() as $route) {
Expand Down

5 comments on commit 98e70f0

@Seldaek
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why it can't end with a '/', well of course it makes it easier to be sure that the next nested prefix starts with a slash, but I'd prefer explicitness here. Type /prefix, or /prefix/, or whatever you want, but if there is no / at the beginning of the URL, there should be an error.

@fabpot
Copy link
Member Author

@fabpot fabpot commented on 98e70f0 Apr 26, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It cannot end with / as this is a prefix to something else that must start with a /.

@Seldaek
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well ok, all I'm saying is it should throw an error if there is no leading slash, and if there is a trailing slash, instead of silently fixing it. Otherwise the routing file looks like a mess.

@fabpot
Copy link
Member Author

@fabpot fabpot commented on 98e70f0 Apr 26, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem is that it's late now to do this as it will probably break a lot of code (which routing file looks like a mess?).

@Seldaek
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I think that prefix: foo when you really mean prefix: /foo looks a bit messy. You specify urls, it should look like urls.

Please sign in to comment.