diff --git a/control/Controller.php b/control/Controller.php index e66a6f3c9..8f4ae4234 100644 --- a/control/Controller.php +++ b/control/Controller.php @@ -552,8 +552,9 @@ class Controller extends RequestHandler implements TemplateGlobalProvider { public static function join_links() { $args = func_get_args(); $result = ""; - $querystrings = array(); + $queryargs = array(); $fragmentIdentifier = null; + foreach($args as $arg) { // Find fragment identifier - keep the last one if(strpos($arg,'#') !== false) { @@ -562,7 +563,8 @@ class Controller extends RequestHandler implements TemplateGlobalProvider { // Find querystrings if(strpos($arg,'?') !== false) { list($arg, $suffix) = explode('?',$arg,2); - $querystrings[] = $suffix; + parse_str($suffix, $localargs); + $queryargs = array_merge($queryargs, $localargs); } if((is_string($arg) && $arg) || is_numeric($arg)) { $arg = (string)$arg; @@ -571,7 +573,8 @@ class Controller extends RequestHandler implements TemplateGlobalProvider { } } - if($querystrings) $result .= '?' . implode('&', $querystrings); + if($queryargs) $result .= '?' . http_build_query($queryargs); + if($fragmentIdentifier) $result .= "#$fragmentIdentifier"; return $result; diff --git a/tests/control/ControllerTest.php b/tests/control/ControllerTest.php index c0e9bd090..edf87e70c 100644 --- a/tests/control/ControllerTest.php +++ b/tests/control/ControllerTest.php @@ -236,9 +236,9 @@ class ControllerTest extends FunctionalTest { $this->assertEquals("admin/crm?existing=1&flush=1", Controller::join_links("admin/crm?existing=1", "?flush=1")); $this->assertEquals("admin/crm/MyForm?a=1&b=2&c=3", Controller::join_links("?a=1", "admin/crm", "?b=2", "MyForm?c=3")); - - /* Note, however, that it doesn't deal with duplicates very well. */ - $this->assertEquals("admin/crm?flush=1&flush=1", Controller::join_links("admin/crm?flush=1", "?flush=1")); + + // And duplicates are handled nicely + $this->assertEquals("admin/crm?foo=2&bar=3&baz=1", Controller::join_links("admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3")); $this->assertEquals ( 'admin/action', Controller::join_links('admin/', '/', '/action'), 'Test that multiple slashes are trimmed.'