fixing nasty deletion bug from #15, included unit tests to trigger it

and reworked persistence classes to through exceptions rather to fail
silently

doc/README.md

@@ -10,7 +10,7 @@ Details about [installing phpDocumentor](http://phpdoc.org/docs/latest/getting-s
 can be found in its own documentation.
 
 Example for Debian and Ubuntu:
-    $ sudo aptitude install pear graphviz
+    $ sudo aptitude install php-pear graphviz
     $ sudo pear channel-discover pear.phpdoc.org
     $ sudo pear install phpdoc/phpDocumentor
 

lib/persistence.php

@@ -67,7 +67,7 @@ abstract class persistence
     protected static function _exists($filename)
     {
         self::_initialize();
-        return is_file(self::$_path . '/' . $filename);
+        return is_file(self::$_path . DIRECTORY_SEPARATOR . $filename);
     }
 
     /**
@@ -75,23 +75,29 @@ abstract class persistence
      *
      * @access protected
      * @static
+     * @throws Exception
      * @return void
      */
     protected static function _initialize()
     {
         // Create storage directory if it does not exist.
-        if (!is_dir(self::$_path)) mkdir(self::$_path, 0705);
+        if (!is_dir(self::$_path))
+            if (!@mkdir(self::$_path))
+                throw new Exception('unable to create directory ' . self::$_path, 10);
 
         // Create .htaccess file if it does not exist.
-        $file = self::$_path . '/.htaccess';
+        $file = self::$_path . DIRECTORY_SEPARATOR . '.htaccess';
         if (!is_file($file))
         {
-            file_put_contents(
+            $writtenBytes = @file_put_contents(
                 $file,
                 'Allow from none' . PHP_EOL .
                 'Deny from all'. PHP_EOL,
                 LOCK_EX
             );
+            if ($writtenBytes === false || $writtenBytes < 30) {
+                throw new Exception('unable to write to file ' . $file, 11);
+            }
         }
     }
 
@@ -102,14 +108,17 @@ abstract class persistence
      * @static
      * @param  string $filename
      * @param  string $data
+     * @throws Exception
      * @return string
      */
     protected static function _store($filename, $data)
     {
         self::_initialize();
-        $file = self::$_path . '/' . $filename;
+        $file = self::$_path . DIRECTORY_SEPARATOR . $filename;
-        file_put_contents($file, $data, LOCK_EX);
+        $writtenBytes = @file_put_contents($file, $data, LOCK_EX);
-        chmod($file, 0705);
+        if ($writtenBytes === false || $writtenBytes < strlen($data)) {
+            throw new Exception('unable to write to file ' . $file, 13);
+        }
         return $file;
     }
 }

lib/serversalt.php

@@ -47,12 +47,11 @@ class serversalt extends persistence
         }
         else // fallback to mt_rand()
         {
-            for($i = 0; $i < 16; ++$i) {
+            for($i = 0; $i < 256; ++$i) {
                 $randomSalt .= base_convert(mt_rand(), 10, 16);
             }
         }
-        self::$_salt = $randomSalt;
+        return $randomSalt;
-        return self::$_salt;
     }
 
     /**
@@ -60,21 +59,41 @@ class serversalt extends persistence
      *
      * @access public
      * @static
+     * @throws Exception
      * @return string
      */
     public static function get()
     {
-    	if (strlen(self::$_salt)) return self::$_salt;
+        if (strlen(self::$_salt)) return self::$_salt;
 
         $file = 'salt.php';
-        if (!self::_exists($file)) {
+        if (self::_exists($file)) {
+            $items = explode('|', @file_get_contents(self::getPath($file)));
+            if (!is_array($items) || count($items) != 3) {
+                throw new Exception('unable to read file ' . self::getPath($file), 20);
+            }
+            self::$_salt = $items[1];
+        } else {
+            self::$_salt = self::generate();
             self::_store(
-            	$file,
+                $file,
-                '<?php /* |'. self::generate() . '| */ ?>'
+                '<?php /* |'. self::$_salt . '| */ ?>'
             );
         }
-        $items = explode('|', file_get_contents(self::getPath($file)));
+        return self::$_salt;
-        self::$_salt = $items[1];
+    }
-        return $items[1];
+
+    /**
+     * set the path
+     *
+     * @access public
+     * @static
+     * @param  string $path
+     * @return void
+     */
+    public static function setPath($path)
+    {
+    	self::$_salt = '';
+        parent::setPath($path);
     }
 }

lib/trafficlimiter.php

@@ -47,6 +47,7 @@ class trafficlimiter extends persistence
      * @access public
      * @static
      * @param  string $ip
+     * @throws Exception
      * @return bool
      */
     public static function canPass($ip)

lib/zerobin.php

@@ -339,7 +339,7 @@ class zerobin
             // Generate the "delete" token.
             // The token is the hmac of the pasteid signed with the server salt.
             // The paste can be delete by calling http://myserver.com/zerobin/?pasteid=<pasteid>&deletetoken=<deletetoken>
