In the aftermath of an internal refactoring of an internal code-base to turn something eager into something more lazy using Generators, it occurred to me that while PHP refuses to serialize() a Generator, accidentally running json_encode() over a Generator produces the worst possible result:
It silently encodes generators as {}.
I would like to write an RFC to first deprecating and later disallowing to json_encode Generators.
If you think this is a worthwhile thing to do an RFC for, I would kindly ask for somebody to grant me RFC karma, so I can start working on one.
On Aug 30, 2024 at 09:42:05, John Coggeshall <john@coggenterprises.com>
wrote:
This seems entirely reasonable and I would welcome an RFC for it (unless
someone has a better idea as to how to handle this case).
The other options I considered were to either materialize the Generator,
but given that they can run indefinitely this seems very dangerous or
json-encoding them as but that would be lying in most cases, so doing
what serialize() does felt like the only sensible option and would prevent
a nasty case of silent failure.
Le 30 août 2024 à 09:36, Philip Hofstetter <phofstetter@sensational.ch> a écrit :
Hello,
In the aftermath of an internal refactoring of an internal code-base to turn something eager into something more lazy using Generators, it occurred to me that while PHP refuses to serialize() a Generator, accidentally running json_encode() over a Generator produces the worst possible result:
It silently encodes generators as {}.
I would like to write an RFC to first deprecating and later disallowing to json_encode Generators.
Hi,
Note that the issue is not limited to Generators. For instance a Closure object is also json-encodable but not serialisable.
IMO, every internal class (not just Generator) that is marked as non-serialisable, should also be non-json-encodable.
Note that the issue is not limited to Generators. For instance a Closure object is also json-encodable but not serialisable.
IMO, every internal class (not just Generator) that is marked as non-serialisable, should also be non-json-encodable.
Thank you for brining this up.
When working on the implementation to link to the RFC, I came across ZEND_ACC_NOT_SERIALIZABLE and its usage with serialize() and it occurred to me that there’s probably some sense in making json_encode look at that flag in general.
Of course this would greatly increase the amount of possible deprecation warnings thrown and other objects flagged with ZEND_ACC_NOT_SERIALIZABLE (resource handles, reflection, and, yes, Closure) are not as likely to cause harm as Generators do because refactoring something from arrays to Generators is a relatively common operation which also is mostly transparent to the majority of callers, so a deprecation warning and later error will provide actual value in this case whereas json_encode() over an accidentally present, say, CurlHandle is of no consequence for a user, so little value is provided by the eventually thrown exception.
Implementation-wise, I can say that using ZEND_ACC_NOT_SERIALIZABLE is super simple and turning this into a 2 line patch (plus a few lines of test code), but I’m a bit concerned about the large BC surface, even though I do agree that conceptionally ZEND_ACC_NOT_SERIALIZABLE should be used.
I will write the RFC based on ZEND_ACC_NOT_SERIALIZABLE and if it turns out in discussion or in the voting phase that the BC concerns are too grave, I will find and propose an implementation that’s more specific to Generators.