updated .htaccess format, refactored .htaccess creation logic and improving code coverage, fixes #194

.gitignore

@@ -31,6 +31,7 @@ vendor/**/build_phar.php
 # Ignore local node modules, unit testing logs, api docs and eclipse project files
 js/node_modules/
 tst/log/
+tst/ConfigurationCombinationsTest.php
 .settings
 .buildpath
 .project

lib/Data/Filesystem.php

@@ -12,6 +12,7 @@
 
 namespace PrivateBin\Data;
 
+use Exception;
 use PrivateBin\Json;
 use PrivateBin\Model\Paste;
 
@@ -41,16 +42,16 @@ class Filesystem extends AbstractData
      */
     public static function getInstance($options = null)
     {
+        // if needed initialize the singleton
+        if (!(self::$_instance instanceof self)) {
+            self::$_instance = new self;
+        }
         // if given update the data directory
         if (
             is_array($options) &&
             array_key_exists('dir', $options)
         ) {
             self::$_dir = $options['dir'] . DIRECTORY_SEPARATOR;
-        }
-        // if needed initialize the singleton
-        if (!(self::$_instance instanceof self)) {
-            self::$_instance = new self;
             self::_init();
         }
         return self::$_instance;
@@ -293,7 +294,7 @@ class Filesystem extends AbstractData
     }
 
     /**
-     * initialize privatebin
+     * Initialize data store
      *
      * @access private
      * @static
@@ -303,15 +304,20 @@ class Filesystem extends AbstractData
     {
         // Create storage directory if it does not exist.
         if (!is_dir(self::$_dir)) {
-            mkdir(self::$_dir, 0700);
+            if (!@mkdir(self::$_dir, 0700)) {
+                throw new Exception('unable to create directory ' . self::$_dir, 10);
+            }
         }
-        // Create .htaccess file if it does not exist.
+        $file = self::$_dir . DIRECTORY_SEPARATOR . '.htaccess';
-        if (!is_file(self::$_dir . '.htaccess')) {
+        if (!is_file($file)) {
-            file_put_contents(
+            $writtenBytes = @file_put_contents(
-                self::$_dir . '.htaccess',
+                $file,
-                'Allow from none' . PHP_EOL .
+                'Require all denied' . PHP_EOL,
-                'Deny from all' . PHP_EOL
+                LOCK_EX
             );
+            if ($writtenBytes === false || $writtenBytes < 19) {
+                throw new Exception('unable to write to file ' . $file, 11);
+            }
         }
     }
 

lib/Persistence/AbstractPersistence.php

@@ -86,21 +86,18 @@ abstract class AbstractPersistence
     {
         // Create storage directory if it does not exist.
         if (!is_dir(self::$_path)) {
-            if (!@mkdir(self::$_path)) {
+            if (!@mkdir(self::$_path, 0700)) {
                 throw new Exception('unable to create directory ' . self::$_path, 10);
             }
         }
-
-        // Create .htaccess file if it does not exist.
         $file = self::$_path . DIRECTORY_SEPARATOR . '.htaccess';
         if (!is_file($file)) {
             $writtenBytes = @file_put_contents(
                 $file,
-                'Allow from none' . PHP_EOL .
+                'Require all denied' . PHP_EOL,
-                'Deny from all' . PHP_EOL,
                 LOCK_EX
             );
-            if ($writtenBytes === false || $writtenBytes < 30) {
+            if ($writtenBytes === false || $writtenBytes < 19) {
                 throw new Exception('unable to write to file ' . $file, 11);
             }
         }

lib/PrivateBin.php

@@ -175,17 +175,6 @@ class PrivateBin
      */
     private function _init()
     {
-        foreach (array('cfg', 'lib') as $dir) {
-            if (!is_file(PATH . $dir . DIRECTORY_SEPARATOR . '.htaccess')) {
-                file_put_contents(
-                PATH . $dir . DIRECTORY_SEPARATOR . '.htaccess',
-                'Allow from none' . PHP_EOL .
-                'Deny from all' . PHP_EOL,
-                LOCK_EX
-            );
-            }
-        }
-
         $this->_conf    = new Configuration;
         $this->_model   = new Model($this->_conf);
         $this->_request = new Request;

tst/.gitignore

@@ -1 +0,0 @@
-/ConfigurationCombinationsTest.php

tst/.htaccess

@@ -1,2 +0,0 @@
-Allow from none
-Deny from all

tst/Data/FilesystemTest.php

@@ -8,16 +8,26 @@ class FilesystemTest extends PHPUnit_Framework_TestCase
 
     private $_path;
 
+    private $_invalidPath;
+
     public function setUp()
     {
         /* Setup Routine */
         $this->_path  = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'privatebin_data';
+        $this->_invalidPath = $this->_path . DIRECTORY_SEPARATOR . 'bar';
         $this->_model = Filesystem::getInstance(array('dir' => $this->_path));
+        if (!is_dir($this->_path)) {
+            mkdir($this->_path);
+        }
+        if (!is_dir($this->_invalidPath)) {
+            mkdir($this->_invalidPath);
+        }
     }
 
     public function tearDown()
     {
         /* Tear Down Routine */
+        chmod($this->_invalidPath, 0700);
         Helper::rmDir($this->_path);
     }
 
@@ -37,6 +47,7 @@ class FilesystemTest extends PHPUnit_Framework_TestCase
         $this->assertFalse($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment does not yet exist');
         $this->assertTrue($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), Helper::getComment()), 'store comment');
         $this->assertTrue($this->_model->existsComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId()), 'comment exists after storing it');
