Bugfix encoding with Query::fromParameters and URLSearchParams::fromParameters

[nyamsprod]

Bug Report

Information	Description
Package	uri-components
Version	7.x
PHP version	irrelevant
OS Platform	irrelevant

Summary

Data encoding is inconsistent between classes:

• URLSearchParams is not supported with Query::fromParameters
• application/x-www-form-urlencoded rules are not enforced by URLSearchParams::fromParameters

Standalone code, or other way to reproduce the problem

$input = new DateInterval('P12MT8S');
echo URLSearchParams::fromParameters($input)->toString();

Expected result

"y=0&m=12&d=0&h=0&i=3&s=5&f=0.0&invert=0&days=false&from_string=false" 

Actual result

"y=0&m=12&d=0&h=0&i=3&s=5&f=0&invert=0&days=0&from_string=0"

TL;DR: formatting rules from application/x-www-form-urlencoded are not respected.

Source

The issue is because fromParameters internally uses http_build_query thus the conversion is done following the function algorithm and not the url standard specification.

Side effects

A side effect is how QueryInterface, URLSearchParams and most generally Traversable objects are supposed to be treated by both named constructors.

If we were to fully follow http_build_query then they should never be converted into something and just return the empty string. Currently if those objects are submitted they will be formatted prior to submission to http_build_query BUT if they are nested in an array or in a another object they will get converted to the empty string, which is not consistent.

$input = new URLSearchParans('foo=bar');
echo URLSearchParams::fromParameters($input)->toString(); // currently returns 'foo=bar'
http_build_query($input);                                 // returns ''

//but

$input = ['query' => Query::fromParameters(['foo' => 'bar'])];
URLSearchParams::fromParameters($input)->toString() // return ''
http_build_query($input); // returns ''

Solution Proposal

For consistency, I propose:

• to remove support for QueryInterface, URLSearchParams and most generally Traversable from all fromParameters named constructors.
• to implement an http_build_query independent method for URLSearchParams to enforced application/x-www-form-urlencoded rules.

For reference the following PR is an attempt to resolve the issue #122

If removing the support for such class is deemed a BC break then the alternative is to provide yet another named constructor and deprecate the current ones with a warning on their usage in the documentation.

[nyamsprod]

@kelunik @shadowhand what do you think about the issue and more importantly about the proposed solutions ?

The corresponding PR is currently a draft as the Traversable objects are still supported.

[kelunik]

@nyamsprod I wouldn't support object property iteration at all, but I guess that ship has sailed.

[nyamsprod]

@kelunik http_build_query is based on the following

$parameters = static fn (object|array $data): array => is_array($data) ? $data : get_object_vars($data);

So it does not know or care about anything Traversable related at all hence the bug. The main reason being that the function predates the introduction of the interface in PHP 😄

[nyamsprod]

@kelunik to avoid needless BC break I proposed the following:

• I will deprecated all fromParameters methods because of their inconsistencies but will let them act as is and recommend in the documentation to no longer use them.
• I will add fromPhpVariable which will follow the http_buid_query aka use get_object_vars on object.

[kelunik]

@nyamsprod I'd skip Php in the method name.

[nyamsprod]

will update the method names then thanks for the feedback

[nyamsprod]

Fix has been merge to master via #122