Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
$url = "http://my.site.com/some_path?foo%26bar=42&foo=20&bar=22";
$parsed = parse_url($url);
$query = $parsed['query'];
$array = drupal_get_query_array($query);
$url_back = drupal_http_build_query($array);
var_dump($array);
var_dump($url_back);
// OUTPUT
// array(3) {
// ["foo%26bar"] => string(2) "42"
// ["foo"] => string(2) "20"
// ["bar"] => string(2) "22"
// }
// string(28) "foo%2526bar=42&foo=20&bar=22"
Since drupal_http_build_query is rawurlencoding both the keys and values given to it in the $query array,
// line 490 in includes/common.inc
$key = ($parent ? $parent . '[' . rawurlencode($key) . ']' : rawurlencode($key));
it would seem logical that drupal_get_query_array, which seems to be to opposite function of drupal_http_build_query, is rawurldecoding the keys as well.
However, because it does not rawurldecode the keys, the example above does not result back in the original url, but has a double encoded "%".
Comments
Comment #1
Sneakyvv CreditAttribution: Sneakyvv at Calibrate for Government of Flanders commentedSimply added rawurlencode around the key.
Obviously, this change might have implications for existing code, but I do think this is the correct way the function should work.
I also added an automated test (as it was suggested in #2143461: drupal_get_query_array encodes not enough if query parameter contains a equal sign (=), my previous fix for drupal_get_query_array).
Comment #2
Sneakyvv CreditAttribution: Sneakyvv at Calibrate for Government of Flanders commentedI also found another problem with drupal_get_query_array (#2446671: drupal_get_query_array should make a distinction between empty and non-existing parameters). Since this issue's patch is changing the same line, the patches conflict. Therefore I attached a combined patch in that issue, but I'll upload it here as well.
Comment #3
Sneakyvv CreditAttribution: Sneakyvv commentedComment #4
markus_petrux CreditAttribution: markus_petrux commentedSince both issues provide the same patch... to make it asier to track... may we consider this one a dup of the other?
Comment #5
David_Rothstein CreditAttribution: David_Rothstein commentedLet's keep these issues separate (as I commented on the other issue).
I think this makes sense since it's inconsistent as is, and also the behavior suggested here seems to be what http://php.net/manual/en/function.parse-str.php does.
There's a chance this could break someone's code, though (if they wrote their code expecting the current behavior). So I'm not sure what to do about that. Maybe a release notes mention would be enough to deal with that...?
Comment #6
Sneakyvv CreditAttribution: Sneakyvv commentedI hear what you're saying that this could break someone's code. However, like you said, it isinconsistent. So I agree that perhaps a release note should suffice.