From 597e946ab364c52ad114e717d3b482367c847f8d Mon Sep 17 00:00:00 2001
From: Spencer Mortensen <smortensen@datto.com>
Date: Thu, 8 Jun 2017 17:21:53 -0400
Subject: [PATCH] Preserve ids, if possible, and eliminate error suppression

---
 src/Server.php       | 56 ++++++++++++++++++++++++++------------------
 tests/ServerTest.php | 18 ++++++++++++++
 2 files changed, 51 insertions(+), 23 deletions(-)

diff --git a/src/Server.php b/src/Server.php
index 6216b68..06268fd 100644
--- a/src/Server.php
+++ b/src/Server.php
@@ -59,9 +59,7 @@ public function __construct(Evaluator $evaluator)
      */
     public function reply($json)
     {
-        $input = @json_decode($json, true);
-
-        $output = $this->processInput($input);
+        $output = $this->processInput($json);
 
         if ($output === null) {
             return null;
@@ -73,18 +71,24 @@ public function reply($json)
     /**
      * Processes the user input, and prepares a response (if necessary).
      *
-     * @param array $input
-     * Single request object, or an array of request objects.
+     * @param string $json
+     * Single request object, or an array of request objects, as a JSON string.
      *
      * @return array|null
      * Returns a response object (or an error object) when a query is made.
      * Returns an array of response/error objects when multiple queries are made.
      * Returns null when no response is necessary.
      */
-    private function processInput($input)
+    private function processInput($json)
     {
+        if (!is_string($json)) {
+            return self::parseError();
+        }
+
+        $input = json_decode($json, true);
+
         if (!is_array($input)) {
-            return self::jsonError();
+            return self::parseError();
         }
 
         if (count($input) === 0) {
@@ -144,16 +148,25 @@ private function processRequest($request)
             return self::requestError();
         }
 
-        $version = @$request['jsonrpc'];
+        // The presence of the 'id' key indicates that a response is expected
+        $isQuery = array_key_exists('id', $request);
+
+        $id = &$request['id'];
 
-        if (@$version !== self::VERSION) {
+        if (($id !== null) && !is_int($id) && !is_float($id) && !is_string($id)) {
             return self::requestError();
         }
 
-        $method = @$request['method'];
+        $version = &$request['jsonrpc'];
+
+        if ($version !== self::VERSION) {
+            return self::requestError($id);
+        }
+
+        $method = &$request['method'];
 
         if (!is_string($method)) {
-            return self::requestError();
+            return self::requestError($id);
         }
 
         // The 'params' key is optional, but must be non-null when provided
@@ -161,20 +174,13 @@ private function processRequest($request)
             $arguments = $request['params'];
 
             if (!is_array($arguments)) {
-                return self::requestError();
+                return self::requestError($id);
             }
         } else {
             $arguments = array();
         }
 
-        // The presence of the 'id' key indicates that a response is expected
-        if (array_key_exists('id', $request)) {
-            $id = $request['id'];
-
-            if (!is_int($id) && !is_float($id) && !is_string($id) && ($id !== null)) {
-                return self::requestError();
-            }
-
+        if ($isQuery) {
             return $this->processQuery($id, $method, $arguments);
         }
 
@@ -236,7 +242,7 @@ private function processNotification($method, $arguments)
      * @return array
      * Returns an error object.
      */
-    private static function jsonError()
+    private static function parseError()
     {
         return self::error(null, -32700, 'Parse error');
     }
@@ -245,12 +251,16 @@ private static function jsonError()
      * Returns an error object explaining that the JSON input is not a valid
      * request object.
      *
+     * @param mixed $id
+     * Client-supplied value that allows the client to associate the server response
+     * with the original query.
+     *
      * @return array
      * Returns an error object.
      */
-    private static function requestError()
+    private static function requestError($id = null)
     {
-        return self::error(null, -32600, 'Invalid Request');
+        return self::error($id, -32600, 'Invalid Request');
     }
 
     /**
diff --git a/tests/ServerTest.php b/tests/ServerTest.php
index 528e7b6..1cc5b65 100644
--- a/tests/ServerTest.php
+++ b/tests/ServerTest.php
@@ -110,6 +110,24 @@ public function testInvalidJson()
         $this->compare($input, $output);
     }
 
+    public function testMissingJsonrpcVersion()
+    {
+        $input = '{"method": "subtract", "params": [3, 2], "id": 1}';
+
+        $output = '{"jsonrpc": "2.0", "id": 1, "error": {"code": -32600, "message": "Invalid Request"}}';
+
+        $this->compare($input, $output);
+    }
+
+    public function testInvalidJsonrpcVersion()
+    {
+        $input = '{"jsonrpc": "2.1", "id": 1, "method": "subtract", "params": [3, 2]}';
+
+        $output = '{"jsonrpc": "2.0", "id": 1, "error": {"code": -32600, "message": "Invalid Request"}}';
+
+        $this->compare($input, $output);
+    }
+
     public function testInvalidMethod()
     {
         $input = '{"jsonrpc": "2.0", "method": 1, "params": [1, 2]}';