Use full function namespaces to avoid clashes with serialization #179
Conversation
`Promise.php` contains function calls that expect the current namespace. However if you serialize & then later unserialize a Promise, this namespace is lost, and the function calls clash with other defined global functions. E.g. if using Laravel, you serialize a `\React\Promise\Promise` instance, and then later unserialize & attempt to resolve it - the call to `resolve` on line 232 clashes with Laravel's global `resolve` function - causing it to throw an exception. By using the full namespace on the function call this eliminates this possible problem. Also makes the code an easier read IMO making it obvious that these are namespaced functions.
|
@brentkelly Thanks for bringing this up, this is an interesting one! As discussed in #180, a promise should not usually be serialized because it contains transient state when it's still pending. On top of this, it looks like this changeset would introduce a partial patch to work around a limitation in Opis Closure only. In other words, this isn't required to address an issue in this library, but rather make sure another library can work better. I think the way forward here would be to report this upstream and then see if it makes sense to patch this project. That being said, I'm not opposed to getting this in once this is sorted out upstream. If you can come up with a way to reproduce this issue in this project and provide additional tests to highlight in which specific situation this is required, we can avoid future regressions and I'm happy to get this in. Thanks for looking into this! |
|
I'm closing this for now as it hasn't received any input in a while and I believe this has been answered. If you feel this is still relevant, please come back with more details and we can always reopen this 👍 Thank you for your effort! |
Promise.phpcontains function calls (resolveandrejectfromsrc/functions.php) that expect the current namespace. However if you serialize & then later unserialize aDeferredinstance, this namespace is lost, and the function calls clash with other defined global functions.E.g. if using Laravel, you configure & then serialize a
\React\Promise\Deferredinstance, and then later unserialize & attempt to resolve it - the call toresolveon line 232 clashes with Laravel's globalresolvefunction - causing it to throw an exception as it runs through the global Laravel function.Using Opis/Closure for serializing.
By using the full namespace on the function call this eliminates this possible problem. Also makes the code an easier read IMO making it obvious that these are namespaced (and not global) functions.