+        $this->assertFalse($this->_model->createComment(Helper::getPasteId(), Helper::getPasteId(), Helper::getCommentId(), Helper::getComment()), 'unable to store the same comment twice');
         $comment           = json_decode(json_encode(Helper::getComment()));
         $comment->id       = Helper::getCommentId();
         $comment->parentid = Helper::getPasteId();
@@ -127,4 +138,26 @@ 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/ModelTest.php

@@ -82,6 +82,7 @@ class ModelTest extends PHPUnit_Framework_TestCase
         $comment = $paste->getComment(Helper::getPasteId());
         $comment->setData($commentData['data']);
         $comment->setNickname($commentData['meta']['nickname']);
+        $comment->getParentId();
         $comment->store();
 
         $comment = $paste->getComment(Helper::getPasteId(), Helper::getCommentId());
@@ -189,6 +190,27 @@ class ModelTest extends PHPUnit_Framework_TestCase
         $this->assertFalse(Paste::isValidId('../bar/baz'), 'path attack');
     }
 
+    /**
+     * @expectedException Exception
+     * @expectedExceptionCode 64
+     */
+    public function testInvalidPaste()
+    {
+        $this->_model->getPaste(Helper::getPasteId())->delete();
+        $paste = $this->_model->getPaste(Helper::getPasteId());
+        $paste->get();
+    }
+
+    /**
+     * @expectedException Exception
+     * @expectedExceptionCode 61
+     */
+    public function testInvalidData()
+    {
+        $paste = $this->_model->getPaste();
+        $paste->setData('');
+    }
+
     /**
      * @expectedException Exception
      * @expectedExceptionCode 62
@@ -199,6 +221,37 @@ class ModelTest extends PHPUnit_Framework_TestCase
         $paste->getComment(Helper::getPasteId());
     }
 
+    /**
+     * @expectedException Exception
+     * @expectedExceptionCode 67
+     */
+    public function testInvalidCommentDeletedPaste()
+    {
+        $pasteData = Helper::getPaste();
+        $paste = $this->_model->getPaste(Helper::getPasteId());
+        $paste->setData($pasteData['data']);
+        $paste->store();
+
+        $comment = $paste->getComment(Helper::getPasteId());
+        $paste->delete();
+        $comment->store();
+    }
+
+    /**
+     * @expectedException Exception
+     * @expectedExceptionCode 68
+     */
+    public function testInvalidCommentData()
+    {
+        $pasteData = Helper::getPaste();
+        $paste = $this->_model->getPaste(Helper::getPasteId());
+        $paste->setData($pasteData['data']);
+        $paste->store();
+
+        $comment = $paste->getComment(Helper::getPasteId());
+        $comment->store();
+    }
+
     public function testExpiration()
     {
         $pasteData = Helper::getPaste();

tst/PrivateBinTest.php

@@ -140,21 +140,18 @@ class PrivateBinTest extends PHPUnit_Framework_TestCase
     public function testHtaccess()
     {
         $this->reset();
-        $dirs = array('cfg', 'lib');
+        $file = $this->_path . DIRECTORY_SEPARATOR . '.htaccess';
-        foreach ($dirs as $dir) {
+        @unlink($file);
-            $file = PATH . $dir . DIRECTORY_SEPARATOR . '.htaccess';
+
-            @unlink($file);
+        $_POST                            = Helper::getPaste();
-        }
+        $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest';
+        $_SERVER['REQUEST_METHOD']        = 'POST';
+        $_SERVER['REMOTE_ADDR']           = '::1';
         ob_start();
         new PrivateBin;
         ob_end_clean();
-        foreach ($dirs as $dir) {
+
-            $file = PATH . $dir . DIRECTORY_SEPARATOR . '.htaccess';
+        $this->assertFileExists($file, 'htaccess recreated');
-            $this->assertFileExists(
-                $file,
-                "$dir htaccess recreated"
-            );
-        }
     }
 
     /**

tst/SjclTest.php

@@ -1,11 +1,13 @@
 <?php
 
+use PrivateBin\Persistence\ServerSalt;
 use PrivateBin\Sjcl;
 
 class SjclTest extends PHPUnit_Framework_TestCase
 {
     public function testSjclValidatorValidatesCorrectly()
     {
+        ServerSalt::setPath(sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'privatebin_data');
         $paste = Helper::getPasteWithAttachment();
         $this->assertTrue(Sjcl::isValid($paste['data']), 'valid sjcl');
         $this->assertTrue(Sjcl::isValid($paste['attachment']), 'valid sjcl');