Handle null or non-JSON result of cPanel UAPI

More unit testing. More documentation.
roundcube · Dec 9, 2016 · bd5eaf9 · bd5eaf9
1 parent dd29ca1
commit bd5eaf9
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 20 deletions.
diff --git a/plugins/password/drivers/cpanel_webmail.php b/plugins/password/drivers/cpanel_webmail.php
@@ -28,6 +28,15 @@
 
 class rcube_cpanel_webmail_password
 {
+    /**
+     * Changes the user's password. It is called by password.php.
+     * See "Driver API" README and password.php for the interface details.
+     *
+     * @param string $curpas current (old) password
+     * @param string $newpass new requested password
+     * @return mixed int code or assoc array with 'code' and 'message', see
+     * "Driver API" README and password.php
+     */
     public function save($curpas, $newpass)
     {
         $user    = $_SESSION['username'];
@@ -66,17 +75,19 @@ public static function url()
      *
      * @param string $response JSON response by the Cpanel UAPI
      *
-     * @return mixed response code or array
+     * @return mixed response code or array, see <code>save</code>
      */
     public static function decode_response($response)
     {
         if (!$response) {
             return PASSWORD_CONNECT_ERROR;
         }
 
-        $result = json_decode($response);
+        // $result should be `null` or `stdClass` object
+        $result = json_decode($response, !JSON_OBJECT_AS_ARRAY);
 
-        if ($result->status === 1) {
+        // The UAPI may return HTML instead of JSON on missing authentication
+        if (is_object($result) && $result->status === 1) {
             return PASSWORD_SUCCESS;
         }
 

diff --git a/plugins/password/tests/Password.php b/plugins/password/tests/Password.php
@@ -20,27 +20,58 @@ function test_constructor()
         $this->assertInstanceOf('rcube_plugin', $plugin);
     }
 
+    /**
+     * cpanel_webmail driver test
+     */
     function test_driver_cpanel_webmail()
     {
-        $driver = 'cpanel_webmail';
+        $driver_class = $this->load_driver('cpanel_webmail');
+
+        $error_result = $driver_class::decode_response(false);
+        $this->assertEquals($error_result, PASSWORD_CONNECT_ERROR);
+
+        $bad_result = $driver_class::decode_response(null);
+        $this->assertEquals($bad_result, PASSWORD_CONNECT_ERROR);
+
+        $null_result = $driver_class::decode_response('null');
+        $this->assertEquals($null_result, PASSWORD_ERROR);
+
+        $malformed_result = $driver_class::decode_response('random {string]!');
+        $this->assertEquals($malformed_result, PASSWORD_ERROR);
+
+        $other_result = $driver_class::decode_response('{"a":"b"}');
+        $this->assertEquals($other_result, PASSWORD_ERROR);
+
+        $fail_response   = '{"data":null,"errors":["Execution of Email::passwdp'
+                . 'op (api version:3) is not permitted inside of webmail"],"sta'
+                . 'tus":0,"metadata":{},"messages":null}';
+        $error_message   = 'Execution of Email::passwdpop (api version:3) is no'
+                . 't permitted inside of webmail';
+        $expected_result = array(
+            'code'    => PASSWORD_ERROR,
+            'message' => $error_message
+        );
+        $fail_result     = $driver_class::decode_response($fail_response);
+        $this->assertEquals($expected_result, $fail_result);
+
+        $success_response = '{"metadata":{},"data":null,"messages":null,"errors'
+                . '":null,"status":1}';
+        $good_result      = $driver_class::decode_response($success_response);
+        $this->assertEquals($good_result, PASSWORD_SUCCESS);
+    }
+
+    /**
+     * Loads a driver's source file, checks that its class exist and returns the
+     * driver's class name.
+     *
+     * @param string $driver driver name, example: "chpasswd"
+     * @return string driver's class name, example: "rcube_chpasswd_password"
+     */
+    function load_driver($driver)
+    {
         include_once __DIR__ . "/../drivers/$driver.php";
         $driver_class = "rcube_${driver}_password";
         $this->assertTrue(class_exists($driver_class));
-
-        $json_response_fail = '{"data":null,"errors":'
-                . '["Execution of Email::passwdpop (api version:3) is not '
-                . 'permitted inside of webmail"],"status":0,"metadata":{},'
-                . '"messages":null}';
-        $result = $driver_class::decode_response($json_response_fail);
-        $this->assertTrue(is_array($result));
-        $this->assertEquals($result['code'], PASSWORD_ERROR);
-        $expected_message = 'Execution of Email::passwdpop (api version:3) is'
-                . ' not permitted inside of webmail';
-        $this->assertEquals($result['message'], $expected_message);
-
-        $json_response_success = '{"metadata":{},"data":null,"messages":null,'
-                . '"errors":null,"status":1}';
-        $result = $driver_class::decode_response($json_response_success);
-        $this->assertEquals($result, PASSWORD_SUCCESS);
+        return $driver_class;
     }
 }