Skip to content

Fix FileHandler reading non-existent files #1

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.

Sign up for GitHub

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

Merged
GwendolenLynch merged 3 commits into from
Oct 16, 2017
Merged

Fix FileHandler reading non-existent files #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7b43bc0
Clean up FileHandler src and tests.
CarsonF Oct 16, 2017
ecb4bcb
Remove depth property since it's not used.
CarsonF Oct 16, 2017
0c8723c
Fix FileHandler reading non-existent or unreadable files.
CarsonF Oct 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 25 additions & 12 deletions src/Handler/FileHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Bolt\Session\Handler;

use Bolt\Common\Thrower;
use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
use Symfony\Component\Filesystem\Exception\IOException;
Expand All @@ -16,8 +17,6 @@
*/
class FileHandler extends AbstractHandler implements LazyWriteHandlerInterface
{
/** @var int */
protected $depth;
/** @var int */
protected $mode;
/** @var string */
Expand All @@ -41,7 +40,6 @@ public function __construct($savePath = null, LoggerInterface $logger = null, Fi
$this->logger = $logger ?: new NullLogger();

// @see http://php.net/manual/en/session.configuration.php#ini.session.save-path
$depth = 1;
$mode = 0600;
$savePath = $savePath ?: sys_get_temp_dir();

Expand All @@ -53,20 +51,17 @@ public function __construct($savePath = null, LoggerInterface $logger = null, Fi

$path = explode(';', $savePath);
if ($count === 1) {
$depth = $path[0];
$savePath = $path[1];
} else {
$depth = $path[0];
$mode = intval($path[1], 8);
$savePath = $path[2];
}
}

if (!is_dir($savePath)) {
$this->fs->mkdir($savePath, 0777);
$this->fs->mkdir($savePath);
}

$this->depth = $depth;
$this->mode = $mode;
$this->savePath = $savePath;
}
Expand All @@ -76,24 +71,40 @@ public function __construct($savePath = null, LoggerInterface $logger = null, Fi
*/
public function read($sessionId)
{
return file_get_contents($this->getSessionFileName($sessionId));
$file = $this->getSessionFileName($sessionId);

try {
if ($this->fs->exists($file)) {
try {
return Thrower::call('file_get_contents', $file);
} catch (\ErrorException $e) {
$this->logger->error(sprintf('Unable to read session file: %s', $file), ['exception' => $e]);
}
}
} catch (IOException $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
}

return '';
}

/**
* {@inheritdoc}
*/
public function write($sessionId, $data)
{
$file = $this->getSessionFileName($sessionId);

try {
$this->fs->dumpFile($this->getSessionFileName($sessionId), $data);
$this->fs->dumpFile($file, $data);
} catch (IOException $e) {
$this->logger->error('Unable to write session file to ' . $this->savePath);

return false;
}

try {
$this->fs->chmod($this->getSessionFileName($sessionId), $this->mode);
$this->fs->chmod($file, $this->mode);
} catch (IOException $e) {
$this->logger->error('Unable to set correct permissions on session file in ' . $this->savePath);
}
Expand All @@ -117,12 +128,14 @@ public function updateTimestamp($sessionId, $data)
*/
public function destroy($sessionId)
{
$file = $this->getSessionFileName($sessionId);

try {
$this->fs->remove($this->getSessionFileName($sessionId));
$this->fs->remove($file);

return true;
} catch (IOException $e) {
$this->logger->error('Unable to remove session file ' . $this->getSessionFileName($sessionId));
$this->logger->error('Unable to remove session file ' . $file);

return false;
}
Expand Down
79 changes: 42 additions & 37 deletions tests/Handler/FileHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,88 +19,93 @@ class FileHandlerTest extends TestCase
/** @var string */
protected $savePath;
/** @var string */
protected $sessionName;
protected $sessionId;
/** @var string */
protected $sessionFile;

public function setUp()
{
$this->vfs = VfsStream::setup();
$this->savePath = $this->vfs->url();
$this->sessionName = 'george';
$this->sessionFile = $this->savePath . '/' . $this->sessionName . '.sess';
$this->sessionId = 'george';
$this->sessionFile = $this->savePath . '/' . $this->sessionId . '.sess';
}

public function testConstructor()
public function testOpen()
{
$this->assertClassHasAttribute('savePath', FileHandler::class);
$this->assertClassHasAttribute('fs', FileHandler::class);
$fsh = new FileHandler($this->savePath);

$result = $fsh->open(null, 'PHPSESSID');
$this->assertTrue($result);
}

public function testClose()
{
$fsh = new FileHandler($this->savePath);

$this->assertObjectHasAttribute('savePath', $fsh);
$this->assertObjectHasAttribute('fs', $fsh);
$result = $fsh->close();
$this->assertTrue($result);
}

public function testRead()
{
file_put_contents($this->sessionFile, 'kittens');

$this->assertAttributeEquals($this->savePath, 'savePath', $fsh);
$fsh = new FileHandler($this->savePath);

$result = $fsh->read($this->sessionId);
$this->assertSame('kittens', $result);
}

public function testOpen()
public function testReadNew()
{
$fsh = new FileHandler($this->savePath);

$result = $fsh->open($this->savePath, $this->sessionName);
$this->assertTrue($result);
$result = $fsh->read('new');
$this->assertSame('', $result);
}

public function testClose()
public function testReadUnreadable()
{
file_put_contents($this->savePath . '/unreadable.sess', '');
chmod($this->savePath . '/unreadable.sess', 0);

$fsh = new FileHandler($this->savePath);

$result = $fsh->close();
$this->assertTrue($result);
$result = $fsh->read('unreadable');
$this->assertSame('', $result);
}

/**
* @covers \Bolt\Session\Handler\FileHandler::write
* @covers \Bolt\Session\Handler\FileHandler::read
*/
public function testWriteRead()
public function testWrite()
{
$fsh = new FileHandler($this->savePath);
$fsh->open($this->savePath, $this->sessionName);

$result = $fsh->write($this->sessionName, 'kittens');
$result = $fsh->write($this->sessionId, 'kittens');
$this->assertTrue($result);

$result = $fsh->read($this->sessionName);
$this->assertSame('kittens', $result);
$this->assertFileExists($this->sessionFile);
$this->assertStringEqualsFile($this->sessionFile, 'kittens');
}

public function testDestroy()
{
$fsh = new FileHandler($this->savePath);
$fsh->open($this->savePath, $this->sessionName);
file_put_contents($this->sessionFile, 'kittens');

$fsh->write($this->sessionName, 'kittens');
$this->assertFileExists($this->sessionFile);
$fsh = new FileHandler($this->savePath);

$result = $fsh->destroy($this->sessionName);
$result = $fsh->destroy($this->sessionId);
$this->assertTrue($result);
$this->assertFileNotExists($this->sessionName);
$this->assertFileNotExists($this->sessionFile);
}

public function testGc()
{
$fsh = new FileHandler($this->savePath);
$fsh->open($this->savePath, $this->sessionName);
file_put_contents($this->sessionFile, 'kittens');
touch($this->sessionFile, time() - 5);

$fsh->write($this->sessionName, 'kittens');
$this->assertFileExists($this->sessionFile);
$fsh = new FileHandler($this->savePath);

sleep(1);
$result = $fsh->gc(1);
$this->assertTrue($result);
$this->assertFileNotExists($this->sessionName);
$this->assertFileNotExists($this->sessionFile);
}
}