T dannhauer additional tests for horde/url

[TDannhauer]

Additional tests prepared

[Copilot]

Pull Request Overview

This PR adds additional test coverage for the Horde URL library, specifically focusing on parameter handling edge cases. The tests cover stringable objects as parameters, nested arrays containing Horde_Url instances, and the behavior of Horde_Url parameters in both escaped and raw output modes.

• Adds test for handling stringable objects as URL parameters (marked as incomplete)
• Adds test for nested array parameters containing Horde_Url instances (marked as incomplete)
• Adds test for Horde_Url parameter behavior in raw vs escaped output modes

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
test/Horde/Url/StringableParamTest.php	Tests stringable object handling as URL parameters
test/Horde/Url/NestedParamHordeUrlTest.php	Tests nested array parameters containing Horde_Url instances
test/Horde/Url/HordeUrlParamRawModeTest.php	Tests Horde_Url parameter behavior in raw vs escaped modes

Comments suppressed due to low confidence (2)

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

Method uses tabs for indentation which is inconsistent with PSR-12 coding standards that recommend 4 spaces.

Suggested change

	}
	private function getStringableObject($value)
	{
	return new class($value) {
	private $value;
	public function __construct($value) { $this->value = $value; }
	public function __toString() { return (string)$this->value; }
	};
	}

[Copilot]

Method uses tabs for indentation which is inconsistent with PSR-12 coding standards that recommend 4 spaces.

Suggested change

	}
	public function testNestedArrayWithHordeUrlIsCurrentlyNotConvertedRecursively()
	{
	$url = new Horde_Url('test');
	$url->add('outer', [
	'inner' => new Horde_Url('https://example.com/test?_t=1&_h=2')
	]);
	// Current behavior: Only top-level values are normalized by PR #2.
	// Nested arrays containing Horde_Url will still be expanded by http_build_query.
	// This test documents current behavior and should be adapted once recursion is implemented.
	$this->assertStringContainsString('outer%5Binner%5D%5Bparameters%5D', (string)$url);
	$this->markTestIncomplete('Recursive normalization of nested Horde_Url parameters is not implemented yet.');
	}

[Copilot]

Method uses tabs for indentation which is inconsistent with PSR-12 coding standards that recommend 4 spaces.

Suggested change

	}
	public function testHordeUrlParamEscapedAndRaw()
	{
	$url = new Horde_Url('test');
	$url->add('url', new Horde_Url('https://example.com/test?_t=123456&_h=Abcd123'));
	$this->assertEquals(
	'test?url=https%3A%2F%2Fexample.com%2Ftest%3F_t%3D123456%26_h%3DAbcd123',
	(string)$url
	);
	// Raw output should not HTML-escape the ampersands
	$url->setRaw(true);
	$this->assertEquals(
	'test?url=https%3A%2F%2Fexample.com%2Ftest%3F_t%3D123456%26_h%3DAbcd123',
	(string)$url
	);
	}

[amulet1]

Maybe we should add something else here to see the difference with ampersands?

        $url->add('x', 123);

[amulet1]

If needed, it is not hard to modify the code to support objects at deeper levels.

The 523d5e5 commit broke object-to-string conversion (in particular, the Horde_Url case) while also added support for nested arrays.

To properly fix it, we need to agree on which object(s) exactly we are converting to string [using strval() or (string)]. Currently the fix is specifically for Horde_Url class. If it is not only Horde_Url, we could use a hardcoded list of class names, or check if __toString() method is defined. Both ways have some cons and pros.

Also, an alternative approach might be to check/convert objects passed to add method in $value parameter instead of doing it in toString() method. This might work if $parameters is only used internally.

[amulet1]

Do we want to make it truly universal? Then I guess we need to check for presence of __toString method (see my other comments also). Are we okay with using ReflectionClass::hasMethodor method_exists (instead of explicit conversion)?

[TDannhauer]

To be honest PHP is not my home run, so I do not feel experienced enough for the decisions there. @ralflang what do you think?

[amulet1]

Please see #4.