zendframework / zend-expressive-authentication-oauth2

OAuth2 (server) authentication middleware for PSR-7 applications.
BSD 3-Clause "New" or "Revised" License
35 stars 20 forks source link

Previous response bodies in $response->getBody()->getContent() #16

Closed wshafer closed 6 years ago

wshafer commented 6 years ago

Provide a narrative description of what you are trying to accomplish.

$this->responsePrototype is not building a new response object for each response. So using Swoole or PHPReact results in previous responses bleeding into each other.

Code to reproduce the issue

$response = $this-handle($request);
$response = $this-handle($request);
$response->getBody()->getContent();

Expected results

Only one response per request

Actual results

Mutiple response bodies per request

michalbundyra commented 6 years ago

@wshafer I think it's resolved with the lates release (5 hours ago). Can you confirm what version are you using?

/cc @weierophinney

wshafer commented 6 years ago

Nope. This is still pulling the Response from the container and not a new one for each response. So if a response has already been built, the previous response will be returned. Even though using a with{function}() returns a clone of the response, the initial stream is still used for each clone, so when you write() it just goes to the end of the existing stream.

weierophinney commented 6 years ago

@webimpress It's not resolved. The ResponsePrototypeTrait from zend-expressive-authentication pulls the ResponseInterface service, and returns it directly; if none is available, it returns a zend-diactoros response instance.

In the latter case, it's fine; it's returning a discrete response instance. However, in the case of the ResponseInterface service being present, it needs to read:

return $container->has(ResponseInterface::class)
    ? ($container->get(ResponseInterface::class)()
    : new Response();

That said, I'd argue that these adapters and zend-expressive-authentication should be composing a response factory instead of the prototype. That will also allow us to stop having the soft requirement on zend-diactoros, as it will be up to the consumer to supply the ResponseInterface service.

So, we need:

weierophinney commented 6 years ago

Actually... let's go with the following:

weierophinney commented 6 years ago

Fixed with #17.