The 523d5e5 commit resolved issues with URL parameters represented by a nested array deeper than level one by replacing the custom code with http_build_query.

However, it also created a regression, as http_build_query encodes object properties, while the original code used rawurlencode which implicitly converted objects to string.

This created a specific issue when $this->parameters associative array contained Horde_Url instance as value.

For example, it is used to keep the original URL in a signed format in other to later redirect to it after login. The expected query string in such case is {login_url}?url={signed_original_url}&horde_logout_token={token}.

However, using http_build_query breaks this format, as the Horde_Url objects are no longer converted to string [using __toString() magic method], instead all their public properties are encoded in the query string. This is exactly what is causing the issue described here.

The Horde_Url object is added as parameter in Horde_Registry's getLogoutUrl() method.

Note, there are multiple other cases like that.

It is also not clear if there are cases with classes other than Horde_Url that are no longer converted to string.

There are different possible solutions.

This PR attempts to address the specific issue with Horde_Url instances in the $this->parameters.

This, together with horde/Core#27 resolves horde/base#12.

@ralflang, please review.

I'd say this calls for unit test coverage.

I just added a unit test. Hopefully I did it correctly.

With this code:

$url = new Horde_Url('test?foo=1&bar=2');
$url->add('url', new Horde_Url('https://example.com/test?_t=123456&_h=Abcd123'));
print strval($url);

Current result (wrong):
test?foo=1&bar=2&url%5Banchor%5D=&url%5Bparameters%5D%5B_t%5D=123456&url%5Bparameters%5D%5B_h%5D=Abcd123&url%5Braw%5D=1&url%5Burl%5D=https%3A%2F%2Fexample.com%2Ftest

With the PR applied (correct, same as before 523d5e5 commit):
test?foo=1&bar=2&url=https%3A%2F%2Fexample.com%2Ftest%3F_t%3D123456%26_h%3DAbcd123

@TDannhauer: please could you review/merge?

Thanks for the PR and test. Your test suceeds, but others do not:

Current results: 26 tests, 71 assertions, 2 failures, 2 incomplete.
Remaining failures are legacy expectations vs http_build_query behavior:
Missing parameter without value “fez” (null dropped)
Indexed arrays encoded as [0],[1] vs expected []-style

as you are deeper in the topic than me: What is your proposal to get URL testset clean again?

Could you please tell me how exactly do you test? I need instructions on how to reproduce.

Also, if you test without applying this PR, is everything clean?

Please see horde/Core#28 also.

@amulet1 : Can you pls have a look at #3 with three additional tests prepared?

Yes, but later.

run run it from cursor & gpt-5:

my-horde-deployment/vendor/bin/phpunit -c my-horde-deployment/vendor/horde/url/phpunit.xml.dist | cat

with the result digested by cursor:

PHPUnit 9.6.24 by Sebastian Bergmann and contributors.

.F...F..................                                          24 / 24 (100%)

Time: 00:00.019, Memory: 6.00 MB

There were 2 failures:

1) Horde\Url\AddTest::testAddSimple
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'test?foo=1&bar=2&baz=3&fez'
+'test?foo=1&bar=2&baz=3'

/var/www/torben/web/horde-deployment/vendor/horde/url/test/Horde/Url/AddTest.php:25

2) Horde\Url\AddTest::testAddMultiple
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'test?foo[]=1&foo[]=2'
+'test?foo%5B0%5D=1&foo%5B1%5D=2'

/var/www/torben/web/horde-deployment/vendor/horde/url/test/Horde/Url/AddTest.php:84

FAILURES!
Tests: 24, Assertions: 69, Failures: 2.

Both failures are also due to commit 523d5e5 (i.e. relying on http_build_query).

I can restore the original behavior, but also support nested arrays properly. Just need answers to questions in my comments in #3.