Bugfix/all node ssl options (#105)

* Update README for http ssl option

* Add sslOptions attribute and spread that for the https server options

* Update https test server to use ssl option

* Add new emptyBuffer method to dump the contents of the message buffer

* Push request chunks to message buffer and validate entire buffer

- no delimiter used since one is not typically sent for large json data

* Add test for emptyBuffer

* Register 'end' event for http listener to emptyBuffer on response complete

* Call on('end') listener outside of data listener and check if buffer is empty

- prevents maxEventListeners warning
- dont try to parse empty buffer string

* Add generic _removeFromArray method
- used to remove clients from this.clients
- used to remove pendingRequests from this.pendingRequests

* - Return false from _maybeHandleRequest if notification received
- Dont send a response to a batch request if the respnse is empty
- Only attempt to get results for batch request messages if they are not notifications

README.md

@@ -6,8 +6,8 @@
   - [Download & Installation](#download--installation)
   - [Class Documentation](#class-documentation)
   - [CLI tool](#cli-tool)
-    - [Install](#install)
-    - [Usage](#usage)
+      - [Install](#install)
+      - [Usage](#usage)
   - [Use of the library as a standalone module](#use-of-the-library-as-a-standalone-module)
     - [Initialization](#initialization)
       - [WS Client for browser](#ws-client-for-browser)
@@ -18,10 +18,10 @@
       - [Other client and server options](#other-client-and-server-options)
     - [Code Demos](#code-demos)
       - [Initialization](#initialization-1)
-        - [TCP](#tcp)
-        - [HTTP](#http)
-        - [HTTPS](#https)
-        - [Websocket](#websocket)
+          - [TCP](#tcp)
+          - [HTTP](#http)
+          - [HTTPS](#https)
+          - [Websocket](#websocket)
       - [Server side](#server-side)
         - [Instantiation and Listening](#instantiation-and-listening)
         - [Closing the connection](#closing-the-connection)
@@ -297,7 +297,8 @@ const server = new Jaysonic.server.http()
 
 Pass `https` to the `sheme` option to use an https client or server.
 
-The https server requires an SSL key and certificate file.
+The https server requires an SSL key and certificate file. Valid options for the `ssl` object are any of the options
+given by https://nodejs.org/api/tls.html#tls_tls_createsecurecontext_options.
 
 ```js
 const Jaysonic = require("jaysonic");
@@ -307,9 +308,11 @@ const client = new Jaysonic.client.http({ scheme: "https" });
 
 // https server
 const server = new Jaysonic.server.http({
-  scheme: "http",
-  key: fs.readFileSync("key.pem"),
-  cert: fs.readFileSync("server.crt")
+  scheme: "https",
+  ssl: {
+    key: fs.readFileSync("key.pem"),
+    cert: fs.readFileSync("server.crt")
+  }
 });
 ```
 

src/client/protocol/http.js

@@ -74,14 +74,19 @@ class HttpClientProtocol extends JsonRpcClientProtocol {
   /**
    * Setup `this.listener.on("data")` event to listen for data coming into the client.
    *
-   * The HTTP client does not use the messageBuffer since each request should
-   * only receive one response at a time.
+   * The HTTP client does not use the delimiter since the completion of a response indicates
+   * the end of data.
    *
    * Calls [_waitForData]{@link JsonRpcClientProtocol#_waitForData}
    */
   listen() {
-    this.listener.on("data", (data) => {
-      this._waitForData(data);
+    this.listener.on("data", (chunk) => {
+      this.messageBuffer.push(chunk);
+    });
+    this.listener.on("end", () => {
+      if (this.messageBuffer.buffer !== "") {
+        this._waitForData(this.messageBuffer.emptyBuffer());
+      }
     });
   }
 

src/server/http.js

@@ -14,15 +14,14 @@ class HttpServerFactory extends JsonRpcServerFactory {
    * the WsServerProtocol has the following properties:
    *
    * @property {'http'|'https'} scheme The scheme to allow connections with
-   * @property {file} key The private SSL key file
-   * @property {file} cert The SSL certificate file
+   * @property {object} sslOptions Any of the ssl options for the http server according to https://nodejs.org/api/tls.html#tls_tls_createsecurecontext_options
    */
   constructor(options) {
     super(options);
     this.scheme = this.options.scheme || "http";
-    this.key = this.options.key;
-    this.cert = this.options.cert;
+    this.sslOptions = this.options.ssl;
     this.protocol = HttpServerProtocol;
+    this.pendingRequests = [];
   }
 
   /** @inheritdoc */
@@ -31,8 +30,7 @@ class HttpServerFactory extends JsonRpcServerFactory {
       this.server = new http.Server();
     } else if (this.scheme === "https") {
       this.server = new https.Server({
-        key: this.key,
-        cert: this.cert
+        ...this.sslOptions
       });
     } else {
       throw Error("Invalid scheme");
@@ -58,47 +56,19 @@ class HttpServerFactory extends JsonRpcServerFactory {
         this.options.delimiter
       );
       pcol.clientConnected();
+      this.pendingRequests.push(pcol);
     });
   }
 
   /**
-   * Called when client receives a `connection` event.
+   * Called when the server receives a `connection` event.
    *
-   * @param {stream.Duplex} client Instance of `stream.Duplex`
-   * @returns {stream.Duplex} Returns an instance of `stream.Duplex`
+   * @param {net.Socket} client Instance of `net.Socket`
+   * @returns {net.Socket} Returns an instance of `net.Socket`
    */
   clientConnected(client) {
     return client;
   }
-
-  /**
-   * Called when client disconnects from server.
-   *
-   * @param {stream.Duplex} client Instance of `stream.Duplex`
-   * @returns {object|error} Returns an object of {host, port} for the given protocol instance, or {error}
-   * if there was an error retrieving the client
-   */
-  clientDisconnected(client) {
-    return this.removeDisconnectedClient(client);
-  }
-
-  /**
-   * Removes disconnected client from `this.clients` list
-   *
-   * @param {stream.Duplex} client Instance of `stream.Duplex`
-   * @returns {object|error} Returns an object of {host, port} for the given protocol instance, or {error}
-   * if there was an error retrieving the client
-   */
-  removeDisconnectedClient(client) {
-    const clientIndex = this.clients.findIndex(c => c === client);
-    if (clientIndex === -1) {
-      return {
-        error: `Unknown client ${JSON.stringify(client)}`
-      };
-    }
-    const [disconnectedClient] = this.clients.splice(clientIndex, 1);
-    return disconnectedClient;
-  }
 }
 
 module.exports = HttpServerFactory;

src/server/index.js

@@ -295,33 +295,36 @@ class JsonRpcServerFactory extends EventEmitter {
   /**
    * Called when client disconnects from server.
    *
-   * If overwriting, its recommended to call {@link JsonRpcServerFactory.removeDisconnectedClient} manually
+   * If overwriting, its recommended to call {@link JsonRpcServerFactory._removeFromArray} manually
    * to ensure `this.clients` is cleaned up
    *
+   * Calls `this._removeFromArray` and removes disconnected client from `this.clients` list
+   *
    * @param {JsonRpcServerProtocol} pcol A {@link JsonRpcServerProtocol} instance
    * @returns {object|error} Returns an object of {host, port} for the given protocol instance, or {error}
    * if there was an error retrieving the client
    */
   clientDisconnected(pcol) {
-    return this.removeDisconnectedClient(pcol);
+    return this._removeFromArray(pcol, this.clients);
   }
 
   /**
-   * Removes disconnected client from `this.clients` list
+   * Removes item from the given list
    *
-   * @param {JsonRpcServerProtocol} pcol A {@link JsonRpcServerProtocol} instance
-   * @returns {object|error} Returns an object of {host, port} for the given protocol instance, or {error}
+   * @param {*} item Any item in the list
+   * @param {array} array The array to remove the item from
+   * @returns {*|error} Returns the item that was removed from the array or {error}
    *
    */
-  removeDisconnectedClient(pcol) {
-    const clientIndex = this.clients.findIndex(p => p === pcol);
-    if (clientIndex === -1) {
+  _removeFromArray(item, array) {
+    const itemIndex = array.findIndex(c => c === item);
+    if (itemIndex === -1) {
       return {
-        error: `Unknown client ${JSON.stringify(pcol)}`
+        error: `Unable to remove ${JSON.stringify(item)}`
       };
     }
-    const [protocol] = this.clients.splice(clientIndex, 1);
-    return protocol.client;
+    const [removedItem] = array.splice(itemIndex, 1);
+    return removedItem;
   }
 
   /**

src/server/protocol/base.js

@@ -127,11 +127,16 @@ class JsonRpcServerProtocol {
       }
       // possible batch request
       this.gotBatchRequest(result).then((res) => {
-        this.writeToClient(JSON.stringify(res) + this.delimiter);
+        if (res.length !== 0) {
+          // if all the messages in the batch were notifications,
+          // then we wouldnt want to return anything
+          this.writeToClient(JSON.stringify(res) + this.delimiter);
+        }
       });
     } else if (result === Object(result) && !("id" in result)) {
       // no id, so assume notification
       this.gotNotification(result);
+      return false;
     } else {
       this.validateMessage(result);
       return true;
@@ -243,10 +248,14 @@ class JsonRpcServerProtocol {
     const batchResponses = requests
       .map((request) => {
         try {
-          this._maybeHandleRequest(request);
-          return this.getResult(request)
-            .then(result => JSON.parse(result))
-            .catch(error => JSON.parse(error));
+          const isMessage = this._maybeHandleRequest(request);
+          if (isMessage) {
+            // if its a notification we dont want to return anything
+            return this.getResult(request)
+              .then(result => JSON.parse(result))
+              .catch(error => JSON.parse(error));
+          }
+          return null;
         } catch (e) {
           // basically reject the whole batch if any one thing fails
           return JSON.parse(e.message);

src/server/protocol/http.js

@@ -65,14 +65,18 @@ class HttpServerProtocol extends JsonRpcServerProtocol {
    * Pushes received data into `messageBuffer` and calls
    * [_waitForData]{@link JsonRpcServerProtocol#_waitForData}.
    *
+   * The HTTP server does not use the delimiter since the completion of a request indicates
+   * the end of data.
+   *
    * @extends JsonRpcServerProtocol.clientConnected
    */
   clientConnected() {
-    this.client.on(this.event, (data) => {
-      this.client.on("end", () => {
-        this.messageBuffer.push(data);
-        this._waitForData();
-      });
+    this.client.on(this.event, (chunk) => {
+      this.messageBuffer.push(chunk);
+    });
+    this.client.on("end", () => {
+      this._validateData(this.messageBuffer.emptyBuffer());
+      this.factory._removeFromArray(this, this.factory.pendingRequests);
     });
   }
 

src/util/buffer.js

@@ -81,6 +81,20 @@ class MessageBuffer {
     return null;
   }
 
+  /**
+   *
+   * Returns the contents of `this.buffer`.
+   *
+   * Particularily useful for http buffering, where delimiters might not be used.
+   *
+   * @returns {string} message
+   */
+  emptyBuffer() {
+    const data = this.buffer;
+    this.buffer = "";
+    return data;
+  }
+
   /**
    * @returns {function} MessageBuffer.getMessage()
    */

tests/client/http-client.test.js

@@ -1,8 +1,12 @@
 const { expect } = require("chai");
+const chai = require("chai");
+const spies = require("chai-spies");
 const Jaysonic = require("../../src");
 const data = require("../large-data.json");
 const { serverHttp } = require("../test-server");
 
+chai.use(spies);
+
 const clientHttp = new Jaysonic.client.http();
 
 describe("HTTP Client", () => {

tests/client/tcp-client.test.js

@@ -287,7 +287,7 @@ describe("TCP Client", () => {
       });
     });
   });
-  describe("notifications", () => {
+  describe("receiving notifications", () => {
     it("should handle receiving a notification", (done) => {
       const callback = (message) => {
         expect(message).to.be.eql({

tests/server/http-server.test.js

@@ -29,11 +29,11 @@ describe("Http Server", () => {
     it("should return 'Unknown client' from clientDisconnected if client was not found", (done) => {
       let res;
       serverHttp.clientDisconnected = (client) => {
-        res = serverHttp.removeDisconnectedClient(client);
+        res = serverHttp._removeFromArray(client, serverHttp.clients);
       };
       serverHttp.clientDisconnected({});
       try {
-        expect(res).to.be.eql({ error: "Unknown client {}" });
+        expect(res).to.be.eql({ error: "Unable to remove {}" });
         done();
       } catch (e) {
         console.log(e);

tests/server/tcp-server.test.js

@@ -71,10 +71,10 @@ describe("TCP Server", () => {
     it("should return 'Unknown client' from clientDisconnected if client was not found", (done) => {
       let res;
       server.clientDisconnected = (pcol) => {
-        res = server.removeDisconnectedClient(pcol);
+        res = server._removeFromArray(pcol, server.clients);
       };
       server.clientDisconnected({});
-      expect(res).to.be.eql({ error: "Unknown client {}" });
+      expect(res).to.be.eql({ error: "Unable to remove {}" });
       done();
     });
     it("should be unable to listen multiple times", (done) => {

tests/test-server.js

@@ -13,9 +13,11 @@ const serverHttp = new Jaysonic.server.http();
 const serverHttpV1 = new Jaysonic.server.http({ version: 1 });
 const serverHttps = new Jaysonic.server.http({
   scheme: "https",
-  key: fs.readFileSync("tests/key.pem"),
-  cert: fs.readFileSync("tests/server.crt"),
-  ca: "selfSignedRootCaPemCrtBuffer"
+  ssl: {
+    key: fs.readFileSync("tests/key.pem"),
+    cert: fs.readFileSync("tests/server.crt"),
+    ca: "selfSignedRootCaPemCrtBuffer"
+  }
 });
 const wss = new Jaysonic.server.ws();
 

tests/util/message-buffer.test.js

@@ -33,14 +33,23 @@ const res2 = {
 };
 
 describe("Message Buffer", () => {
-  describe("get.message()", () => {
+  describe("getMessage()", () => {
     it("should return null if no delimiter found", (done) => {
       const buffer = new MessageBuffer("\n");
       buffer.push("foo");
       expect(buffer.getMessage()).to.equal(null);
       done();
     });
   });
+  describe("emptyBuffer()", () => {
+    it("should return the message buffer and set it to empty", (done) => {
+      const buffer = new MessageBuffer();
+      buffer.push("foo\r\nbar");
+      expect(buffer.emptyBuffer()).to.equal("foo\r\nbar");
+      expect(buffer.buffer).to.equal("");
+      done();
+    });
+  });
   describe("server side", () => {
     before((done) => {
       server.listen().then(() => {