Skip to content

Fix GH-22142: Assertion failure in zendi_try_get_long() on IS_UNDEF. - #22143

Closed
devnexen wants to merge 2 commits into
php:masterfrom
devnexen:gh22142
Closed

Fix GH-22142: Assertion failure in zendi_try_get_long() on IS_UNDEF.#22143
devnexen wants to merge 2 commits into
php:masterfrom
devnexen:gh22142

Conversation

@devnexen

Copy link
Copy Markdown
Member

No description provided.

@devnexen
devnexen marked this pull request as ready for review May 25, 2026 06:15
@ndossche

ndossche commented Aug 22, 2026

Copy link
Copy Markdown
Member

I believe this patch is wrong.

The reason IS_UNDEF ends up being passed to zendi_try_get_long is because deflate_init followed by zlib_get_long_option read an uninit property from an object. This is an IS_UNDEF zval that gets passed to that function.

zval_get_long (not the try-variant) treats IS_UNDEF and IS_NULL both as 0. IMO, IS_UNDEF should just never end up in zendi_try_get_long in the first place and the code should throw on an uninitialized property access.

If we just look at the exception message in the test, it says null, but that's inconsistent with the fact null gets treated as 0.
I believe #22144 was much closer to the real fix, albeit with a wrong exception message.

@Girgias

Copy link
Copy Markdown
Member

Yeah I'm not sure what I was thinking here...

@devnexen

Copy link
Copy Markdown
MemberAuthor

will revert ... it s master only by the way, 8.5 still uses zval_get_long here, the unreachable branch only became reachable with #21860.

also zlib_create_dictionary_string has the same issue and a Zend side patch cannot reach it. and A/H with an object only exists in zlib and SplArray, so nothing else can hand an IS_UNDEF there.

only thing, I would skip the option rather than throw. get_object_vars omits uninitialised typed props and we deprecate the object form in favour of it, so both ought to behave the same ...

@ndossche

Copy link
Copy Markdown
Member

okay, sounds good

Sign up for freeto 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

@devnexen@ndossche@Girgias