When validating my site on W3, I get errors on ampersands in URL's. The URL's use url(), which in turn uses drupal_http_build_query() to parse the $query parameter.

At the bottom of this function, all query parameters are exploded using &. According to W3, this should be &.

Unfortunately, this cannot be fixed by just correcting this inside this function. The buggy output produced by this function is already used by other functions that assume it is buggy and goes on to correct it. Correcting it here will then result in a double correction, which breaks core (e.g. clicking on "Check manually" on the Administration » Reports » Available updates), and probably a lot of other modules that does the same thing.

Fixing this in this function will require major refactoring of core, and some way to deal with the fallout of the "fix" breaking other modules.

CommentFileSizeAuthor
#1 drupal-use_encoded_ampersands-2289665-1.patch314 bytesgisle

Comments

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented
Assigned: Unassigned » gisle
Status: Active » Needs review
StatusFileSize
new drupal-use_encoded_ampersands-2289665-1.patch314 bytes

I have fixed this with the attached patch.

Please review.

Status: Needs review » Needs work

The last submitted patch, 1: drupal-use_encoded_ampersands-2289665-1.patch, failed testing.

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented
Assigned: gisle » Unassigned
Issue summary: View changes
Status: Needs work » Active

It turns out that this is used elsewhere in core, and some of those places fix the buggy output from this function (i.e. they encode the ampersand). So just fixing it here will lead to major breakage elsewhere. Setting it back to "Active", in case somebody smarter than me sees a solution and wants to have a go at it, but IMHO it is unfixable and should be closed.

heine’s picture

heine commented
Status: Active » Closed (won't fix)

A literal & must indeed be encoded as & in HTML. drupal_http_build_query however only deals with generating a correctly encoded query string. There's no promise, nor requirement that you can put this string as-is in HTML.

If you use the return value anywhere, you need to make sure to properly encode it for the environment it is used in.

heine’s picture

heine commented

Or, in other words, either use l() or check_plain / check_url yourself where appropriate.

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented
Issue summary: View changes

Just for the record:

$query = drupal_http_build_query(array = (
     'comic1' => 'Jerry is nice',
     'comic2' => 'Tom&Jerry=friends',
  )
);

produces:

comic1=Jerry%20is%20nice&comic2=Tom%26Jerry%3Dfriends

so replacing all & with & in your own code should fix it. It is a bit awkward, but doable.