Skip to content

ext/uri: Increases test coverage - #23342

Open
NickSdot wants to merge 1 commit into
php:masterfrom
NickSdot:hardening/ext-uri-coverage
Open

ext/uri: Increases test coverage#23342
NickSdot wants to merge 1 commit into
php:masterfrom
NickSdot:hardening/ext-uri-coverage

Conversation

@NickSdot

@NickSdot NickSdot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

This is fully Codex made. I threw a coverage report at it to close uncovered lines/branch gaps. It came up with a tonne of edge cases; this PR is the result of me massaging it a bit, and after filtering out nonsense that did not actually increase coverage as well as things that seemed too esoteric. So everything here increases coverage even though things might seem already tested -- that's because of using slightly different api paths.

After #23286 and #23551 this is no longer fully Codex made, and was rather a starting point.

Coverage: ext/uri/tests

Base: 48d2b45e0748a0a83eddc841cb493a0ae817159a upstream/master
Tree: 8e4b223da30cf74a109274054999784586525836 working tree

+--------+----------+---------+--------------------+-------------------+--------+---------+
|        |    Tests | Sources |              Lines |          Branches |   Time |  Memory |
+--------+----------+---------+--------------------+-------------------+--------+---------+
| Base   |      519 |       7 | 2035/2281 (89.22%) | 733/1234 (59.40%) |  8.50s | 28.0 MB |
| Tree   |      563 |       7 | 2110/2281 (92.50%) | 792/1234 (64.18%) | 10.31s | 28.0 MB |
| Change | +44 / -0 |       0 |  +75 / -0 (+3.29%) | +59 / -0 (+4.78%) | +1.81s | +0.0 MB |
+--------+----------+---------+--------------------+-------------------+--------+---------+

@NickSdot
NickSdot force-pushed the hardening/ext-uri-coverage branch 3 times, most recently from 10422c5 to a0562e4 Compare September 4, 2026 14:29
@NickSdot
NickSdot force-pushed the hardening/ext-uri-coverage branch from a0562e4 to 8e4b223 Compare September 4, 2026 18:32
@NickSdot

NickSdot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Codex proposed the below which sounds sensible, and would add +4 lines/+2 branches. But not sure if it justifies adding another test helper. Thoughts?

C Code
static zend_result zend_test_uri_component_read_failure(
	ZEND_ATTRIBUTE_UNUSED void *uri,
	ZEND_ATTRIBUTE_UNUSED php_uri_component_read_mode read_mode,
	ZEND_ATTRIBUTE_UNUSED zval *retval
) {
	return FAILURE;
}

static ZEND_FUNCTION(zend_test_uri_parse_to_struct_failing_fragment)
{
	php_uri_parser parser = *php_uri_get_parser(NULL);
	parser.property_handler.fragment.read = zend_test_uri_component_read_failure;
	php_uri *uri = php_uri_parse_to_struct(
		&parser, ZEND_STRL("x://u:p@h:1/p?q#f"), PHP_URI_COMPONENT_READ_MODE_RAW, true
	);

	RETURN_BOOL(uri == NULL);
}
Test Code
--TEST--
Test zend_test_uri_parse_to_struct_failing_fragment() - error - fragment retrieval failure
--EXTENSIONS--
zend_test
--FILE--
<?php

var_dump(zend_test_uri_parse_to_struct_failing_fragment());

?>
--EXPECT--
bool(true)

@@ -0,0 +1,23 @@
--TEST--

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.

Debatable if this is worth testing. Thoughts?

@NickSdot
NickSdot marked this pull request as ready for review September 4, 2026 18:49
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