-            $deletetoken = hash_hmac('sha1', $dataid , serversalt::get());
+            $deletetoken = hash_hmac('sha1', $dataid, serversalt::get());
 
             // 0 = no error
             $this->_return_message(0, $dataid, array('deletetoken' => $deletetoken));
@@ -373,7 +373,8 @@ class zerobin
         }
 
         // Make sure token is valid.
-        if (filter::slow_equals($deletetoken, hash_hmac('sha1', $dataid , serversalt::get())))
+        serversalt::setPath($this->_conf['traffic']['dir']);
+        if (!filter::slow_equals($deletetoken, hash_hmac('sha1', $dataid, serversalt::get())))
         {
             $this->_error = 'Wrong deletion token. Paste was not deleted.';
             return;

lib/zerobin/data.php

@@ -38,8 +38,8 @@ class zerobin_data extends zerobin_abstract
     {
         // if given update the data directory
         if (
-        	is_array($options) &&
+            is_array($options) &&
-        	array_key_exists('dir', $options)
+            array_key_exists('dir', $options)
         ) self::$_dir = $options['dir'] . DIRECTORY_SEPARATOR;
         // if needed initialize the singleton
         if(!(self::$_instance instanceof zerobin_data)) {

lib/zerobin/db.php

@@ -80,13 +80,6 @@ class zerobin_db extends zerobin_abstract
                 array_key_exists('opt', $options)
             )
             {
-                self::$_db = new PDO(
-                    $options['dsn'],
-                    $options['usr'],
-                    $options['pwd'],
-                    $options['opt']
-                );
-
                 // check if the database contains the required tables
                 self::$_type = strtolower(
                     substr($options['dsn'], 0, strpos($options['dsn'], ':'))
@@ -132,9 +125,16 @@ class zerobin_db extends zerobin_abstract
                         throw new Exception(
                             'PDO type ' .
                             self::$_type .
-                            ' is currently not supported.'
+                            ' is currently not supported.',
+                            5
                         );
                 }
+                self::$_db = new PDO(
+                    $options['dsn'],
+                    $options['usr'],
+                    $options['pwd'],
+                    $options['opt']
+                );
                 $statement = self::$_db->query($sql);
                 $tables = $statement->fetchAll(PDO::FETCH_COLUMN, 0);
 
@@ -266,7 +266,7 @@ class zerobin_db extends zerobin_abstract
             array($pasteid)
         );
         if (
-        		array_key_exists($pasteid, self::$_cache)
+            array_key_exists($pasteid, self::$_cache)
         ) unset(self::$_cache[$pasteid]);
     }
 

tst/bootstrap.php

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

tst/serversalt.php

@@ -0,0 +1,100 @@
+<?php
+class serversaltTest extends PHPUnit_Framework_TestCase
+{
+    private $_path;
+
+    private $_invalidPath;
+
+    private $_otherPath;
+
+    private $_invalidFile;
+
+    public function setUp()
+    {
+        /* Setup Routine */
+        $this->_path = PATH . 'data';
+        if(!is_dir($this->_path)) mkdir($this->_path);
+        serversalt::setPath($this->_path);
+
+        $this->_otherPath = $this->_path . DIRECTORY_SEPARATOR . 'foo';
+
+        $this->_invalidPath = $this->_path . DIRECTORY_SEPARATOR . 'bar';
+        if(!is_dir($this->_invalidPath)) mkdir($this->_invalidPath);
+        $this->_invalidFile = $this->_invalidPath . DIRECTORY_SEPARATOR . 'salt.php';
+    }
+
+    public function tearDown()
+    {
+        /* Tear Down Routine */
+        chmod($this->_invalidPath, 0700);
+        helper::rmdir($this->_path);
+    }
+
+    public function testGeneration()
+    {
+        // generating new salt
+        serversalt::setPath($this->_path);
+        $salt = serversalt::get();
+        require 'mcrypt_mock.php';
+        $this->assertNotEquals($salt, serversalt::generate());
+
+        // try setting a different path and resetting it
+        serversalt::setPath($this->_otherPath);
+        $this->assertNotEquals($salt, serversalt::get());
+        serversalt::setPath($this->_path);
+        $this->assertEquals($salt, serversalt::get());
+    }
+
+    /**
+     * @expectedException Exception
+     * @expectedExceptionCode 11
+     */
+    public function testPathShenanigans()
+    {
+        // try setting an invalid path
+        chmod($this->_invalidPath, 0000);
+        serversalt::setPath($this->_invalidPath);
+        serversalt::get();
+    }
+
+    /**
+     * @expectedException Exception
+     * @expectedExceptionCode 20
+     */
+    public function testFileRead()
+    {
+        // try setting an invalid file
+        chmod($this->_invalidPath, 0700);
+        file_put_contents($this->_invalidFile, '');
+        chmod($this->_invalidFile, 0000);
+        serversalt::setPath($this->_invalidPath);
+        serversalt::get();
+    }
+
+    /**
+     * @expectedException Exception
+     * @expectedExceptionCode 13
+     */
+    public function testFileWrite()
+    {
+        // try setting an invalid file
+        chmod($this->_invalidPath, 0700);
+        @unlink($this->_invalidFile);
+        file_put_contents($this->_invalidPath . DIRECTORY_SEPARATOR . '.htaccess', '');
+        chmod($this->_invalidPath, 0500);
+        serversalt::setPath($this->_invalidPath);
+        serversalt::get();
+    }
+
+    /**
+     * @expectedException Exception
+     * @expectedExceptionCode 10
+     */
+    public function testPermissionShenanigans()
+    {
+        // try creating an invalid path
+        chmod($this->_invalidPath, 0000);
+        serversalt::setPath($this->_invalidPath . DIRECTORY_SEPARATOR . 'baz');
+        serversalt::get();
+    }
+}

tst/vizhash16x16.php

@@ -10,22 +10,20 @@ class vizhash16x16Test extends PHPUnit_Framework_TestCase
     public function setUp()
     {
         /* Setup Routine */
-        $this->_path = PATH . 'data' . DIRECTORY_SEPARATOR;
+        $this->_path = PATH . 'data';
-        $this->_dataDirCreated = !is_dir($this->_path);
+        if(!is_dir($this->_path)) mkdir($this->_path);
-        if($this->_dataDirCreated) mkdir($this->_path);
+        $this->_file = $this->_path . DIRECTORY_SEPARATOR . 'vizhash.png';
-        $this->_file = $this->_path . 'vizhash.png';
+        serversalt::setPath($this->_path);
     }
 
     public function tearDown()
     {
         /* Tear Down Routine */
-    		if($this->_dataDirCreated) {
+        chmod($this->_path, 0700);
-		        helper::rmdir($this->_path);
+        if(!@unlink($this->_file)) {
-        } else {
+            throw new Exception('Error deleting file "' . $this->_file . '".');
-            if(!@unlink($this->_file)) {
-            	throw new Exception('Error deleting file "' . $this->_file . '".');
-            }
         }
+        helper::rmdir($this->_path);
     }
 
     public function testVizhashGeneratesUniquePngsPerIp()
@@ -37,10 +35,5 @@ class vizhash16x16Test extends PHPUnit_Framework_TestCase
         $this->assertEquals('image/png', $finfo->file($this->_file));
         $this->assertNotEquals($pngdata, $vz->generate('2001:1620:2057:dead:beef::cafe:babe'));
         $this->assertEquals($pngdata, $vz->generate('127.0.0.1'));
-
-        // generating new salt
-        $salt = serversalt::get();
-        require 'mcrypt_mock.php';
-        $this->assertNotEquals($salt, serversalt::generate());
     }
 }

