php-http / message

HTTP Message related tools
http://php-http.org
MIT License
1.29k stars 41 forks source link

DiactorosMessageFactory createResponse does not rewind strem if created with string body #108

Closed BigMichi1 closed 4 years ago

BigMichi1 commented 5 years ago
Q A
Bug? yes
New Feature? no
Version 1.7.2

Actual Behavior

$contents = (new DiactorosMessageFactory())->createResponse(200, null, [], "Hello")->getBody()->getContents();

$contents should be 'Hello', instead it is empty, it only works when a rewind is performed

$stream = (new DiactorosMessageFactory())->createResponse(200, null, [], "Hello")->getBody();
$stream->rewind();
$contents = $stream->getContents();

this is actually really hard to do in some situations when the response is used inside a framework wich uses php-http/message

Expected Behavior

$contents = (new DiactorosMessageFactory())->createResponse(200, null, [], "Hello")->getBody()->getContents();

$contents should be 'Hello'

Steps to Reproduce

simpliest way to reproduce is to use the one liner shown above.

actually i am facing this problem together with geocoder-php so when i create the HttpClient in the code i must add this plugin to the chain like

    $rewind = new class implements Plugin
    {
        /**
         * @inheritdoc
         */
        public function handleRequest(RequestInterface $request, callable $next, callable $first): Promise
        {
            return $next($request)->then(function (ResponseInterface $response) {
                $response->getBody()->rewind();
                return $response;
            }, function (\Exception $exception) {
                throw $exception;
            });
        }
    };
    $decoderPlugin = new DecoderPlugin(['use_content_encoding' => true]);
    return new PluginClient(HttpClientDiscovery::find(), [$rewind, $decoderPlugin]);

removing the $rewind from the plugin list the client will no longer work

Possible Solutions

see above

dbu commented 5 years ago

does this happen with other message implementations too, or only with diactoros? i wonder if the specification is unprecise on this or if this is a bug of diactoros?

dbu commented 5 years ago

discussion is going on here: https://groups.google.com/forum/m/#!topic/php-fig/S5YIw-Pu1yM

lets see what the outcome is and if we have to change something in the factory here.

Nyholm commented 5 years ago

I gave me 50 cents at the mailing group. I think this is a non issue. You should use __toString() instead of getContents() and you will always be fine.

sagikazarmark commented 5 years ago

I'm going to start a new website: dayzsincegetcontentscausedtrouble.com

It's going to be like "dayz since a new JS framework was born": constant zero.

dbu commented 4 years ago

am i correct in assuming that we can close the issue here, as it is not a problem of php-http/message? it seems something with diactoros vs the psr vs php streams being confusing?

Nyholm commented 4 years ago

The no PSR define if the stream should be rewinded or not. Implementations are not consistent and I dont see a reason for them to be.

If you want to start using a stream, you should never assume its state.