Contrib.social

\Drupal\Component\Utility\UrlHelper::buildQuery() adds extra ampersands

Open on Drupal.org →
Created on 2 February 2012, over 13 years ago
Updated 19 December 2023, over 1 year ago

Problem/Motivation

Calling drupal_http_build_query() --or another function like url() which calls drupal_http_build_query-- with some empty params adds extra ampersand signs. Take a look at next code:

$q = array(
  'action' => 'search',
  'filter' => array(
    'name' => array(),
    'city' => array(),
    'location' => array(),
  ),
  'page' => 10,
);

echo drupal_http_build_query($q);

This displays

action=search&&&&page=10

and you notice that 3 extra amps are added. I know this is because name, city and location are empty but this can happen often if you are making some complex filters.
PHP http_build_query() handles well these empty arrays but drupal don't. With PHP http_build_query we get

action=search&page=10

which is correct.

Steps to reproduce

Proposed resolution

Modify buildQuery to ignore empty arrays and NULL valiues.

--- original proposal ---
To fix this we have to check if $params is empty or not before implode. Please check patch:

--- a/includes/common.inc
+++ b/includes/common.inc
@@ -489,7 +489,8 @@ function drupal_http_build_query(array $query, $parent = '') {
     }
   }
 
-  return implode('&', $params);
+  $params = array_filter($params);
+  return empty($params) ? NULL : implode('&', $params);
 }

Remaining tasks

Review
Commit

User interface changes

API changes

Data model changes

Release notes snippet

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Base  →

Last updated 33 minutes ago

Created by

🇷🇴Romania Sorin Sarca

Live updates comments and jobs are added and updated live.
  • Needs backport to D7

    After being applied to the 8.x branch, it should be considered for backport to the 7.x branch. Note: This tag should generally remain even after the backport has been written, approved, and committed.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • First commit to issue fork.
  • Comment over 1 year ago →
    🇵🇭Philippines abhaypai

    Landing here from Bug smash initiative.

    Few updates from end:

    1. I am still able to replicate this issue on D11.
    2. Added patch compatible with 11.x-dev version with interdiff from #57
    3. Adding Needs Tests tag for help in solving failures in test
    4. Did initial level of testing and works as expected.
    5. Updated IS by adding sample for for Drupal 11
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update over 1 year ago
    Custom Commands Failed
  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇵🇭Philippines abhaypai

    Updating status to Needs Review after test run:

    Need some advice or suggestion for following doubts.

    1. Wondering why phpstan is throwing error elseif (!isset($value)) { i didnt change line for this and its giving error
    2. Its weird that 1806 test run failed and at the same time it also shows core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php passed

    Will create MR, once above issue is addressed.

  • Status changed to Needs work over 1 year ago
  • Comment over 1 year ago →
    🇺🇸United States smustgrave

    Think it's saying your change caused that check to be redundant. So may need to look at the solution and make sure.

  • First commit to issue fork.
  • Merge request !11378UrlHelper::buildQuery() adds extra ampersands → (Open) created by dcam
  • Pipeline finished with Failed
    2 months ago
    Total: 96s
    #440319
  • Comment 2 months ago →
    🇺🇸United States dcam

    I added #65 to the MR.

  • Pipeline finished with Failed
    2 months ago
    Total: 686s
    #440341
contrib.social Blog FAQ Discussions
Production build 0.71.5 2024