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

Using validateJsonBody() before getResource() makes request body empty #93

Closed
Doqnach opened this issue Dec 23, 2019 · 3 comments
Closed
Labels

Comments

@Doqnach
Copy link

Doqnach commented Dec 23, 2019

What did I do?

Using qpautrat/woohoolabs-yin-bundle, which provides JsonApi $jsonApi, in Symfony:

<?php
// {"data":[{"id":"my-id1","type":"MyResource","attributes":{"a":1}}]}
final class MyController extends AbstractController
{
    public function myAction(JsonApi $jsonApi): ResponseInterface
    {
        try {
            $requestValidator = new RequestValidator(new DefaultExceptionFactory());
            $requestValidator->negotiate($jsonApi->request);
            $requestValidator->validateQueryParams($jsonApi->request);
            $requestValidator->validateJsonBody($jsonApi->request);
        } catch (MediaTypeUnsupported $e) {
            return $jsonApi->respond()->genericError($e->getErrorDocument(), Response::HTTP_NOT_ACCEPTABLE);
        } catch (QueryParamUnrecognized|RequestBodyInvalidJson|JsonApiExceptionInterface $e) {
            return $jsonApi->respond()->genericError($e->getErrorDocument(), Response::HTTP_BAD_REQUEST);
        }

        dd(
            $jsonApi->request->getResource()
        );
    }
}

What did I expect to happen:
Outputs the "data":

array:1 [
  0 => array:3 [
    "id" => "my-id1"
    "type" => "MyResource"
    "attributes" => array:1 [
      "a" => 1
    ]
  ]
]

What actually happens:

null

Workaround:

<?php
final class MyController extends AbstractController
{
    public function myAction(JsonApi $jsonApi): ResponseInterface
    {
        try {
            $requestValidator = new RequestValidator(new DefaultExceptionFactory());
            $requestValidator->negotiate($jsonApi->request);
            $requestValidator->validateQueryParams($jsonApi->request);

            // if you don't do this, the validation mangles the body
            $jsonApi->request->getParsedBody();
            $requestValidator->validateJsonBody($jsonApi->request);
        } catch (MediaTypeUnsupported $e) {
            return $jsonApi->respond()->genericError($e->getErrorDocument(), Response::HTTP_NOT_ACCEPTABLE);
        } catch (QueryParamUnrecognized|RequestBodyInvalidJson|JsonApiExceptionInterface $e) {
            return $jsonApi->respond()->genericError($e->getErrorDocument(), Response::HTTP_BAD_REQUEST);
        }

        dd(
            $jsonApi->request->getResource()
        );
    }
}

The cause here is in WoohooLabs\Yin\JsonApi\Negotiation\RequestValidator::validateJsonBody() doing:

        $errorMessage = $this->validateJsonMessage($request->getBody()->__toString());

This is called without deserializing the body Stream using WoohooLabs\Yin\JsonApi\Serializer\JsonDeserializer. Because the stream is based on php://input, once it was read, you never get those bytes back (without maybe rewinding... but I haven't got that to work). So if you then try to call $jsonApi->request->getParsedBody() or $jsonApi->request->getResource(), this will then give you no content.

First filed at qpautrat/woohoolabs-yin-bundle (qpautrat/woohoolabs-yin-bundle#19) but deemed an upstream issue.

@kocsismate
Copy link
Member

kocsismate commented Dec 29, 2019

Hi @Doqnach ,

Thanks for the report, it certainly is a bug.

Yin unfortunately can't use the deserializer in this case because we are trying to provide error (debug) information about a possibly malformed JSON content. However, the problem could be mitigated by doing something similar in RequestValidator::validateJsonBody() to what is done by JsonSerializer: https://github.com/woohoolabs/yin/blob/master/src/JsonApi/Serializer/JsonSerializer.php#L31

I think this is the right way to fix your problem. What do you think about it?

@Doqnach
Copy link
Author

Doqnach commented Dec 30, 2019

That looks like a good solution to me!
I tried the rewind at some points in the code but that wasn't yet giving me the right behaviour yet, so hopefully doing it at that point in the above way will solve it.

@kocsismate
Copy link
Member

I believe it will help :) So I'm releasing a new version now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants