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

4.x Port of Fix for #2568 #2588

Merged
merged 4 commits into from Mar 2, 2019
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
13 changes: 10 additions & 3 deletions Slim/Router.php
Expand Up @@ -170,12 +170,19 @@ public function setBasePath(string $basePath): self
*/
public function setCacheFile(?string $cacheFile): RouterInterface
{
$this->cacheFile = $cacheFile;
if ($cacheFile && file_exists($cacheFile) && !is_readable($cacheFile)) {
throw new RuntimeException(
sprintf('Router cache file `%s` is not readable', $cacheFile)
);
}

if (is_string($cacheFile) && !is_writable(dirname($cacheFile))) {
throw new RuntimeException('Router cacheFile directory must be writable');
if ($cacheFile && !file_exists($cacheFile) && !is_writable(dirname($cacheFile))) {
throw new RuntimeException(
sprintf('Router cache file directory `%s` is not writable', dirname($cacheFile))
);
}

$this->cacheFile = $cacheFile;
return $this;
}

Expand Down
2 changes: 1 addition & 1 deletion composer.json
Expand Up @@ -53,7 +53,7 @@
},
"autoload-dev": {
"files": [
"tests/Assets/HeaderFunctions.php"
"tests/Assets/PhpFunctionOverrides.php"
]
},
"scripts": {
Expand Down
32 changes: 2 additions & 30 deletions tests/Assets/HeaderFunctions.php → tests/Assets/HeaderStack.php
Expand Up @@ -6,7 +6,7 @@
* We put these into the Slim namespace, so that Slim\App will use these versions of header() and
* headers_sent() when we test its output.
*/
namespace Slim;
namespace Slim\Tests\Assets;

/**
* Zend Framework (http://framework.zend.com/)
Expand All @@ -31,7 +31,7 @@
/**
* Store output artifacts
*/
class HeaderStackTestAsset
class HeaderStack
{
/**
* @var string[][]
Expand Down Expand Up @@ -84,31 +84,3 @@ public static function has($header)
return false;
}
}

/**
* Have headers been sent?
*
* @return false
*/
function headers_sent()
{
return false;
}

/**
* Emit a header, without creating actual output artifacts
*
* @param string $string
* @param bool $replace
* @param int|null $statusCode
*/
function header($string, $replace = true, $statusCode = null)
{
HeaderStackTestAsset::push(
[
'header' => $string,
'replace' => $replace,
'status_code' => $statusCode,
]
);
}
68 changes: 68 additions & 0 deletions tests/Assets/PhpFunctionOverrides.php
@@ -0,0 +1,68 @@
<?php
/**
* Override PHP namespaced functions into the Slim\App namespace.
*
* header() and headers_sent() are from Zend-Diactoros zend-diactoros/test/TestAsset/Functions.php and are
* Copyright (c) 2015-2016 Zend Technologies USA Inc. (http://www.zend.com) and are licensed under the New BSD License
* at https://github.com/zendframework/zend-diactoros/blob/master/LICENSE.md.
*
*/
namespace Slim;

use Slim\Tests\Assets\HeaderStack;

/**
* Have headers been sent?
*
* @return false
*/
function headers_sent()
{
return false;
}

/**
* Emit a header, without creating actual output artifacts
*
* @param string $string
* @param bool $replace
* @param int|null $statusCode
*/
function header($string, $replace = true, $statusCode = null)
{
HeaderStack::push(
[
'header' => $string,
'replace' => $replace,
'status_code' => $statusCode,
]
);
}

/**
* Is a file descriptor writable
*
* @param $file
* @return bool
*/
function is_readable($file)
{
if (stripos($file, 'non-readable.cache') !== false) {
return false;
}
return true;
}

/**
* Is a path writable
*
* @param $path
* @return bool
*/
function is_writable($path)
{
if (stripos($path, 'non-writable-directory') !== false) {
return false;
}
return true;
}
14 changes: 7 additions & 7 deletions tests/ResponseEmitterTest.php
Expand Up @@ -8,8 +8,8 @@
*/
namespace Slim\Tests;

use Slim\HeaderStackTestAsset;
use Slim\ResponseEmitter;
use Slim\Tests\Assets\HeaderStack;
use Slim\Tests\Mocks\MockStream;
use Slim\Tests\Mocks\SmallChunksStream;

Expand All @@ -21,12 +21,12 @@ class ResponseEmitterTest extends TestCase
{
public function setUp()
{
HeaderStackTestAsset::reset();
HeaderStack::reset();
}

public function tearDown()
{
HeaderStackTestAsset::reset();
HeaderStack::reset();
}

public function testRespond()
Expand All @@ -47,8 +47,8 @@ public function testRespondNoContent()
$responseEmitter = new ResponseEmitter();
$responseEmitter->emit($response);

$this->assertEquals(false, HeaderStackTestAsset::has('Content-Type'));
$this->assertEquals(false, HeaderStackTestAsset::has('Content-Length'));
$this->assertEquals(false, HeaderStack::has('Content-Type'));
$this->assertEquals(false, HeaderStack::has('Content-Length'));
$this->expectOutputString('');
}

Expand Down Expand Up @@ -143,7 +143,7 @@ public function testResponseReplacesPreviouslySetHeaders()
['header' => 'HTTP/1.1 200 OK', 'replace' => true, 'status_code' => 200],
];

$this->assertSame($expectedStack, HeaderStackTestAsset::stack());
$this->assertSame($expectedStack, HeaderStack::stack());
}

