Skip to content

/ext/standard: Check for empty string in linkinfo()#21793

Open
LamentXU123 wants to merge 5 commits intophp:masterfrom
LamentXU123:refactor
Open

/ext/standard: Check for empty string in linkinfo()#21793
LamentXU123 wants to merge 5 commits intophp:masterfrom
LamentXU123:refactor

Conversation

@LamentXU123
Copy link
Copy Markdown
Contributor

If link_len == 0, it now emits the same warning style as the existing filesystem error path and returns -1 immediately, avoiding the later estrndup() / zend_dirname() work on an empty input as per the TODO message.

Comment thread ext/standard/link.c Outdated
@LamentXU123
Copy link
Copy Markdown
Contributor Author

LamentXU123 commented Apr 19, 2026

Ideally we'd be able to audit all the call sites and basically just mandate that the path passed to zend_dirname() must not be empty nor contain null bytes.

AFAIK This can be fixed inside the zend_dirname function. A simple check for the arguments would work (?) We may still need to customize the error message by passing some info (i.e. php function name that use zend_dirname) to the API to make the error message precise.

@Girgias
Copy link
Copy Markdown
Member

Girgias commented Apr 19, 2026

Ideally we'd be able to audit all the call sites and basically just mandate that the path passed to zend_dirname() must not be empty nor contain null bytes.

AFAIK This can be fixed inside the zend_dirname function. A simple check for the arguments would work (?) We may still need to customize the error message by passing some info (i.e. php function name that use zend_dirname) to the API to make the error message precise.

I don't want to pass the argnum to a Zend API, it is better to enforce the semantics at the call site as I can't imagine most use sites of this API have empty strings.

Copy link
Copy Markdown
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

UPGRADING also needs to be updated.

Comment thread ext/standard/tests/file/symlink_link_linkinfo_is_link_error1.phpt Outdated
@LamentXU123 LamentXU123 requested a review from Girgias April 20, 2026 01:46
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.

3 participants