Skip to content

Zlib: Fix the bug when zval_get_long silently cast several options into long in deflate_init - #21860

Merged
Girgias merged 4 commits into
php:masterfrom
LamentXU123:refact
Apr 29, 2026
Merged

Zlib: Fix the bug when zval_get_long silently cast several options into long in deflate_init#21860
Girgias merged 4 commits into
php:masterfrom
LamentXU123:refact

Conversation

@LamentXU123

@LamentXU123LamentXU123 commented Apr 24, 2026

Copy link
Copy Markdown
Member

I opened #21841 yesterday to fix the case in one option strategy and after a closer look this morning I realized almost every options in this function has the same problem. So this follow-up PR fix them all. (I will take a look of similar occasions in the future :) ).

plus: to make things easy I write a helper function to throw errors

Comment threadext/zlib/tests/deflate_init_strategy_type_error.phpt Outdated

@GirgiasGirgias left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Possible suggestion

Comment threadext/zlib/zlib.c Outdated
Comment threadext/zlib/zlib.c Outdated
Comment threadext/zlib/zlib.c
return true;
}

ZVAL_DEINDIRECT(option_buffer);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Blerg, we need to deindirect because it uses the shitty H ZPP specifier...

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yeah... ㄟ( ▔, ▔ )ㄏ
I think it would be good to add a comment here... Anyway, thanks for advice ;)

Comment threadext/zlib/zlib.c Outdated
LamentXU123and others added 2 commits April 27, 2026 22:16
Co-authored-by: Gina Peter Banyard <girgias@php.net>
@Girgias
Girgias merged commit 70f0ca5 into php:masterApr 29, 2026
19 checks passed
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

@LamentXU123@devnexen@Girgias