FIX Use last of duplicate query params in join_links

When using Controller::join_links to join two links with identical query
params, both query params would be used in the result, ending up with
links that look like `../edit/show/14?locale=en_NZ&locale=mi_NZ`

This patch eliminates duplicate query params, so only the last one
for any key is present in the output.
This commit is contained in:
Hamish Friedlander 2013-09-20 11:28:49 +12:00
parent 7627d95555
commit a8c1dd7f53
2 changed files with 9 additions and 6 deletions

View File

@ -552,8 +552,9 @@ class Controller extends RequestHandler implements TemplateGlobalProvider {
public static function join_links() { public static function join_links() {
$args = func_get_args(); $args = func_get_args();
$result = ""; $result = "";
$querystrings = array(); $queryargs = array();
$fragmentIdentifier = null; $fragmentIdentifier = null;
foreach($args as $arg) { foreach($args as $arg) {
// Find fragment identifier - keep the last one // Find fragment identifier - keep the last one
if(strpos($arg,'#') !== false) { if(strpos($arg,'#') !== false) {
@ -562,7 +563,8 @@ class Controller extends RequestHandler implements TemplateGlobalProvider {
// Find querystrings // Find querystrings
if(strpos($arg,'?') !== false) { if(strpos($arg,'?') !== false) {
list($arg, $suffix) = explode('?',$arg,2); 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)) { if((is_string($arg) && $arg) || is_numeric($arg)) {
$arg = (string)$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"; if($fragmentIdentifier) $result .= "#$fragmentIdentifier";
return $result; return $result;

View File

@ -237,8 +237,8 @@ class ControllerTest extends FunctionalTest {
$this->assertEquals("admin/crm/MyForm?a=1&b=2&c=3", $this->assertEquals("admin/crm/MyForm?a=1&b=2&c=3",
Controller::join_links("?a=1", "admin/crm", "?b=2", "MyForm?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. */ // And duplicates are handled nicely
$this->assertEquals("admin/crm?flush=1&flush=1", Controller::join_links("admin/crm?flush=1", "?flush=1")); $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 ( $this->assertEquals (
'admin/action', Controller::join_links('admin/', '/', '/action'), 'Test that multiple slashes are trimmed.' 'admin/action', Controller::join_links('admin/', '/', '/action'), 'Test that multiple slashes are trimmed.'