public function testResponseDoesNotReplacePreviouslySetSetCookieHeaders()
Expand All @@ -161,7 +161,7 @@ public function testResponseDoesNotReplacePreviouslySetSetCookieHeaders()
['header' => 'HTTP/1.1 200 OK', 'replace' => true, 'status_code' => 200],
];

$this->assertSame($expectedStack, HeaderStackTestAsset::stack());
$this->assertSame($expectedStack, HeaderStack::stack());
}

public function testIsResponseEmptyWithNonEmptyBodyAndTriggeringStatusCode()
Expand Down
41 changes: 34 additions & 7 deletions tests/RouterTest.php
Expand Up @@ -23,6 +23,16 @@ class RouterTest extends TestCase
/** @var Router */
protected $router;

/** @var string */
protected $cacheFile;

public function tearDown()
{
if (file_exists($this->cacheFile)) {
unlink($this->cacheFile);
}
}

public function setUp()
{
$callableResolver = new CallableResolver();
Expand Down Expand Up @@ -315,16 +325,33 @@ public function testPathForWithModifiedRoutePattern()
}

/**
* Test if cacheFile is not a writable directory
*
* @expectedException \RuntimeException
* @expectedExceptionMessage Router cacheFile directory must be writable
* Test cache file exists but is not writable
*/
public function testSettingInvalidCacheFileNotExisting()
public function testCacheFileExistsAndIsNotReadable()
{
$this->router->setCacheFile(
dirname(__FILE__) . uniqid(microtime(true)) . '/' . uniqid(microtime(true))
$this->cacheFile = __DIR__ . '/non-readable.cache';
file_put_contents($this->cacheFile, '<?php return []; ?>');

$this->expectException(
'\RuntimeException',
sprintf('Router cache file `%s` is not readable', $this->cacheFile)
);

$this->router->setCacheFile($this->cacheFile);
}
/**
* Test cache file does not exist and directory is not writable
*/
public function testCacheFileDoesNotExistsAndDirectoryIsNotWritable()
{
$cacheFile = __DIR__ . '/non-writable-directory/router.cache';

$this->expectException(
'\RuntimeException',
sprintf('Router cache file directory `%s` is not writable', dirname($cacheFile))
);

$this->router->setCacheFile($cacheFile);
}

/**
Expand Down