Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace addContentLengthHeader with ContentLength Middleware #2254

Merged
merged 5 commits into from Aug 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@

### Added

- [#2254](https://github.com/slimphp/Slim/pull/2254) Added Middleware\ContentLengthMiddleware
- [#2166](https://github.com/slimphp/Slim/pull/2166) Added Middleware\OutputBufferingMiddleware

### Deprecated
Expand All @@ -13,6 +14,8 @@

### Removed

- [#2254](https://github.com/slimphp/Slim/pull/2254) `addContentLengthHeader` setting is removed
- [#2166](https://github.com/slimphp/Slim/pull/2166) `outputBuffering` setting is removed
- [#2067](https://github.com/slimphp/Slim/pull/2067) Remove App::VERSION
- [#2078](https://github.com/slimphp/Slim/pull/2078) Remove App::subRequest()
- [#2098](https://github.com/slimphp/Slim/pull/2098) Remove CallableResolverTrait
Expand Down
13 changes: 0 additions & 13 deletions Slim/App.php
Expand Up @@ -82,7 +82,6 @@ class App
'responseChunkSize' => 4096,
'determineRouteBeforeAppMiddleware' => false,
'displayErrorDetails' => false,
'addContentLengthHeader' => true,
'routerCacheFile' => false,
];

Expand Down Expand Up @@ -808,18 +807,6 @@ protected function finalize(ResponseInterface $response)
return $response->withoutHeader('Content-Type')->withoutHeader('Content-Length');
}

// Add Content-Length header if `addContentLengthHeader` setting is set
if ($this->getSetting('addContentLengthHeader') == true) {
if (ob_get_length() > 0) {
throw new \RuntimeException("Unexpected data in output buffer. " .
"Maybe you have characters before an opening <?php tag?");
}
$size = $response->getBody()->getSize();
if ($size !== null && !$response->hasHeader('Content-Length')) {
$response = $response->withHeader('Content-Length', (string) $size);
}
}

return $response;
}

Expand Down
1 change: 0 additions & 1 deletion Slim/Container.php
Expand Up @@ -53,7 +53,6 @@ class Container extends PimpleContainer implements ContainerInterface
'outputBuffering' => 'append',
'determineRouteBeforeAppMiddleware' => false,
'displayErrorDetails' => false,
'addContentLengthHeader' => true,
'routerCacheFile' => false,
];

Expand Down
30 changes: 30 additions & 0 deletions Slim/Middleware/ContentLengthMiddleware.php
@@ -0,0 +1,30 @@
<?php
namespace Slim\Middleware;

use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Message\ResponseInterface;
use Slim\Http\Body;

class ContentLengthMiddleware
{
/**
* Invoke
*
* @param ServerRequestInterface $request PSR7 server request
* @param ResponseInterface $response PSR7 response
* @param callable $next Middleware callable
* @return ResponseInterface PSR7 response
*/
public function __invoke(ServerRequestInterface $request, ResponseInterface $response, callable $next)
{
$response = $next($request, $response);

// Add Content-Length header if not already added
$size = $response->getBody()->getSize();
if ($size !== null && !$response->hasHeader('Content-Length')) {
$response = $response->withHeader('Content-Length', (string) $size);
}

return $response;
}
}
2 changes: 2 additions & 0 deletions UPGRADING.md
@@ -1,9 +1,11 @@
# How to upgrade

* [2254] - You need to add the `Middleware\ContentLengthMiddleware` middleware if you want Slim to add the Content-Length header this automatically.
* [2166] - You need to add the `Middleware\OutputBufferingMiddleware` middleware to capture echo'd or var_dump'd output from your code.
* [2098] - You need to add the App's router to the container for a straight upgrade. If you've created your own router factory in the container though, then you need to set it into the $app.
* [2102] - You must inject custom route invocation strategy with `$app->getRouter()->setDefaultInvocationStrategy($myStrategy)`

[2254]: https://github.com/slimphp/Slim/pull/2254
[2166]: https://github.com/slimphp/Slim/pull/2166
[2098]: https://github.com/slimphp/Slim/pull/2098
[2102]: https://github.com/slimphp/Slim/pull/2102
43 changes: 4 additions & 39 deletions tests/AppTest.php
Expand Up @@ -79,14 +79,14 @@ public function testAddSettings()
{
$app = new App();
$app->addSettings(['foo' => 'bar']);
$this->assertAttributeContains('foo', 'settings', $app);
$this->assertAttributeContains('bar', 'settings', $app);
}

public function testAddSetting()
{
$app = new App();
$app->addSetting('foo', 'bar');
$this->assertAttributeContains('foo', 'settings', $app);
$this->assertAttributeContains('bar', 'settings', $app);
}

/********************************************************************************
Expand Down Expand Up @@ -1953,11 +1953,11 @@ public function testFinalize()

$response = new Response();
$response->getBody()->write('foo');
$response = $response->withHeader('Content-Type', 'text/plain');

$response = $method->invoke(new App(), $response);

$this->assertTrue($response->hasHeader('Content-Length'));
$this->assertEquals('3', $response->getHeaderLine('Content-Length'));
$this->assertTrue($response->hasHeader('Content-Type'));
}

public function testFinalizeWithoutBody()
Expand Down Expand Up @@ -2027,41 +2027,6 @@ public function testCallingAnUncallableContainerKeyThrows()
$app->foo('bar');
}

public function testOmittingContentLength()
{
$method = new \ReflectionMethod('Slim\App', 'finalize');
$method->setAccessible(true);

$response = new Response();
$response->getBody()->write('foo');

$app = new App();
$app->addSetting('addContentLengthHeader', false);
$response = $method->invoke($app, $response);

$this->assertFalse($response->hasHeader('Content-Length'));
}

/**
* @expectedException \RuntimeException
* @expectedExceptionMessage Unexpected data in output buffer
*/
public function testForUnexpectedDataInOutputBuffer()
{
$this->expectOutputString('test'); // needed to avoid risky test warning
echo "test";
$method = new \ReflectionMethod('Slim\App', 'finalize');
$method->setAccessible(true);

$response = new Response();
$response->getBody()->write('foo');

$app = new App();
$container = $app->getContainer();
$container['settings']['addContentLengthHeader'] = true;
$response = $method->invoke($app, $response);
}

public function testUnsupportedMethodWithoutRoute()
{
$app = new App();
Expand Down
44 changes: 44 additions & 0 deletions tests/Middleware/ContentLengthMiddlewareTest.php
@@ -0,0 +1,44 @@
<?php
/**
* Slim Framework (https://slimframework.com)
*
* @link https://github.com/slimphp/Slim
* @copyright Copyright (c) 2011-2017 Josh Lockhart
* @license https://github.com/slimphp/Slim/blob/3.x/LICENSE.md (MIT License)
*/
namespace Slim\Tests\Middleware;

use PHPUnit\Framework\TestCase;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Slim\Http\Body;
use Slim\Http\Headers;
use Slim\Http\Request;
use Slim\Http\Response;
use Slim\Http\Uri;
use Slim\Middleware\ContentLengthMiddleware;

class ContentLengthMiddlewareTest extends TestCase
{
public function testAddsContentLenght()
{
$mw = new ContentLengthMiddleware('append');

$uri = Uri::createFromString('https://example.com:443/foo/bar?abc=123');
$headers = new Headers();
$cookies = [];
$serverParams = [];
$body = new Body(fopen('php://temp', 'r+'));
$request = new Request('GET', $uri, $headers, $cookies, $serverParams, $body);
$response = new Response();

$next = function (ServerRequestInterface $req, ResponseInterface $res) {
$res->write('Body');
return $res;
};

$newResponse = $mw($request, $response, $next);

$this->assertEquals(4, $newResponse->getHeaderLine('Content-Length'));
}
}