UnionOfRAD/lithium — Issue #340: Removing default behaviour relying on `?url=`
Author: greut
Created: 2012-02-19 17:42:08+00:00
State: open

We've recently hit the wall because with a [third party application](http://www.kelkoo.co.uk/co_15259-overview-of-tradedoubler.html) sending us this kind of url `?tduid=123&url=/foo/bar`. It made me thinking and I'm now seeing this `?url=` thing as harmful. `REQUEST_URI` can be enough in most cases (the [_nginx_ documentation](http://lithify.me/docs/manual/configuration/servers/nginx.wiki) already uses it but not the apache one btw).

Some exemples of _breaking_ websites:

- http://www.nateabele.com/about?url=1
- http://www.totsy.com/pages/contact?url=register

The change:
-----------------

```diff
diff --git a/action/Request.php b/action/Request.php
index 6510ae8..95eec27 100644
--- a/action/Request.php
+++ b/action/Request.php
@@ -568,9 +568,6 @@ class Request extends \lithium\net\http\Request {
if (isset($this->_config['url'])) {
return rtrim($this->_config['url'], '/');
}
- if (!empty($_GET['url']) ) {
- return rtrim($_GET['url'], '/');
- }
if ($uri = $this->env('REQUEST_URI')) {
return str_replace($this->env('base'), '/', parse_url($u
}
```

The backward compatibility hack
--------------------------------------------

If you still need that, you can tweak your index.php Request creation. It's much difficult to do it the other way round when the `$_GET['url']` is part of `Request::_url()`.

```php
array(
'url' => !empty($_GET['url']) ? $_GET['url'] : '/'
)
)));
```

I know I'm asking for a big backward incompatible change here since I assume many websites are using the default [.htaccess](https://github.com/UnionOfRAD/framework/blob/master/app/webroot/.htaccess) or suggested [Apache configuration](http://lithify.me/docs/manual/configuration/servers/apache.wiki). Please consider it for a cleaner lithium :-)