tst/zerobin/db.php

@@ -65,4 +65,81 @@ class zerobin_dbTest extends PHPUnit_Framework_TestCase
         $this->assertFalse($this->_model->existsComment(self::$pasteid, self::$pasteid, self::$commentid), 'comment was deleted with paste');
         $this->assertFalse($this->_model->read(self::$pasteid), 'paste can no longer be found');
     }
+
+    /**
+     * @expectedException PDOException
+     */
+    public function testGetIbmInstance()
+    {
+        zerobin_db::getInstance(array(
+            'dsn' => 'ibm:', 'usr' => null, 'pwd' => null,
+            'opt' => array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION)
+        ));
+    }
+
+    /**
+     * @expectedException PDOException
+     */
+    public function testGetInformixInstance()
+    {
+        zerobin_db::getInstance(array(
+            'dsn' => 'informix:', 'usr' => null, 'pwd' => null,
+            'opt' => array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION)
+        ));
+    }
+
+    /**
+     * @expectedException PDOException
+     */
+    public function testGetMssqlInstance()
+    {
+        zerobin_db::getInstance(array(
+            'dsn' => 'mssql:', 'usr' => null, 'pwd' => null,
+            'opt' => array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION)
+        ));
+    }
+
+    /**
+     * @expectedException PDOException
+     */
+    public function testGetMysqlInstance()
+    {
+        zerobin_db::getInstance(array(
+            'dsn' => 'mysql:', 'usr' => null, 'pwd' => null,
+            'opt' => array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION)
+        ));
+    }
+
+    /**
+     * @expectedException PDOException
+     */
+    public function testGetOciInstance()
+    {
+        zerobin_db::getInstance(array(
+            'dsn' => 'oci:', 'usr' => null, 'pwd' => null,
+            'opt' => array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION)
+        ));
+    }
+
+    /**
+     * @expectedException PDOException
+     */
+    public function testGetPgsqlInstance()
+    {
+        zerobin_db::getInstance(array(
+            'dsn' => 'pgsql:', 'usr' => null, 'pwd' => null,
+            'opt' => array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION)
+        ));
+    }
+
+    /**
+     * @expectedException Exception
+     * @expectedExceptionCode 5
+     */
+    public function testGetFooInstance()
+    {
+        zerobin_db::getInstance(array(
+            'dsn' => 'foo:', 'usr' => null, 'pwd' => null, 'opt' => null
+        ));
+    }
 }