From 6c4b31d47b472054b3850b0beb89a3bde0f4520d Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Tue, 14 Dec 2021 22:23:30 -0800
Subject: [PATCH] Civi::pipe - Comply more strictly with JSON-RPC v2.0. Don't
 allow scalar `params`.

---
 Civi/Pipe/JsonRpc.php                         |  3 +++
 .../phpunit/Civi/Pipe/JsonRpcSessionTest.php  | 24 +++++++++++++------
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/Civi/Pipe/JsonRpc.php b/Civi/Pipe/JsonRpc.php
index 0712dea0d4..02c02abf31 100644
--- a/Civi/Pipe/JsonRpc.php
+++ b/Civi/Pipe/JsonRpc.php
@@ -66,6 +66,9 @@ class JsonRpc {
       if (($request['jsonrpc'] ?? '') !== '2.0' || !is_string($request['method'])) {
         throw new \InvalidArgumentException('Invalid Request', -32600);
       }
+      if (isset($request['params']) && !is_array($request['params'])) {
+        throw new \InvalidArgumentException('Invalid params', -32602);
+      }
 
       $result = $dispatcher($request['method'], $request['params'] ?? []);
       return static::createResponseSuccess($request, $result);
diff --git a/tests/phpunit/Civi/Pipe/JsonRpcSessionTest.php b/tests/phpunit/Civi/Pipe/JsonRpcSessionTest.php
index f1b01d7bd6..df9e2c7d22 100644
--- a/tests/phpunit/Civi/Pipe/JsonRpcSessionTest.php
+++ b/tests/phpunit/Civi/Pipe/JsonRpcSessionTest.php
@@ -35,7 +35,7 @@ class JsonRpcSessionTest extends \CiviUnitTestCase {
     parent::tearDown();
   }
 
-  public function testInvalid() {
+  public function testInvalid_BadMethod() {
     $responseLines = $this->runLines([
       '{"jsonrpc":"2.0","method":"wiggum"}',
     ]);
@@ -45,19 +45,29 @@ class JsonRpcSessionTest extends \CiviUnitTestCase {
     $this->assertEquals(-32601, $decode['error']['code']);
   }
 
+  public function testInvalid_MalformedParams() {
+    $responseLines = $this->runLines([
+      '{"jsonrpc":"2.0","id":"a","method":"echo","params":123}',
+    ]);
+    $decode = json_decode($responseLines[1], 1);
+    $this->assertEquals('2.0', $decode['jsonrpc']);
+    $this->assertEquals('Invalid params', $decode['error']['message']);
+    $this->assertEquals(-32602, $decode['error']['code']);
+  }
+
   public function testEcho() {
     $this->assertRequestResponse([
-      '{"jsonrpc":"2.0","id":"a","method":"echo","params":123}' => '{"jsonrpc":"2.0","result":123,"id":"a"}',
-      '{"jsonrpc":"2.0","id":"a","method":"echo","params":true}' => '{"jsonrpc":"2.0","result":true,"id":"a"}',
+      '{"jsonrpc":"2.0","id":null,"method":"echo"}' => '{"jsonrpc":"2.0","result":[],"id":null}',
+      '{"jsonrpc":"2.0","id":"a","method":"echo","params":{"color":"blue"}}' => '{"jsonrpc":"2.0","result":{"color":"blue"},"id":"a"}',
       '{"jsonrpc":"2.0","id":"a","method":"echo","params":[1,4,9]}' => '{"jsonrpc":"2.0","result":[1,4,9],"id":"a"}',
-      '{"jsonrpc":"2.0","id":null,"method":"echo","params":123}' => '{"jsonrpc":"2.0","result":123,"id":null}',
+      '{"jsonrpc":"2.0","id":null,"method":"echo","params":[123]}' => '{"jsonrpc":"2.0","result":[123],"id":null}',
     ]);
   }
 
   public function testBatch() {
     $batchLine = '[' .
       '{"jsonrpc":"2.0","id":"a","method":"wiggum"},' .
-      '{"jsonrpc":"2.0","id":"b","method": "echo","params":123}' .
+      '{"jsonrpc":"2.0","id":"b","method": "echo","params":[123]}' .
       ']';
     $responseLines = $this->runLines([$batchLine]);
     $decode = json_decode($responseLines[1], 1);
@@ -69,7 +79,7 @@ class JsonRpcSessionTest extends \CiviUnitTestCase {
 
     $this->assertEquals('2.0', $decode[1]['jsonrpc']);
     $this->assertEquals('b', $decode[1]['id']);
-    $this->assertEquals(123, $decode[1]['result']);
+    $this->assertEquals([123], $decode[1]['result']);
   }
 
   public function testInvalidControl() {
@@ -83,7 +93,7 @@ class JsonRpcSessionTest extends \CiviUnitTestCase {
     $this->assertRequestResponse([
       '{"jsonrpc":"2.0","id":"c","method":"options"}' => '{"jsonrpc":"2.0","result":{"apiError":"array","bufferSize":524288,"responsePrefix":null},"id":"c"}',
       '{"jsonrpc":"2.0","id":"c","method":"options","params":{"responsePrefix":"ZZ"}}' => 'ZZ{"jsonrpc":"2.0","result":{"responsePrefix":"ZZ"},"id":"c"}',
-      '{"jsonrpc":"2.0","id":"c","method": "echo","params":123}' => 'ZZ{"jsonrpc":"2.0","result":123,"id":"c"}',
+      '{"jsonrpc":"2.0","id":"c","method": "echo","params":[123]}' => 'ZZ{"jsonrpc":"2.0","result":[123],"id":"c"}',
     ]);
   }
 
-- 
2.25.1