Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle ObjectConstructor returning NULL #1172

Merged
merged 1 commit into from
Mar 21, 2020
Merged

Handle ObjectConstructor returning NULL #1172

merged 1 commit into from
Mar 21, 2020

Conversation

jankramer
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
Doc updated no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT

The DeserializationGraphNavigator calls the construct method on an instance of ObjectConstructorInterface. According to the return type hint of that method, it is allowed to return NULL. However, when this occurs, the navigator will pass NULL to the startVisitingObject method
of the visitor. startVisitingObject expects an object to be passed in, thus causing the serializer to crash with a type error.

We can prevent this error by calling the visitNull method if we detect that the ObjectConstructor has returned NULL instead of an object.

The DeserializationGraphNavigator calls the construct method on an
instance of ObjectConstructorInterface. According to the return type
hint of that method, it is allowed to return NULL. However, when this
occurs, the navigator will pass NULL to the startVisitingObject method
of the visitor. startVisitingObject expects an object to be passed in,
thus causing the serializer to crash with a type error.

We can prevent this error by calling the `visitNull` method if we detect
that the ObjectConstructor has returned NULL instead of an object.
@goetas
Copy link
Collaborator

goetas commented Mar 15, 2020

Hi, thanks for your contribution.
I did not realize that the signature was allowing NULL.... in my opinion should not happen... but changing that now will result in a BC break.

@jankramer
Copy link
Contributor Author

jankramer commented Mar 15, 2020

Thanks for your quick reply!

Why do you reckon it shouldn't happen? The DoctrineObjectConstructor explicitly defines a NULL strategy, which seems to have valid use cases. At least it does in the application where I encountered this situation.

As it stands now, the NULL strategy is the default strategy, but breaks when encountered at run-time. Are you open to merging this fix, or do you have a suggestion for a different approach to fixing this issue?

@goetas
Copy link
Collaborator

goetas commented Mar 15, 2020

I'm interested in merging this, was just hoping there was a better solution 😁

In the next week will test it and if is OK, do the merge

@jankramer
Copy link
Contributor Author

Gotcha, thanks!

@goetas goetas merged commit 054c136 into schmittjoh:master Mar 21, 2020
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