Skip to content

Conversation

TimWolla
Copy link
Member

@TimWolla TimWolla commented Sep 8, 2025

Fixes #19765.

@TimWolla TimWolla force-pushed the gh19765-object-properties-load-readonly branch from 46e3177 to 9411205 Compare September 8, 2025 18:56
Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

Looks good conceptually, but what about IS_PROP_UNINIT ?

@TimWolla
Copy link
Member Author

TimWolla commented Sep 8, 2025

but what about IS_PROP_UNINIT ?

I don't think this is necessary. It is only read for IS_UNDEF properties, which any overwritten property will no longer be. Setting the flag is only necessary when unsetting.

@nielsdos
Copy link
Member

nielsdos commented Sep 8, 2025

I don't think this is necessary. It is only read for IS_UNDEF properties, which any overwritten property will no longer be. Setting the flag is only necessary when unsetting.

Right indeed.

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! As mentioned privately, we should also unset the IS_PROP_UNINIT and IS_PROP_LAZY flags. However, the former is not consistently tracked on 8.3 anyway, and the latter only exists in 8.4. So this may be postponed. Lazy objects will also need more fixes to keep zend_lazy_object_info up to date.

@arnaud-lb
Copy link
Member

Lazy objects should be ok as this is a helper for internal classes, and only objects of userland classes can be lazy.

@iluuu1994
Copy link
Member

iluuu1994 commented Sep 9, 2025

@arnaud-lb However, some internal classes are not final and may be overridden (e.g. many SPL classes). I do not know off the top of my head whether they can still be lazy though.

Edit: The answer is no. This is a non-issue then indeed!

@TimWolla TimWolla merged commit 215ebbb into php:PHP-8.3 Sep 9, 2025
9 checks passed
TimWolla added a commit that referenced this pull request Sep 9, 2025
* PHP-8.3:
  zend_API: Do not overwrite `readonly` properties in `object_properties_load()` (#19767)
TimWolla added a commit that referenced this pull request Sep 9, 2025
* PHP-8.4:
  zend_API: Do not overwrite `readonly` properties in `object_properties_load()` (#19767)
@TimWolla TimWolla deleted the gh19765-object-properties-load-readonly branch September 9, 2025 17:37
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.

4 participants