Skip to content

Fix GH-13142: Undefined variable name is shortened when contains \0 #13200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Jan 19, 2024

This uses the new %S modifier I introduced in 8d5c3e6
Also makes changes to php_verror such that it can handle strings containing \0

@nielsdos nielsdos changed the title Fix GH13142: Undefined variable name is shortened when contains \0 Fix GH-13142: Undefined variable name is shortened when contains \0 Jan 20, 2024
@@ -1650,6 +1650,20 @@ ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_noreturn(int type, const char *
abort();
}

ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_noreturn_unchecked(int type, const char *format, ...)
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the point of this function, it is 1:1 as zend_error_noreturn above

Copy link
Member Author

Choose a reason for hiding this comment

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

The format attribute

Copy link
Contributor

@mvorisek mvorisek Jan 20, 2024

Choose a reason for hiding this comment

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

I see, ZEND_ATTRIBUTE_FORMAT in .h

--FILE--
<?php

$a = "test\0test";
Copy link
Member

Choose a reason for hiding this comment

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

It might be better if there were also tests in the case of \0test and test\0

@SakiTakamachi
Copy link
Member

Thank you for adding the test case.
LGTM, but I'm not familiar with this area yet, so I'll wait for reviews from more knowledgeable members.

Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

LGTM

@nielsdos
Copy link
Member Author

Merged in fe064d7

@nielsdos nielsdos closed this Jan 20, 2024
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.

Undefined variable name is shortened when contains \0
4 participants