koemeet / JsonApiBundle

Integration of JSON API with Symfony using JMS Serializer
55 stars 26 forks source link

Serializer too less defensive in resource detection #32

Closed olivermack closed 7 years ago

olivermack commented 7 years ago

Hey,

found a small issue in the detection of serializer's detection for resources. Actually I came across this because I got a warning wrapped into an exception containing Warning: array_udiff(): Argument #2 is not an array (JsonApiSerializationVisitor:170).

The issue should be fairly easy to reproduce:

Here's the deal: if you submit a request and validate a form, the default JMSSerializer obviously scans recursively through the data (the form) and its children to extract all errors. Field errors are nested and other errors are then in the root of the validation result. However, the data that is available inside the JsonApiSerializationVisitor is an ArrayObject in cases of validation errors (thus it is NOT an array) which causes the current implementation of validateJsonApiDocument already to return true; which is not intended. The warning above comes from an inconsistent error handling (there's already a todo) which does not handle error-cases where only children have messages (so that the key errors is not defined in the root level). In fact this inconsistent handling is worth another github-issue but in my case it was just due to the fact that the visitor wanted to process data that should not be processed at all.

I submitted a PR (#31). As this caveat is in a quite central place of that tool please verify accordingly if the assumption and the fix is sane as there's no testsuite yet.

Cheers :)

RuslanZavacky commented 7 years ago

@koemeet looks like this can be closed, as #31 is merged.