[4.x]: UrlHelper::buildQuery decodes query parameter names resulting in invalid html attibutes

[leevigraham]

What happened?

Description

UrlHelper::buildQuery decodes query parameter names resulting in invalid html attibutes. Square brackets are not allowed as part of the url and must be encoded.

Steps to reproduce

In a twig template

{{ url('example', {catIds: [1,2,3]}) }}

Expected behavior

should produce /example?catIds%5B0%5D=1&catIds%5B1%5D=2&catIds%5B2%5D=3

Actual behavior

instead it produces: /example?catIds[0]=1&catIds[1]=2&catIds[2]=3

Possible Fix

I'm pretty sure this line is the issue: https://github.com/craftcms/cms/blob/develop/src/helpers/UrlHelper.php#L87

Commenting it out fixes the issue… but I haven't tested side-effects.

Craft CMS version

4.4

PHP version

8.2

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

No response

[brandonkelly]

It looks like we’ve been decoding params via UrlHelper::buildQuery() since 89b54ed, to keep the {draftId} curly braces in-tact here:

cms/src/templates/_layouts/element.html

Line 104 in 89b54ed

{% set draftRedirectUrl = url(element.cpEditUrl, {draftId: '{draftId}'}) %}

But that code got refactored away a while ago, and nothing related to entry publishing is relying on that behavior anymore. So I’ve just deprecated UrlHelper::buildQuery() for Craft 4.5, and switched all usages of it to call http_build_query() directly. (2e97c56)

[engram-design]

I'll mention that this now breaks a URL that contains shorthand Twig (like you mention {draftId} @brandonkelly).

For example, people want to use ?submissionId={id} as the redirectUrl for Formie forms. This is run through UrlHelper::url($url, $request->getQueryStringWithoutPath()) so that it retains any existing query strings with what they supply. So the URL might end up being https://my-site.test?form=contactUs&submission={id}

But this now encodes the braces to be submissionId=%7Bid%7D which doesn't work for when we want to run it through redirectToPostedUrl().

We'll work our way around it for now, but just wanted to flag this.

[engram-design]

Also note that this breaks the multi-site dropdown for things like Globals, where changing the site results in:

/admin/globals/contact?site=%7Bhandle%7D

Due to the use of {handle} in the URL.

[engram-design]

Cheers for the fix - #13603

[brandonkelly]

@engram-design Yep! Thanks for pointing it out. 4.5.2 is out with this change reverted.

[gregorterrill]

Just a heads up in case anyone stumbles across this issue like I did - I think this change was also breaking the Dukt Social plugin (updating to 4.5.2 fixes the issue). Slashes in the parameter of the "Redirect URI" it generates (for Google login, at least) were getting encoded to %2F which Google didn't like - even after adding the encoded version to the whitelist in Google Cloud Console.