removing duplicate code, cleanup of temporary test files

lib/Data/Filesystem.php

@@ -12,9 +12,8 @@
 
 namespace PrivateBin\Data;
 
-use Exception;
-use PrivateBin\Json;
 use PrivateBin\Model\Paste;
+use PrivateBin\Persistence\DataStore;
 
 /**
  * Filesystem
@@ -23,15 +22,6 @@ use PrivateBin\Model\Paste;
  */
 class Filesystem extends AbstractData
 {
-    /**
-     * directory where data is stored
-     *
-     * @access private
-     * @static
-     * @var string
-     */
-    private static $_dir = 'data/';
-
     /**
      * get instance of singleton
      *
@@ -51,8 +41,7 @@ class Filesystem extends AbstractData
             is_array($options) &&
             array_key_exists('dir', $options)
         ) {
-            self::$_dir = $options['dir'] . DIRECTORY_SEPARATOR;
-            self::_init();
+            DataStore::setPath($options['dir']);
         }
         return self::$_instance;
     }
@@ -63,19 +52,19 @@ class Filesystem extends AbstractData
      * @access public
      * @param  string $pasteid
      * @param  array  $paste
-     * @throws Exception
      * @return bool
      */
     public function create($pasteid, $paste)
     {
         $storagedir = self::_dataid2path($pasteid);
-        if (is_file($storagedir . $pasteid)) {
+        $file       = $storagedir . $pasteid;
+        if (is_file($file)) {
             return false;
         }
         if (!is_dir($storagedir)) {
             mkdir($storagedir, 0700, true);
         }
-        return (bool) file_put_contents($storagedir . $pasteid, Json::encode($paste));
+        return DataStore::store($file, $paste);
     }
 
     /**
@@ -156,20 +145,19 @@ class Filesystem extends AbstractData
      * @param  string $parentid
      * @param  string $commentid
      * @param  array  $comment
-     * @throws Exception
      * @return bool
      */
     public function createComment($pasteid, $parentid, $commentid, $comment)
     {
         $storagedir = self::_dataid2discussionpath($pasteid);
-        $filename   = $pasteid . '.' . $commentid . '.' . $parentid;
-        if (is_file($storagedir . $filename)) {
+        $file       = $storagedir . $pasteid . '.' . $commentid . '.' . $parentid;
+        if (is_file($file)) {
             return false;
         }
         if (!is_dir($storagedir)) {
             mkdir($storagedir, 0700, true);
         }
-        return (bool) file_put_contents($storagedir . $filename, Json::encode($comment));
+        return DataStore::store($file, $comment);
     }
 
     /**
@@ -238,8 +226,9 @@ class Filesystem extends AbstractData
     protected function _getExpiredPastes($batchsize)
     {
         $pastes     = array();
+        $mainpath   = DataStore::getPath();
         $firstLevel = array_filter(
-            scandir(self::$_dir),
+            scandir($mainpath),
             'self::_isFirstLevelDir'
         );
         if (count($firstLevel) > 0) {
@@ -247,7 +236,7 @@ class Filesystem extends AbstractData
             for ($i = 0, $max = $batchsize * 10; $i < $max; ++$i) {
                 $firstKey    = array_rand($firstLevel);
                 $secondLevel = array_filter(
-                    scandir(self::$_dir . $firstLevel[$firstKey]),
+                    scandir($mainpath . DIRECTORY_SEPARATOR . $firstLevel[$firstKey]),
                     'self::_isSecondLevelDir'
                 );
 
@@ -258,8 +247,9 @@ class Filesystem extends AbstractData
                 }
 
                 $secondKey = array_rand($secondLevel);
-                $path      = self::$_dir . $firstLevel[$firstKey] .
-                    DIRECTORY_SEPARATOR . $secondLevel[$secondKey];
+                $path      = $mainpath . DIRECTORY_SEPARATOR .
+                    $firstLevel[$firstKey] . DIRECTORY_SEPARATOR .
+                    $secondLevel[$secondKey];
                 if (!is_dir($path)) {
                     continue;
                 }
@@ -293,34 +283,6 @@ class Filesystem extends AbstractData
         return $pastes;
     }
 
-    /**
-     * Initialize data store
-     *
-     * @access private
-     * @static
-     * @return void
-     */
-    private static function _init()
-    {
-        // Create storage directory if it does not exist.
-        if (!is_dir(self::$_dir)) {
-            if (!@mkdir(self::$_dir, 0700)) {
-                throw new Exception('unable to create directory ' . self::$_dir, 10);
-            }
-        }
-        $file = self::$_dir . DIRECTORY_SEPARATOR . '.htaccess';
-        if (!is_file($file)) {
-            $writtenBytes = @file_put_contents(
-                $file,
-                'Require all denied' . PHP_EOL,
-                LOCK_EX
-            );
-            if ($writtenBytes === false || $writtenBytes < 19) {
-                throw new Exception('unable to write to file ' . $file, 11);
-            }
-        }
-    }
-
     /**
      * Convert paste id to storage path.
      *
@@ -338,8 +300,10 @@ class Filesystem extends AbstractData
      */
     private static function _dataid2path($dataid)
     {
-        return self::$_dir . substr($dataid, 0, 2) . DIRECTORY_SEPARATOR .
-            substr($dataid, 2, 2) . DIRECTORY_SEPARATOR;
+        return DataStore::getPath(
+            substr($dataid, 0, 2) . DIRECTORY_SEPARATOR .
+            substr($dataid, 2, 2) . DIRECTORY_SEPARATOR
+        );
     }
 
     /**
@@ -369,7 +333,7 @@ class Filesystem extends AbstractData
     private static function _isFirstLevelDir($element)
     {
         return self::_isSecondLevelDir($element) &&
-            is_dir(self::$_dir . DIRECTORY_SEPARATOR . $element);
+            is_dir(DataStore::getPath($element));
     }
 
     /**

lib/Persistence/DataStore.php

@@ -0,0 +1,48 @@
+<?php
+/**
+ * PrivateBin
+ *
+ * a zero-knowledge paste bin
+ *
+ * @link      https://github.com/PrivateBin/PrivateBin
+ * @copyright 2012 Sébastien SAUVAGE (sebsauvage.net)
+ * @license   https://www.opensource.org/licenses/zlib-license.php The zlib/libpng License
+ * @version   1.1
+ */
+
+namespace PrivateBin\Persistence;
+
+use Exception;
+use PrivateBin\Configuration;
+use PrivateBin\Json;
+
+/**
+ * DataStore
+ *
+ * Handles data storage for Data\Filesystem.
+ */
+class DataStore extends AbstractPersistence
+{
+    /**
+     * store the data
+     *
+     * @access public
+     * @static
+     * @param  string $filename
+     * @param  string $data
+     * @return bool
+     */
+    public static function store($filename, $data)
+    {
+        $path = self::getPath();
+        if (strpos($filename, $path) === 0) {
+            $filename = substr($filename, strlen($path));
+        }
+        try {
+            self::_store($filename, Json::encode($data));
+            return true;
+        } catch (Exception $e) {
+            return false;
+        }
+    }
+}

tst/Bootstrap.php

@@ -172,22 +172,24 @@ class Helper
      */
     public static function rmDir($path)
     {
-        $path .= DIRECTORY_SEPARATOR;
-        $dir = dir($path);
-        while (false !== ($file = $dir->read())) {
-            if ($file != '.' && $file != '..') {
-                if (is_dir($path . $file)) {
-                    self::rmDir($path . $file);
-                } elseif (is_file($path . $file)) {
-                    if (!unlink($path . $file)) {
-                        throw new Exception('Error deleting file "' . $path . $file . '".');
+        if (is_dir($path)) {
+            $path .= DIRECTORY_SEPARATOR;
+            $dir = dir($path);
+            while (false !== ($file = $dir->read())) {
+                if ($file != '.' && $file != '..') {
+                    if (is_dir($path . $file)) {
+                        self::rmDir($path . $file);
+                    } elseif (is_file($path . $file)) {
+                        if (!unlink($path . $file)) {
+                            throw new Exception('Error deleting file "' . $path . $file . '".');
+                        }
                     }
                 }
             }
-        }
-        $dir->close();
-        if (!rmdir($path)) {
-            throw new Exception('Error deleting directory "' . $path . '".');
+            $dir->close();
+            if (!rmdir($path)) {
+                throw new Exception('Error deleting directory "' . $path . '".');
+            }
         }
     }
 

tst/Data/FilesystemTest.php

@@ -110,10 +110,6 @@ class FilesystemTest extends PHPUnit_Framework_TestCase
         }
     }
 
-    /**
-     * @expectedException Exception
-     * @expectedExceptionCode 90
-     */
     public function testErrorDetection()
     {
         $this->_model->delete(Helper::getPasteId());
@@ -123,10 +119,6 @@ class FilesystemTest extends PHPUnit_Framework_TestCase
         $this->assertFalse($this->_model->exists(Helper::getPasteId()), 'paste does still not exist');
     }
 
-    /**
-     * @expectedException Exception
-     * @expectedExceptionCode 90
-     */
     public function testCommentErrorDetection()
     {
         $this->_model->delete(Helper::getPasteId());
@@ -138,26 +130,4 @@ class FilesystemTest extends PHPUnit_Framework_TestCase
         $this->assertFalse($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), $comment), 'unable to store broken comment');
         $this->assertFalse($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment does still not exist');
     }
-
-    /**
-     * @expectedException Exception
-     * @expectedExceptionCode 10
-     */
-    public function testPermissionShenanigans()
-    {
-        // try creating an invalid path
-        chmod($this->_invalidPath, 0000);
-        Filesystem::getInstance(array('dir' => $this->_invalidPath . DIRECTORY_SEPARATOR . 'baz'));
-    }
-
-    /**
-     * @expectedException Exception
-     * @expectedExceptionCode 11
-     */
-    public function testPathShenanigans()
-    {
-        // try setting an invalid path
-        chmod($this->_invalidPath, 0000);
-        Filesystem::getInstance(array('dir' => $this->_invalidPath));
-    }
 }

tst/JsonApiTest.php

@@ -98,6 +98,7 @@ class JsonApiTest extends PHPUnit_Framework_TestCase
         new PrivateBin;
         $content = ob_get_contents();
         ob_end_clean();
+        unlink($file);
         $response = json_decode($content, true);
         $this->assertEquals(0, $response['status'], 'outputs status');
         $this->assertEquals(Helper::getPasteId(), $response['id'], 'outputted paste ID matches input');
@@ -132,6 +133,7 @@ class JsonApiTest extends PHPUnit_Framework_TestCase
         new PrivateBin;
         $content = ob_get_contents();
         ob_end_clean();
+        unlink($file);
         $response = json_decode($content, true);
         $this->assertEquals(0, $response['status'], 'outputs status');
         $this->assertFalse($this->_model->exists(Helper::getPasteId()), 'paste successfully deleted');

tst/RequestTest.php

@@ -63,6 +63,7 @@ class RequestTest extends PHPUnit_Framework_TestCase
         file_put_contents($file, 'data=foo');
         Request::setInputStream($file);
         $request = new Request;
+        unlink($file);
         $this->assertTrue($request->isJsonApiCall(), 'is JSON Api call');
         $this->assertEquals('create', $request->getOperation());
         $this->assertEquals('foo', $request->getParam('data'));