Skip to content

Fix GH-15169: stack overflow when var serialization in ext/standard/var #16159

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 2 commits into from

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Oct 1, 2024

Adding a stack check here as I consider serialization to be a more sensitive place where erroring out with an exception seems appropriate.

…d/var

Adding a stack check here as I consider serialization to be a more
sensitive place where erroring out with an exception seems appropriate.
@nielsdos nielsdos requested a review from arnaud-lb October 1, 2024 19:18
@nielsdos nielsdos requested a review from bukka as a code owner October 1, 2024 19:18
@nielsdos nielsdos linked an issue Oct 1, 2024 that may be closed by this pull request
We now handle the stack limit check in the serializer too, which
prevents the regular error message from popping up.
@nielsdos
Copy link
Member Author

nielsdos commented Oct 1, 2024

The error message in the serializer is less detailed because we don't have access to the error message throwing code of zend_execute.c. While it's possible to duplicate the code, I don't think that's desirable. On master we could move it I guess.

Copy link
Member

@arnaud-lb arnaud-lb 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 nielsdos closed this in bd724bd Oct 2, 2024
nielsdos added a commit to nielsdos/php-src that referenced this pull request Oct 4, 2024
With phpGH-16204 merged, we can use the standard error message for the
recently-merged phpGH-16159.
nielsdos added a commit that referenced this pull request Oct 4, 2024
With GH-16204 merged, we can use the standard error message for the
recently-merged GH-16159.

Closes GH-16225.
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.

stack overflow when var serialization in ext/standard/var
2 participants