slimphp / Slim

Slim is a PHP micro framework that helps you quickly write simple yet powerful web applications and APIs.
http://slimframework.com
MIT License
11.98k stars 1.95k forks source link

Response withJSON method overwriting not replacing body #1818

Closed AmibeWebsites closed 8 years ago

AmibeWebsites commented 8 years ago

Should rewind completely replace the response body?

I'm using withJSON and is seems that instead of replacing the body it's overwriting, and presume fopen uses r+ instead of w+.

return $response->withJSON('abcd')->withJSON('xyz');
// returns 'xyzd'
AmibeWebsites commented 8 years ago

Looking at the code, rewind is doing exactly what it's supposed to do: "Seek to the beginning of the stream."

So if the intention of the Response withJSON method is to replace the response body (as I believe it is) then it's not working as intended.

So perhaps replace:

        $body = $this->getBody();
        $body->rewind();

with

        $this->body = $body = new Body(fopen('php://temp', 'w+'));

Not sure if anything would be lost this way, like body meta, that should be added back in?

And should clone be used instead, like this?

        $clone = clone $this;
        $clone->body = new Body(fopen('php://temp', 'w+'));
        $clone->body->write($json = json_encode($data, $encodingOptions));
geggleto commented 8 years ago

Well you can do ->withBody() ... https://github.com/slimphp/Slim/blob/3.x/Slim/Http/Message.php#L287-L294

atraianovschi commented 8 years ago

Hello,

I don't see why this issue was closed because the current method ->withJson() is just overwriting the stream and not replacing the current body with the new JSON. And I really think this is the approach that everyone is expecting from this method.

For example I have a middleware that should filter the response based on some rights. The ->withJson() method works perfectly when I create the response and when it goes to the middleware I should filter that response and replace it with the filtered version of it, with same method ->withJson().

If the method withBody is replacing the content, and withStatus is also replacing the status, why withJson should not replace the content ?

akrabat commented 8 years ago

I agree with @deddus - withJson should instantiate a new Body as the current Body could be anything…

robertprice commented 8 years ago

I've put a pull request in for this. The code is along the lines @dudewithamood suggested.