Sanity/output checking on unpacked data

horde · Nov 3, 2013 · 6b4f6bb · 6b4f6bb
1 parent a23d40e
commit 6b4f6bb
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 15 deletions.
diff --git a/framework/Pack/lib/Horde/Pack/Driver/Igbinary.php b/framework/Pack/lib/Horde/Pack/Driver/Igbinary.php
@@ -48,7 +48,12 @@ public function pack($data)
      */
     public function unpack($data)
     {
-        return igbinary_unserialize($data);
+        $out = igbinary_unserialize($data);
+        if (!is_null($out) || ($data == igbinary_serialize(null))) {
+            return $out;
+        }
+
+        throw new Horde_Pack_Exception('Error when unpacking serialized data.');
     }
 
 }
diff --git a/framework/Pack/lib/Horde/Pack/Driver/Json.php b/framework/Pack/lib/Horde/Pack/Driver/Json.php
@@ -49,7 +49,12 @@ public function pack($data)
      */
     public function unpack($data)
     {
-        return json_decode(substr($data, 1), ($data[0] == 1));
+        $out = json_decode(substr($data, 1), ($data[0] == 1));
+        if (!is_null($out) || (json_last_error() === JSON_ERROR_NONE)) {
+            return $out;
+        }
+
+        throw new Horde_Pack_Exception('Error when unpacking JSON data.');
     }
 
 }
diff --git a/framework/Pack/lib/Horde/Pack/Driver/Msgpack.php b/framework/Pack/lib/Horde/Pack/Driver/Msgpack.php
@@ -44,7 +44,15 @@ public function pack($data)
      */
     public function unpack($data)
     {
-        return msgpack_unpack($data);
+        ini_set('track_errors', 1);
+        $out = @msgpack_unpack($data);
+        ini_restore('track_errors');
+
+        if (!isset($php_errormsg)) {
+            return $out;
+        }
+
+        throw new Horde_Pack_Exception('Error when unpacking Msgpack data.');
     }
 
 }
diff --git a/framework/Pack/lib/Horde/Pack/Driver/Msgpackserialize.php b/framework/Pack/lib/Horde/Pack/Driver/Msgpackserialize.php
@@ -50,7 +50,15 @@ public function pack($data)
      */
     public function unpack($data)
     {
-        return msgpack_unserialize($data);
+        ini_set('track_errors', 1);
+        $out = @msgpack_unserialize($data);
+        ini_restore('track_errors');
+
+        if (!isset($php_errormsg)) {
+            return $out;
+        }
+
+        throw new Horde_Pack_Exception('Error when unpacking Msgpack data.');
     }
 
 }
diff --git a/framework/Pack/lib/Horde/Pack/Driver/Serialize.php b/framework/Pack/lib/Horde/Pack/Driver/Serialize.php
@@ -41,7 +41,12 @@ public function pack($data)
      */
     public function unpack($data)
     {
-        return unserialize($data);
+        $out = @unserialize($data);
+        if (($out !== false) || ($out == serialize(false))) {
+            return $out;
+        }
+
+        throw new Horde_Pack_Exception('Error when unpacking serialized data.');
     }
 
 }
diff --git a/framework/Pack/test/Horde/Pack/Driver/TestBase.php b/framework/Pack/test/Horde/Pack/Driver/TestBase.php
@@ -43,6 +43,16 @@ protected function setUp()
         }
     }
 
+    public function testNull()
+    {
+        $this->_runTest(null);
+    }
+
+    public function testNullWithCompression()
+    {
+        $this->_runTest(null, true);
+    }
+
     public function testBoolean()
     {
         $this->_runTest(true);
@@ -118,31 +128,50 @@ public function testPhpObjectWithCompression()
         }
     }
 
+    /**
+     * @expectedException Horde_Pack_Exception
+     */
+    public function testExpectedExceptionOnBadUnpack()
+    {
+        $packed = $this->_pack(true, false);
+        self::$pack->unpack($packed[0] . "A{{}");
+    }
+
     /* Internal methods. */
 
     protected function _runTest($data, $compress = false)
     {
-        $packed = self::$pack->pack(
-            $data,
-            array(
-                'compress' => $compress ? 0 : false,
-                'drivers' => array(
-                    $this->drivername
-                )
-            )
-        );
+        $packed = $this->_pack($data, $compress);
 
         $this->assertNotEquals(
             $packed,
             $data
         );
 
-        $unpacked = self::$pack->unpack($packed);
+        $unpacked = $this->_unpack($packed);
 
         $this->assertEquals(
             $data,
             $unpacked
         );
     }
 
+    protected function _pack($data, $compress)
+    {
+        return self::$pack->pack(
+            $data,
+            array(
+                'compress' => $compress ? 0 : false,
+                'drivers' => array(
+                    $this->drivername
+                )
+            )
+        );
+    }
+
+    protected function _unpack($data)
+    {
+        return self::$pack->unpack($data);
+    }
+
 }