Skip to content

ext/uri: Consistency follow up to #23286 - #23551

Draft
NickSdot wants to merge 39 commits into
php:masterfrom
NickSdot:hardening/ext-uri-reorg-follow-up
Draft

ext/uri: Consistency follow up to #23286#23551
NickSdot wants to merge 39 commits into
php:masterfrom
NickSdot:hardening/ext-uri-reorg-follow-up

Conversation

@NickSdot

@NickSdot NickSdot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Depends on: #23286; as requested in #23286 (comment). Usable diff here until 23286 is merged and here rebased.

cc @kocsismate

Edit: failure unrelated

--FILE--
<?php

$url = Uri\WhatWg\Url::parse("foo://example.com");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was exact duplicate of host_error_forbidden_host_codepoint_special2.phpt (now host_error_colon_special.phpt); I assume the intention was to test the same as in the special tests, so I did that.

@@ -1,5 +1,5 @@
--TEST--
Test Uri\WhatWg\Url component modification - error - modifying multiple components with warnings before throwing an exception
Test Uri\WhatWg\Url component modification - error - earlier warnings not in exception

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here no method added. Although withScheme() is used this is testing that warnings aren't carried over -- so I did not see that as method specific?

@@ -1,5 +1,5 @@
--TEST--
Test Uri\Rfc3986\Uri component modification - port - too small number
Test Uri\Rfc3986\Uri::withPort() - error - negative number

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Elsewhere either "negative value" or "negative number" was used. Here we have (int) -1 so negative number should be correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant