RFC: Deprecate json_encode() on classes marked as non-serializable

Hello Rob,

I think it would be good to list these non-serializable objects in the RFC.

It doesn't have to be exhaustive, but the list includes throwables,
weak-maps, weak-references, closures, fibers, etc. Some people may be
relying on this behavior (and I'd be curious to know that use case myself).

Thank you for your note. As I went through to the code-base in order to
compile the list for the RFC, it also occurred to me that all anonymous
classes are marked as ZEND_ACC_NOT_SERIALIZABLE, but json_encode()-ing such
classes is unproblematic and probably very much desired for user land
classes.

I have updated the RFC and attached implementation to allow anonymous
classes to be json_encode()’d, provided their parent class is not marked as
ZEND_ACC_NOT_SERIALIZABLE.

Thank you very much

Philip

Hello,

As per my previous email to the list, I have now created the official RFC to deprecate calling json_serialize() on instances of classes marked with ZEND_ACC_NOT_SERIALIZABLE.

https://wiki.php.net/rfc/deprecate-json_encode-nonserializable

Sharing my experience, I never use json_encode on objects but on arrays (that are both JSON objects or JSON arrays).
When I see an object implementing JsonSerializable, I think it is the wrong approach, and I am usually able to find a better way.
Maybe if we could go back in time, we would not allow json_encode on an object, except if it implemented JsonSerializable, but that ship sailed long ago.

To your proposal, I think the BC break would be pretty big and I don't see a way it would pass.
On https://www.php.net/json_encode we can read:

If a value to be serialized is an object, then by default only publicly visible properties will be included.
So that's the expected behaviour.

Yes, you can say that encoding as JSON is just "another serialization method", but the default method in PHP, using json_encode()/json_decode(), is not symmetrical when using objects.
And here lies the difference with serialize()/unserialize(), as this one aims to be symmetrical, and where it can't be, it has a way to stop the serialization.

I am happy with the current way it works, getting an empty JSON object if there are no public properties for a Generator or Closure.
And I don't think having an error for them would improve the language in a meaningful way, and the BC break is not worth it.

Regards,
Alex

To add to this, we apparently use json_encode at work to serialize custom exceptions, which appears to work. This RFC would break that, I think.

— Rob

I would suggest we take a step back from this and look at it with a bit more of a wider lens. It seems to me that this would be a good place to have an attribute (e.g. #[NotSerializable] ) that could be defined for any class (with ZEND_ACC_NOT_SERIALIZABLE being automatically given this attribute)? It just seems to be a more holistic approach that makes sense, rather than basing it on internal engine stuff and/or limiting it to internal objects.

It's really the opposite: if we had an attribute, it would be used to toggle the existing internal flag.

ZEND_ACC_NOT_SERIALIZABLE is just one of a long list of class, property and method flags which the engine uses internally. You can see the full list here: https://heap.space/xref/php-src/Zend/zend_compile.h?r=58aa6fc8#212

You'll notice that includes things like "final" and "private", which refer to straight-forward syntax in the language, as well as completely internal details like "preloaded". It also includes ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES, which is the flag set if you add the #[AllowDynamicProperties] attribute on a class: https://heap.space/xref/php-src/Zend/zend_attributes.c?r=5a18279b#validate_allow_dynamic_properties

So if we had a #[NotSerializable] attribute (which I agree might be a good idea) it would be implemented by setting the ZEND_ACC_NOT_SERIALIZABLE flag on a class definition, and if this RFC passes, it would automatically apply to json_encode() as well as serialize().

Regards,

Rowan Tommins
[IMSoP]

Le 5 sept. 2024 à 18:03, John Coggeshall john@coggeshall.org a écrit :

As per my previous email to the list, I have now created the official RFC to deprecate calling json_serialize() on instances of classes marked with ZEND_ACC_NOT_SERIALIZABLE.

I would suggest we take a step back from this and look at it with a bit more of a wider lens. It seems to me that this would be a good place to have an attribute (e.g. #[NotSerializable] ) that could be defined for any class (with ZEND_ACC_NOT_SERIALIZABLE being automatically given this attribute)? It just seems to be a more holistic approach that makes sense, rather than basing it on internal engine stuff and/or limiting it to internal objects.

Coogle

Hi,

An attribute adds very little value. It doesn’t add new capability, because you can achieve the same effect with a serialiser that throws unconditionally; it is just a nicer syntax. People generally don’t bother making their classes unserialisable unless they have a good reason; having an attribute won’t really change that.

The core problem is that objects are json-serialisable by default, although most of them are not supposed to be json-serialised. Taking a second step back, if we really want to solve the issue, one should:

• for internal classes, determine which ones could be json-serialisable (at least, stdClass); for all other classes, json_encode(...) shall be disabled (after a deprecation period);
• for user-defined classes, the user shall opt into json-serialisability, either by extending a json-serialisable class, or by using an {attribute / magic method / interface} (chose your bikeshed colour).

—Claude

Hi

Am 2024-09-05 18:03, schrieb John Coggeshall:

I would suggest we take a step back from this and look at it with a bit
more of a wider lens. It seems to me that this would be a good place to
have an attribute (e.g. #[NotSerializable] ) that could be defined for
any class (with ZEND_ACC_NOT_SERIALIZABLE being automatically given
this attribute)? It just seems to be a more holistic approach that
makes sense, rather than basing it on internal engine stuff and/or
limiting it to internal objects.

FWIW There already is such an RFC:
https://wiki.php.net/rfc/not_serializable

Discussion thread is here: https://externals.io/message/121969#121969

Best regards
Tim Düsterhus