Skip to content

SAPI: clear current_user and content_type_dup after releasing them - #22974

Open
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/sapi-clear-released-request-info
Open

SAPI: clear current_user and content_type_dup after releasing them#22974
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/sapi-clear-released-request-info

Conversation

@iliaal

@iliaal iliaal commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

sapi_deactivate_module() frees current_user and content_type_dup but leaves both pointers set, unlike auth_user, auth_password and auth_digest right above them. Nothing reads either field between the free and the next sapi_activate(), so this is consistency rather than a live bug, and a second deactivate would double-free.

This is older than the zend_string conversion in #22902, so it targets 8.4.

@iliaal
iliaal requested a review from bukka as a code owner July 31, 2026 14:43
sapi_deactivate_module() frees both without resetting the pointer, unlike
the auth_user, auth_password and auth_digest fields next to them. Nothing
reads them between the free and the next sapi_activate() today, so this is
consistency rather than a live bug.

Closes phpGH-22974
@devnexen

devnexen commented Aug 1, 2026

Copy link
Copy Markdown
Member

Hmmm the change is not wrong, but to qualify it as bug fix worthy of release branches is really stretching it.

@iliaal

iliaal commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

I figured it would be ok as it purely defensive change with no risk of regression

@devnexen

devnexen commented Aug 1, 2026

Copy link
Copy Markdown
Member

in my side not a clear big no, but would need a little bit more convincing ; because it s not fixing an actual bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants