Handle empty array request per spec.

Fix liniting errors.
isaacgr · Oct 10, 2020 · 95692a7 · 95692a7
1 parent a68f697
commit 95692a7
Show file tree

Hide file tree

Showing 9 changed files with 70 additions and 57 deletions.
diff --git a/src/buffer.js b/src/buffer.js
@@ -13,8 +13,8 @@ class MessageBuffer {
 
   isFinished() {
     if (
-      this.buffer.length === 0 ||
-      this.buffer.indexOf(this.delimiter) === -1
+      this.buffer.length === 0
+      || this.buffer.indexOf(this.delimiter) === -1
     ) {
       return true;
     }

diff --git a/src/client-ws/ws.js b/src/client-ws/ws.js
@@ -90,8 +90,7 @@ class WSClient extends EventTarget {
     this._removeListener(method, cb);
 
     // if no more events of the removed event method are left,remove the group
-    if (this.eventListenerList[method].length === 0)
-      delete this.eventListenerList[method];
+    if (this.eventListenerList[method].length === 0) delete this.eventListenerList[method];
   }
 
   unsubscribeAll(method) {

diff --git a/src/client/protocol/base.js b/src/client/protocol/base.js
@@ -79,7 +79,7 @@ class JsonRpcClientProtocol {
       } else if (!(message === Object(message))) {
         // error out if it cant be parsed
         throw SyntaxError();
-      } else if (!message.id) {
+      } else if (!("id" in message)) {
         // no id, so assume notification
         this.handleNotification(message);
       } else if (message.error) {

diff --git a/src/functions.js b/src/functions.js
@@ -106,17 +106,8 @@ const formatError = ({
   return JSON.stringify(response) + delimiter;
 };
 
-class BatchRequest extends Error {
-  constructor(message, request = undefined) {
-    super(message);
-    this.name = "BatchRequest";
-    this.request = request;
-  }
-}
-
 module.exports = {
   formatRequest,
   formatResponse,
-  formatError,
-  BatchRequest
+  formatError
 };
diff --git a/src/server/index.js b/src/server/index.js
@@ -157,7 +157,7 @@ class JsonRpcServerFactory extends EventEmitter {
 
   clientDisconnected(cb) {
     this.on("clientDisconnected", (client) => {
-      const clientIndex = this.connectedClients.findIndex((c) => client === c);
+      const clientIndex = this.connectedClients.findIndex(c => client === c);
       if (clientIndex === -1) {
         return cb(`Unknown client ${JSON.stringify(client)}`);
       }

diff --git a/src/server/protocol/base.js b/src/server/protocol/base.js
@@ -59,11 +59,22 @@ class JsonRpcServerProtocol {
 
   maybeHandleRequest(result) {
     if (Array.isArray(result)) {
+      if (result.length === 0) {
+        return this.writeToClient(
+          formatError({
+            code: ERR_CODES.invalidRequest,
+            message: ERR_MSGS.invalidRequest,
+            delimiter: this.delimiter,
+            jsonrpc: this.version,
+            id: null
+          })
+        );
+      }
       // possible batch request
       this.handleBatchRequest(result).then((res) => {
         this.writeToClient(JSON.stringify(res) + this.delimiter);
       });
-    } else if (result === Object(result) && !result.id) {
+    } else if (result === Object(result) && !("id" in result)) {
       // no id, so assume notification
       this.handleNotification(result);
     } else {
@@ -80,26 +91,26 @@ class JsonRpcServerProtocol {
     } else if (!(typeof message.method === "string")) {
       const code = ERR_CODES.invalidRequest;
       const errorMessage = ERR_MSGS.invalidRequest;
-      const id = message.id;
+      const { id } = message;
       this._raiseError(errorMessage, code, id);
     } else if (!(message.method in this.factory.methods)) {
       const code = ERR_CODES.methodNotFound;
       const errorMessage = ERR_MSGS.methodNotFound;
-      const id = message.id;
+      const { id } = message;
       this._raiseError(errorMessage, code, id);
     } else if (
-      message.params &&
-      !Array.isArray(message.params) &&
-      !(message.params === Object(message.params))
+      message.params
+      && !Array.isArray(message.params)
+      && !(message.params === Object(message.params))
     ) {
       const code = ERR_CODES.invalidParams;
       const errorMessage = ERR_MSGS.invalidParams;
-      const id = message.id;
+      const { id } = message;
       this._raiseError(errorMessage, code, id);
     } else if (message.jsonrpc && this.version !== "2.0") {
       const code = ERR_CODES.invalidRequest;
       const errorMessage = ERR_MSGS.invalidRequest;
-      const id = message.id;
+      const { id } = message;
       this._raiseError(errorMessage, code, id);
     }
   }
@@ -128,14 +139,14 @@ class JsonRpcServerProtocol {
         try {
           this.maybeHandleRequest(request);
           return this.getResult(request)
-            .then((result) => JSON.parse(result))
-            .catch((error) => JSON.parse(error));
+            .then(result => JSON.parse(result))
+            .catch(error => JSON.parse(error));
         } catch (e) {
           // basically reject the whole batch if any one thing fails
           return JSON.parse(e.message);
         }
       })
-      .filter((el) => el != null);
+      .filter(el => el != null);
     return Promise.all(batchResponses);
   }
 
@@ -158,8 +169,8 @@ class JsonRpcServerProtocol {
           ? this.factory.methods[message.method](params)
           : this.factory.methods[message.method]();
         if (
-          result &&
-          (typeof result.then === "function" || result instanceof Promise)
+          result
+          && (typeof result.then === "function" || result instanceof Promise)
         ) {
           Promise.all([result])
             .then((results) => {

diff --git a/tests/client/tcp-client.test.js b/tests/client/tcp-client.test.js
@@ -146,7 +146,7 @@ describe("TCP Client", () => {
         setTimeout(() => {
           unhook();
           expect(capturedText).to.equal(
-            'Message has no outstanding calls: {"jsonrpc":"2.0","error":{"code":-32700,"message":"Unable to parse message: \'should get a parse error\\r\'"},"id":null}\n'
+            "Message has no outstanding calls: {\"jsonrpc\":\"2.0\",\"error\":{\"code\":-32700,\"message\":\"Unable to parse message: 'should get a parse error\\r'\"},\"id\":null}\n"
           );
           done();
         }, 100);
@@ -184,7 +184,21 @@ describe("TCP Client", () => {
         done();
       });
     });
-    it("should receive 'invalid request' error for non empty array", (done) => {
+    it("should receive 'invalid request' error for and empty array", (done) => {
+      let capturedText = "";
+      const unhook = intercept((text) => {
+        capturedText += text;
+      });
+      client.batch([]);
+      setTimeout(() => {
+        unhook();
+        expect(capturedText).to.equal(
+          "Message has no outstanding calls: {\"jsonrpc\":\"2.0\",\"error\":{\"code\":-32600,\"message\":\"Invalid Request\"},\"id\":null}\n"
+        );
+        done();
+      }, 100);
+    });
+    it("should receive 'invalid request' error in an array for non empty array", (done) => {
       const request = client.batch([1]);
       request.catch((response) => {
         expect(response).to.eql([
@@ -319,29 +333,29 @@ describe("TCP Client", () => {
         ["test", []]
       ]);
     });
-    it('should be unable to subscribe, unsub, or unsub all for "batchResponse"', (done) => {
+    it("should be unable to subscribe, unsub, or unsub all for \"batchResponse\"", (done) => {
       try {
         client.subscribe("batchResponse", () => {});
       } catch (e) {
         expect(e.message).to.be.a(
           "string",
-          '"batchResponse" is a reserved event name'
+          "\"batchResponse\" is a reserved event name"
         );
       }
       try {
         client.unsubscribe("batchResponse", () => {});
       } catch (e) {
         expect(e.message).to.be.a(
           "string",
-          '"batchResponse" is a reserved event name'
+          "\"batchResponse\" is a reserved event name"
         );
       }
       try {
         client.unsubscribeAll("batchResponse", () => {});
       } catch (e) {
         expect(e.message).to.be.a(
           "string",
-          '"batchResponse" is a reserved event name'
+          "\"batchResponse\" is a reserved event name"
         );
       }
       done();

diff --git a/tests/server/tcp-server.test.js b/tests/server/tcp-server.test.js
@@ -15,21 +15,19 @@ server.method("typeerror", ([a]) => {
 });
 server.method(
   "promiseresolve",
-  () =>
-    new Promise((resolve) => {
-      setTimeout(() => {
-        resolve("worked");
-      }, 10);
-    })
+  () => new Promise((resolve) => {
+    setTimeout(() => {
+      resolve("worked");
+    }, 10);
+  })
 );
 server.method(
   "promisereject",
-  () =>
-    new Promise((resolve, reject) => {
-      setTimeout(() => {
-        reject(new Error("broke"));
-      }, 10);
-    })
+  () => new Promise((resolve, reject) => {
+    setTimeout(() => {
+      reject(new Error("broke"));
+    }, 10);
+  })
 );
 
 before((done) => {
@@ -238,7 +236,7 @@ describe("TCP Server", () => {
         .catch((result) => {
           expect(result).to.be.eql({
             jsonrpc: "2.0",
-            error: { code: -32603, message: '"broke"' },
+            error: { code: -32603, message: "\"broke\"" },
             id: 6
           });
           done();

diff --git a/tests/util/components.test.js b/tests/util/components.test.js
@@ -108,7 +108,7 @@ describe("formatRequest", () => {
         options: { delimiter: "\n" }
       };
       const message = formatRequest(params);
-      expect(message).to.be.eql('{"method":"test","jsonrpc":"2.0","id":1}\n');
+      expect(message).to.be.eql("{\"method\":\"test\",\"jsonrpc\":\"2.0\",\"id\":1}\n");
       done();
     });
   });
@@ -185,7 +185,7 @@ describe("formatResponse", () => {
           delimiter: "\n"
         };
         const response = formatResponse(params);
-        expect(response).to.eql('{"result":19,"jsonrpc":"2.0","id":1}\n');
+        expect(response).to.eql("{\"result\":19,\"jsonrpc\":\"2.0\",\"id\":1}\n");
         done();
       });
       it("should return notification with params", (done) => {
@@ -197,7 +197,7 @@ describe("formatResponse", () => {
         };
         const response = formatResponse(params);
         expect(response).to.eql(
-          '{"params":[1,2,3,4,5],"jsonrpc":"2.0","method":"update"}\n'
+          "{\"params\":[1,2,3,4,5],\"jsonrpc\":\"2.0\",\"method\":\"update\"}\n"
         );
         done();
       });
@@ -211,7 +211,7 @@ describe("formatResponse", () => {
         };
         const response = formatResponse(params);
         expect(response).to.eql(
-          '{"result":"Hello JSON-RPC","error":null,"id":1}\n'
+          "{\"result\":\"Hello JSON-RPC\",\"error\":null,\"id\":1}\n"
         );
         done();
       });
@@ -223,7 +223,7 @@ describe("formatResponse", () => {
         };
         const response = formatResponse(params);
         expect(response).to.eql(
-          '{"params":[1,2,3,4,5],"error":null,"method":"update","id":null}\n'
+          "{\"params\":[1,2,3,4,5],\"error\":null,\"method\":\"update\",\"id\":null}\n"
         );
         done();
       });
@@ -259,7 +259,7 @@ describe("formatError", () => {
       };
       const response = formatError(params);
       expect(response).to.eql(
-        '{"jsonrpc":"2.0","error":{"code":-32601,"message":"Method not found"},"id":1}\n'
+        "{\"jsonrpc\":\"2.0\",\"error\":{\"code\":-32601,\"message\":\"Method not found\"},\"id\":1}\n"
       );
       done();
     });
@@ -272,7 +272,7 @@ describe("formatError", () => {
       };
       const response = formatError(params);
       expect(response).to.eql(
-        '{"result":null,"error":{"code":-32601,"message":"Method not found"},"id":1}\n'
+        "{\"result\":null,\"error\":{\"code\":-32601,\"message\":\"Method not found\"},\"id\":1}\n"
       );
       done();
     });
@@ -287,7 +287,7 @@ describe("formatError", () => {
       };
       const response = formatError(params);
       expect(response).to.eql(
-        '{"jsonrpc":"2.0","error":{"code":-32601,"message":"Method not found","data":[1,2,3,4,5]},"id":1}\n'
+        "{\"jsonrpc\":\"2.0\",\"error\":{\"code\":-32601,\"message\":\"Method not found\",\"data\":[1,2,3,4,5]},\"id\":1}\n"
       );
       done();
     });
@@ -305,7 +305,7 @@ describe("getResult()", () => {
       })
       .catch((error) => {
         expect(error).to.eql(
-          '{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid Parameters"},"id":1}\n'
+          "{\"jsonrpc\":\"2.0\",\"error\":{\"code\":-32602,\"message\":\"Invalid Parameters\"},\"id\":1}\n"
         );
         done();
       });