ensure ALL read errors are only exposed in the JSON API to avoid information leakage (i.e. beviour for deleted vs expired pastes), updated test cases & removed duplicate test

lib/PrivateBin.php

@@ -356,38 +356,31 @@ class PrivateBin
     }
 
     /**
-     * Read an existing paste or comment
+     * Read an existing paste or comment, only allowed via a JSON API call
      *
      * @access private
      * @param  string $dataid
      */
     private function _read($dataid)
     {
+        if (!$this->_request->isJsonApiCall()) {
+            return;
+        }
+
         try {
             $paste = $this->_model->getPaste($dataid);
             if ($paste->exists()) {
-                // reading paste is only possible via JSON call
+                $data              = $paste->get();
-                if ($this->_request->isJsonApiCall()) {
+                $this->_doesExpire = property_exists($data, 'meta') && property_exists($data->meta, 'expire_date');
-                    $data              = $paste->get();
+                if (property_exists($data->meta, 'salt')) {
-                    $this->_doesExpire = property_exists($data, 'meta') && property_exists($data->meta, 'expire_date');
+                    unset($data->meta->salt);
-                    if (property_exists($data->meta, 'salt')) {
-                        unset($data->meta->salt);
-                    }
-                    $this->_data = json_encode($data);
                 }
+                $this->_return_message(0, $dataid, (array) $data);
             } else {
-                $this->_error = self::GENERIC_ERROR;
+                $this->_return_message(1, self::GENERIC_ERROR);
             }
         } catch (Exception $e) {
-            $this->_error = $e->getMessage();
+            $this->_return_message(1, $e->getMessage());
-        }
-
-        if ($this->_request->isJsonApiCall()) {
-            if (strlen($this->_error)) {
-                $this->_return_message(1, $this->_error);
-            } else {
-                $this->_return_message(0, $dataid, json_decode($this->_data, true));
-            }
         }
     }
 

tst/PrivateBinTest.php

@@ -679,16 +679,15 @@ class PrivateBinTest extends PHPUnit_Framework_TestCase
      */
     public function testReadInvalidId()
     {
-        $_SERVER['QUERY_STRING'] = 'foo';
+        $_SERVER['QUERY_STRING']          = 'foo';
+        $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest';
         ob_start();
         new PrivateBin;
         $content = ob_get_contents();
         ob_end_clean();
-        $this->assertRegExp(
+        $response = json_decode($content, true);
-            '#<div[^>]*id="errormessage"[^>]*>.*Invalid paste ID\.#s',
+        $this->assertEquals(1, $response['status'], 'outputs error status');
-            $content,
+        $this->assertEquals('Invalid paste ID.', $response['message'], 'outputs error message');
-            'outputs error correctly'
-        );
     }
 
     /**
@@ -696,16 +695,15 @@ class PrivateBinTest extends PHPUnit_Framework_TestCase
      */
     public function testReadNonexisting()
     {
-        $_SERVER['QUERY_STRING'] = Helper::getPasteId();
+        $_SERVER['QUERY_STRING']          = Helper::getPasteId();
+        $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest';
         ob_start();
         new PrivateBin;
         $content = ob_get_contents();
         ob_end_clean();
-        $this->assertRegExp(
+        $response = json_decode($content, true);
-            '#<div[^>]*id="errormessage"[^>]*>.*Paste does not exist, has expired or has been deleted\.#s',
+        $this->assertEquals(1, $response['status'], 'outputs error status');
-            $content,
+        $this->assertEquals('Paste does not exist, has expired or has been deleted.', $response['message'], 'outputs error message');
-            'outputs error correctly'
-        );
     }
 
     /**
@@ -779,21 +777,6 @@ class PrivateBinTest extends PHPUnit_Framework_TestCase
         $this->assertEquals(0, $response['comment_offset'], 'outputs comment_offset correctly');
     }
 
-    /**
-     * @runInSeparateProcess
-     */
-    public function testReadInvalidJson()
-    {
-        $_SERVER['QUERY_STRING']          = Helper::getPasteId();
-        $_SERVER['HTTP_X_REQUESTED_WITH'] = 'JSONHttpRequest';
-        ob_start();
-        new PrivateBin;
-        $content = ob_get_contents();
-        ob_end_clean();
-        $response = json_decode($content, true);
-        $this->assertEquals(1, $response['status'], 'outputs error status');
-    }
-
     /**
      * @runInSeparateProcess
      */