FOSRestBundle icon indicating copy to clipboard operation
FOSRestBundle copied to clipboard

RequestBodyParamConverter tries to convert GET requests

Open MichaelHindley opened this issue 8 years ago • 1 comments

In https://github.com/FriendsOfSymfony/FOSRestBundle/blob/80a41f571250ad4c14f7e36c267eb968575884d8/Request/RequestBodyParamConverter.php#L125-L128

the RequestBodyConverter supports basically every case.

However, GET requests don't have a body or a Content-Type header, and so this block https://github.com/FriendsOfSymfony/FOSRestBundle/blob/80a41f571250ad4c14f7e36c267eb968575884d8/Request/RequestBodyParamConverter.php#L91-L97

always fails when there is a GET request with this message:

The format "" is not supported for deserialization.

Since $request->getContentType() will always be an empty string in GET requests.

Since GET requests don't specify a Content-Type (https://tools.ietf.org/html/rfc7231#section-3.1.1.5) I feel this is an error, and the RequestBodyParamConverter should not "support" requests that are not POST/PUT/PATCH, or not support requests that have an empty Body.

Reproduce:

In config.yml:

body_converter: { enabled: true, validate: true }

    /**
     * @Rest\Get("/moo")
     */
    public function getCowSound()
    {
        return $this->view(["moo"], Response::HTTP_OK);
    }

MichaelHindley avatar Nov 28 '17 09:11 MichaelHindley

It would seem that this is fixed in 2.3 by introducing the extra check

&& 'fos_rest.request_body' === $configuration->getConverter()

giving the user granular control over when the request body parameter is used.

Is this correct?

MichaelHindley avatar Nov 28 '17 10:11 MichaelHindley