From ba5b16e34664898e45e29c0f618121eaf9d7a85e Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 23 Aug 2017 16:37:53 -0500 Subject: [PATCH 1/4] Removes output buffer awareness from Server, emitters By removing a call to `ob_start()` within `Server::listen()`, we can solve the issue of detecting when we have both a response and content in the output buffer; in most cases, we will have already sent headers, which will cause an exception to be raised; we can also check the _current_ output buffer and, if non-empty, raise an exception. This means we can: - Remove the `SapiEmitterTrait::flush()` implementation, and all calls to it. - Remove the `$maxBufferLevel` argument to each emitter. - Remove tests regarding interactions of emitters with the output buffer. This is a backwards-incompatible change. However, it fixes a rather tricky problem that occurs currently when mixing buffered output and response instances. --- src/Response/SapiEmitter.php | 8 +--- src/Response/SapiEmitterTrait.php | 38 ++++++++++--------- src/Response/SapiStreamEmitter.php | 8 +--- src/Server.php | 10 +---- test/Response/SapiEmitterTest.php | 22 ----------- test/Response/SapiStreamEmitterTest.php | 6 +-- test/ServerIntegrationTest.php | 49 ------------------------- test/ServerTest.php | 6 --- 8 files changed, 30 insertions(+), 117 deletions(-) delete mode 100644 test/ServerIntegrationTest.php diff --git a/src/Response/SapiEmitter.php b/src/Response/SapiEmitter.php index a9613429..ab1d7e08 100644 --- a/src/Response/SapiEmitter.php +++ b/src/Response/SapiEmitter.php @@ -23,19 +23,15 @@ class SapiEmitter implements EmitterInterface * body content via the output buffer. * * @param ResponseInterface $response - * @param null|int $maxBufferLevel Maximum output buffering level to unwrap. */ - public function emit(ResponseInterface $response, $maxBufferLevel = null) + public function emit(ResponseInterface $response) { - if (headers_sent()) { - throw new RuntimeException('Unable to emit response; headers already sent'); - } + $this->checkForPreviousOutput(); $response = $this->injectContentLength($response); $this->emitStatusLine($response); $this->emitHeaders($response); - $this->flush($maxBufferLevel); $this->emitBody($response); } diff --git a/src/Response/SapiEmitterTrait.php b/src/Response/SapiEmitterTrait.php index 1e1667ae..dd4a6508 100644 --- a/src/Response/SapiEmitterTrait.php +++ b/src/Response/SapiEmitterTrait.php @@ -10,9 +10,30 @@ namespace Zend\Diactoros\Response; use Psr\Http\Message\ResponseInterface; +use RuntimeException; trait SapiEmitterTrait { + /** + * Checks to see if content has previously been sent. + * + * If either headers have been sent, or the current output buffer contains + * content, raises an exception. + * + * @throws RuntimeException if headers have already been sent. + * @throws RuntimeException if the current output buffer is not empty. + */ + private function checkForPreviousOutput() + { + if (headers_sent()) { + throw new RuntimeException('Unable to emit response; headers already sent'); + } + $bufferContents = ob_get_contents(); + if (! empty($bufferContents)) { + throw new RuntimeException('Output has been emitted previously; cannot emit response: ' . $bufferContents); + } + } + /** * Inject the Content-Length header if is not already present. * @@ -77,23 +98,6 @@ private function emitHeaders(ResponseInterface $response) } } - /** - * Loops through the output buffer, flushing each, before emitting - * the response. - * - * @param int|null $maxBufferLevel Flush up to this buffer level. - */ - private function flush($maxBufferLevel = null) - { - if (null === $maxBufferLevel) { - $maxBufferLevel = ob_get_level(); - } - - while (ob_get_level() > $maxBufferLevel) { - ob_end_flush(); - } - } - /** * Filter a header name to wordcase * diff --git a/src/Response/SapiStreamEmitter.php b/src/Response/SapiStreamEmitter.php index 1804da4f..7f95c824 100644 --- a/src/Response/SapiStreamEmitter.php +++ b/src/Response/SapiStreamEmitter.php @@ -24,20 +24,16 @@ class SapiStreamEmitter implements EmitterInterface * body content via the output buffer. * * @param ResponseInterface $response - * @param null|int $maxBufferLevel Maximum output buffering level to unwrap. * @param int $maxBufferLength Maximum output buffering size for each iteration */ - public function emit(ResponseInterface $response, $maxBufferLevel = null, $maxBufferLength = 8192) + public function emit(ResponseInterface $response, $maxBufferLength = 8192) { - if (headers_sent()) { - throw new RuntimeException('Unable to emit response; headers already sent'); - } + $this->checkForPreviousOutput(); $response = $this->injectContentLength($response); $this->emitStatusLine($response); $this->emitHeaders($response); - $this->flush($maxBufferLevel); $range = $this->parseContentRange($response->getHeaderLine('Content-Range')); diff --git a/src/Server.php b/src/Server.php index 768d7f38..43c4432d 100644 --- a/src/Server.php +++ b/src/Server.php @@ -150,24 +150,18 @@ public static function createServerFromRequest( * If provided a $finalHandler, that callable will be used for * incomplete requests. * - * Output buffering is enabled prior to invoking the attached - * callback; any output buffered will be sent prior to any - * response body content. - * * @param null|callable $finalHandler */ public function listen(callable $finalHandler = null) { $callback = $this->callback; - ob_start(); - $bufferLevel = ob_get_level(); - $response = $callback($this->request, $this->response, $finalHandler); if (! $response instanceof ResponseInterface) { $response = $this->response; } - $this->getEmitter()->emit($response, $bufferLevel); + + $this->getEmitter()->emit($response); } /** diff --git a/test/Response/SapiEmitterTest.php b/test/Response/SapiEmitterTest.php index 62aa6320..7bc9e3e7 100644 --- a/test/Response/SapiEmitterTest.php +++ b/test/Response/SapiEmitterTest.php @@ -13,26 +13,4 @@ class SapiEmitterTest extends AbstractEmitterTest { - public function testEmitsBufferLevel() - { - ob_start(); - echo "level" . ob_get_level() . " "; // 2 - ob_start(); - echo "level" . ob_get_level() . " "; // 3 - ob_start(); - echo "level" . ob_get_level() . " "; // 4 - $response = (new Response()) - ->withStatus(200) - ->withAddedHeader('Content-Type', 'text/plain'); - $response->getBody()->write('Content!'); - ob_start(); - $this->emitter->emit($response); - $this->assertEquals('Content!', ob_get_contents()); - ob_end_clean(); - $this->assertEquals('level4 ', ob_get_contents(), 'current buffer level string must remains after emit'); - ob_end_clean(); - $this->emitter->emit($response, 2); - $this->assertEquals('level2 level3 Content!', ob_get_contents(), 'must buffer until specified level'); - ob_end_clean(); - } } diff --git a/test/Response/SapiStreamEmitterTest.php b/test/Response/SapiStreamEmitterTest.php index fe401755..72443eed 100644 --- a/test/Response/SapiStreamEmitterTest.php +++ b/test/Response/SapiStreamEmitterTest.php @@ -210,7 +210,7 @@ function ($bufferLength) use (& $peakBufferLength) { ->withBody($stream->reveal()); ob_start(); - $this->emitter->emit($response, null, $maxBufferLength); + $this->emitter->emit($response, $maxBufferLength); $emittedContents = ob_get_clean(); if ($seekable) { @@ -351,7 +351,7 @@ function ($bufferLength) use (& $peakBufferLength) { ->withBody($stream->reveal()); ob_start(); - $this->emitter->emit($response, null, $maxBufferLength); + $this->emitter->emit($response, $maxBufferLength); $emittedContents = ob_get_clean(); $stream->rewind()->shouldNotBeCalled(); @@ -497,7 +497,7 @@ function () use (& $closureTrackMemoryUsage) { gc_disable(); - $this->emitter->emit($response, null, $maxBufferLength); + $this->emitter->emit($response, $maxBufferLength); ob_end_flush(); diff --git a/test/ServerIntegrationTest.php b/test/ServerIntegrationTest.php deleted file mode 100644 index 8f83b872..00000000 --- a/test/ServerIntegrationTest.php +++ /dev/null @@ -1,49 +0,0 @@ -prophesize(ServerRequestInterface::class)->reveal(); - $response = $this->prophesize(ResponseInterface::class)->reveal(); - - $emitter = $this->prophesize(SapiStreamEmitter::class); - $emitter - ->emit( - $response, - $currentObLevel + 1 - ) - ->shouldBeCalled(); - - $middleware = function ($req, $res) use ($request, $response) { - TestCase::assertSame($request, $req); - TestCase::assertSame($response, $res); - return $res; - }; - - $server = new Server( - $middleware, - $request, - $response - ); - $server->setEmitter($emitter->reveal()); - $server->listen(); - - ob_end_clean(); - } -} diff --git a/test/ServerTest.php b/test/ServerTest.php index 0fab8bf8..c9bf5297 100644 --- a/test/ServerTest.php +++ b/test/ServerTest.php @@ -109,7 +109,6 @@ public function testEmitterSetter() $this->expectOutputString(''); $server->listen(); - ob_end_flush(); } public function testCreateServerWillCreateDefaultInstancesForRequestAndResponse() @@ -153,7 +152,6 @@ public function testListenInvokesCallbackAndSendsResponse() $this->expectOutputString('FOOBAR'); $server->listen(); - ob_end_flush(); $this->assertContains('HTTP/1.1 200 OK', HeaderStack::stack()); $this->assertContains('Content-Type: text/plain', HeaderStack::stack()); @@ -179,7 +177,6 @@ public function testListenEmitsStatusHeaderWithoutReasonPhraseIfNoReasonPhrase() $this->expectOutputString('FOOBAR'); $server->listen(); - ob_end_flush(); $this->assertContains('HTTP/1.1 299', HeaderStack::stack()); $this->assertContains('Content-Type: text/plain', HeaderStack::stack()); @@ -204,7 +201,6 @@ public function testEnsurePercentCharactersDoNotResultInOutputError() $this->expectOutputString('100%'); $server->listen(); - ob_end_flush(); $this->assertContains('HTTP/1.1 200 OK', HeaderStack::stack()); $this->assertContains('Content-Type: text/plain', HeaderStack::stack()); @@ -233,7 +229,6 @@ public function testEmitsHeadersWithMultipleValuesMultipleTimes() $server = Server::createServer($callback, $server, [], [], [], []); $server->listen(); - ob_end_flush(); $this->assertContains('HTTP/1.1 200 OK', HeaderStack::stack()); $this->assertContains('Content-Type: text/plain', HeaderStack::stack()); @@ -311,7 +306,6 @@ public function testListenPassesCallableArgumentToCallback() $this->response ); $server->listen($final); - ob_end_flush(); $this->assertTrue($invoked); } } From 0725467882508ea18b13db8c8d23b7dc803e402f Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Mon, 11 Sep 2017 14:21:10 -0500 Subject: [PATCH 2/4] Removes Content-Length injection from SAPI stream emitters Per #251, this patch removes Content-Length injection from SAPI stream emitters. It also renames the `checkForPreviousOutput()` method to `assertNoPreviousOutput()` to make it more clear that the method is a no-op under proper usage. The `assertNoPreviousOutput()` method also borrows an idea from the comment to the issue, indicating we can check for (1) a non-zero output buffer, and (2) use `ob_get_length()` to determine if it has content. --- src/Response/SapiEmitter.php | 4 +--- src/Response/SapiEmitterTrait.php | 31 ++++++--------------------- src/Response/SapiStreamEmitter.php | 5 +---- test/Response/AbstractEmitterTest.php | 1 - test/ServerTest.php | 1 - 5 files changed, 8 insertions(+), 34 deletions(-) diff --git a/src/Response/SapiEmitter.php b/src/Response/SapiEmitter.php index ab1d7e08..45c97448 100644 --- a/src/Response/SapiEmitter.php +++ b/src/Response/SapiEmitter.php @@ -26,9 +26,7 @@ class SapiEmitter implements EmitterInterface */ public function emit(ResponseInterface $response) { - $this->checkForPreviousOutput(); - - $response = $this->injectContentLength($response); + $this->assertNoPreviousOutput(); $this->emitStatusLine($response); $this->emitHeaders($response); diff --git a/src/Response/SapiEmitterTrait.php b/src/Response/SapiEmitterTrait.php index dd4a6508..d4c9c76a 100644 --- a/src/Response/SapiEmitterTrait.php +++ b/src/Response/SapiEmitterTrait.php @@ -17,40 +17,21 @@ trait SapiEmitterTrait /** * Checks to see if content has previously been sent. * - * If either headers have been sent, or the current output buffer contains - * content, raises an exception. + * If either headers have been sent or the output buffer contains content, + * raises an exception. * * @throws RuntimeException if headers have already been sent. - * @throws RuntimeException if the current output buffer is not empty. + * @throws RuntimeException if output is present in the output buffer. */ - private function checkForPreviousOutput() + private function assertNoPreviousOutput() { if (headers_sent()) { throw new RuntimeException('Unable to emit response; headers already sent'); } - $bufferContents = ob_get_contents(); - if (! empty($bufferContents)) { - throw new RuntimeException('Output has been emitted previously; cannot emit response: ' . $bufferContents); - } - } - /** - * Inject the Content-Length header if is not already present. - * - * @param ResponseInterface $response - * @return ResponseInterface - */ - private function injectContentLength(ResponseInterface $response) - { - if (! $response->hasHeader('Content-Length')) { - // PSR-7 indicates int OR null for the stream size; for null values, - // we will not auto-inject the Content-Length. - if (null !== $response->getBody()->getSize()) { - return $response->withHeader('Content-Length', (string) $response->getBody()->getSize()); - } + if (ob_get_level() > 0 && ob_get_length() > 0) { + throw new RuntimeException('Output has been emitted previously; cannot emit response'); } - - return $response; } /** diff --git a/src/Response/SapiStreamEmitter.php b/src/Response/SapiStreamEmitter.php index 7f95c824..3f61d66c 100644 --- a/src/Response/SapiStreamEmitter.php +++ b/src/Response/SapiStreamEmitter.php @@ -28,10 +28,7 @@ class SapiStreamEmitter implements EmitterInterface */ public function emit(ResponseInterface $response, $maxBufferLength = 8192) { - $this->checkForPreviousOutput(); - - $response = $this->injectContentLength($response); - + $this->assertNoPreviousOutput(); $this->emitStatusLine($response); $this->emitHeaders($response); diff --git a/test/Response/AbstractEmitterTest.php b/test/Response/AbstractEmitterTest.php index 6c3de377..46532bb8 100644 --- a/test/Response/AbstractEmitterTest.php +++ b/test/Response/AbstractEmitterTest.php @@ -44,7 +44,6 @@ public function testEmitsResponseHeaders() ob_end_clean(); $this->assertContains('HTTP/1.1 200 OK', HeaderStack::stack()); $this->assertContains('Content-Type: text/plain', HeaderStack::stack()); - $this->assertContains('Content-Length: 8', HeaderStack::stack()); } public function testEmitsMessageBody() diff --git a/test/ServerTest.php b/test/ServerTest.php index c9bf5297..2abf7ffb 100644 --- a/test/ServerTest.php +++ b/test/ServerTest.php @@ -251,7 +251,6 @@ public function testEmitsHeadersWithMultipleValuesMultipleTimes() */ public function testHeaderOrderIsHonoredWhenEmitted($stack) { - array_pop($stack); // ignore "Content-Length" automatically set by the response emitter $header = array_pop($stack); $this->assertContains( 'Set-Cookie: bar=baz; expires=Wed, 8 Oct 2014 10:30; path=/foo/bar; domain=example.com', From 755ded6c0d6936e24950dccc5e9b40260b422642 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Mon, 11 Sep 2017 15:08:41 -0500 Subject: [PATCH 3/4] Adds CHANGELOG for #270 --- CHANGELOG.md | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5672f914..37eea6ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,23 @@ All notable changes to this project will be documented in this file, in reverse ### Changed -- Nothing. +- [#270](https://github.com/zendframework/zend-diactoros/pull/270) changes the + behavior of `Zend\Diactoros\Server`: it no longer creates an output buffer. + +- [#270](https://github.com/zendframework/zend-diactoros/pull/270) changes the + behavior of the two SAPI emitters in two backwards-incompatible ways: + + - They no longer auto-inject a `Content-Length` header. If you need this + functionality, zendframework/zend-expressive-helpers 4.1+ provides it via + `Zend\Expressive\Helper\ContentLengthMiddleware`. + + - They no longer flush the output buffer. Instead, if headers have been sent, + or the output buffer exists and has a non-zero length, the emitters raise an + exception, as mixed PSR-7/output buffer content creates a blocking issue. + If you are emitting content via `echo`, `print`, `var_dump`, etc., or not + catching PHP errors or exceptions, you will need to either fix your + application to always work with a PSR-7 response, or provide your own + emitters that allow mixed output mechanisms. ### Deprecated From a22b27315a146727df1a4b755caefc60abca8d2e Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Mon, 11 Sep 2017 15:25:37 -0500 Subject: [PATCH 4/4] Updates documentation of emitters - Ensures they do not discuss output buffering. - Adds section on `SapiStreamEmitter`. --- doc/book/emitting-responses.md | 53 +++++++++++++++++++++++++++++++--- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/doc/book/emitting-responses.md b/doc/book/emitting-responses.md index 7715601a..897b64ae 100644 --- a/doc/book/emitting-responses.md +++ b/doc/book/emitting-responses.md @@ -3,10 +3,15 @@ If you are using a non-SAPI PHP implementation and wish to use the `Server` class, or if you do not want to use the `Server` implementation but want to emit a response, this package provides an interface, `Zend\Diactoros\Response\EmitterInterface`, defining a method `emit()` for emitting the -response. A single implementation is currently available, `Zend\Diactoros\Response\SapiEmitter`, -which will use the native PHP functions `header()` and `echo` in order to emit the response. If you -are using a non-SAPI implementation, you will need to create your own `EmitterInterface` -implementation. +response. + +Diactoros provides two implementations currently, both for working with +traditional Server API (SAPI) implementations: `Zend\Diactoros\Response\SapiEmitter` +and `Zend\Diactoros\Response\SapiStreamEmitter`. Each uses the native `header()` +PHP function to emit headers, and `echo()` to emit the response body. + +If you are using a non-SAPI implementation, you will need to create your own +`EmitterInterface` implementation. For example, the `SapiEmitter` implementation of the `EmitterInterface` can be used thus: @@ -16,3 +21,43 @@ $response->getBody()->write("some content\n"); $emitter = new Zend\Diactoros\Response\SapiEmitter(); $emitter->emit($response); ``` + +## Emitting ranges of streamed files + +The `SapiStreamEmitter` is useful when you want to emit a `Content-Range`. As an +example, to stream a range of bytes from a file to a client, the client can pass +the following header: + +```http +Range: bytes=1024-2047 +``` + +Your application would then populate the response with a `Content-Range` header: + +```php +$range = $request->getHeaderLine('range'); +$range = str_replace('=', ' ', $range); + +$body = new Stream($pathToFile); +$size = $body->getSize(); +$range .= '/' . $size; + +$response = new Response($body); +$response = $response->withHeader('Content-Range', $range); +``` + +> Note: you will likely want to ensure the range specified falls within the +> content size of the streamed body! + +The `SapiStreamEmitter` detects the `Content-Range` header and emits only the +bytes specified. + +```php +$emitter = new SapiStreamEmitter(); +$emitter->emit($response); +``` + +The `SapiStreamEmitter` may be used in place of the `SapiEmitter`, even when not +sending files. However, unlike the `SapiEmitter`, it will emit a chunk of +content at a time instead of the full content at once, which could lead to +performance overhead. The default chunk size is 8192 bytes.