refactor: move packet validation to the server/client

socketio · Jun 21, 2023 · 6ddf6b3 · 6ddf6b3
1 parent 887396d
commit 6ddf6b3
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 51 deletions.
diff --git a/lib/index.ts b/lib/index.ts
@@ -131,11 +131,6 @@ export class Encoder {
   }
 }
 
-// see https://stackoverflow.com/questions/8511281/check-if-a-value-is-an-object-in-javascript
-function isObject(value: any): boolean {
-  return Object.prototype.toString.call(value) === "[object Object]";
-}
-
 interface DecoderReservedEvents {
   decoded: (packet: Packet) => void;
 }
@@ -262,12 +257,7 @@ export class Decoder extends Emitter<{}, {}, DecoderReservedEvents> {
 
     // look up json data
     if (str.charAt(++i)) {
-      const payload = this.tryParse(str.substr(i));
-      if (Decoder.isPayloadValid(p.type, payload)) {
-        p.data = payload;
-      } else {
-        throw new Error("invalid payload");
-      }
+      p.data = this.tryParse(str.substr(i));
     }
 
     debug("decoded %s as %j", str, p);
@@ -282,28 +272,6 @@ export class Decoder extends Emitter<{}, {}, DecoderReservedEvents> {
     }
   }
 
-  private static isPayloadValid(type: PacketType, payload: any): boolean {
-    switch (type) {
-      case PacketType.CONNECT:
-        return isObject(payload);
-      case PacketType.DISCONNECT:
-        return payload === undefined;
-      case PacketType.CONNECT_ERROR:
-        return typeof payload === "string" || isObject(payload);
-      case PacketType.EVENT:
-      case PacketType.BINARY_EVENT:
-        return (
-          Array.isArray(payload) &&
-          (typeof payload[0] === "number" ||
-            (typeof payload[0] === "string" &&
-              RESERVED_EVENTS.indexOf(payload[0]) === -1))
-        );
-      case PacketType.ACK:
-      case PacketType.BINARY_ACK:
-        return Array.isArray(payload);
-    }
-  }
-
   /**
    * Deallocates a parser's resources
    */
@@ -359,3 +327,56 @@ class BinaryReconstructor {
     this.buffers = [];
   }
 }
+
+function isNamespaceValid(nsp: unknown) {
+  return typeof nsp === "string";
+}
+
+const isInteger =
+  Number.isInteger ||
+  function (value) {
+    return (
+      typeof value === "number" &&
+      isFinite(value) &&
+      Math.floor(value) === value
+    );
+  };
+
+function isAckIdValid(id: unknown) {
+  return id === undefined || isInteger(id);
+}
+
+// see https://stackoverflow.com/questions/8511281/check-if-a-value-is-an-object-in-javascript
+function isObject(value: any): boolean {
+  return Object.prototype.toString.call(value) === "[object Object]";
+}
+
+function isDataValid(type: PacketType, payload: unknown) {
+  switch (type) {
+    case PacketType.CONNECT:
+      return payload === undefined || isObject(payload);
+    case PacketType.DISCONNECT:
+      return payload === undefined;
+    case PacketType.EVENT:
+      return (
+        Array.isArray(payload) &&
+        (typeof payload[0] === "number" ||
+          (typeof payload[0] === "string" &&
+            RESERVED_EVENTS.indexOf(payload[0]) === -1))
+      );
+    case PacketType.ACK:
+      return Array.isArray(payload);
+    case PacketType.CONNECT_ERROR:
+      return typeof payload === "string" || isObject(payload);
+    default:
+      return false;
+  }
+}
+
+export function isPacketValid(packet: Packet): boolean {
+  return (
+    isNamespaceValid(packet.nsp) &&
+    isAckIdValid(packet.id) &&
+    isDataValid(packet.type, packet.data)
+  );
+}
diff --git a/test/parser.js b/test/parser.js
@@ -1,4 +1,4 @@
-const { PacketType, Decoder, Encoder } = require("..");
+const { PacketType, Decoder, Encoder, isPacketValid } = require("..");
 const expect = require("expect.js");
 const helpers = require("./helpers.js");
 
@@ -108,23 +108,6 @@ describe("socket.io-parser", () => {
   });
 
   it("throw an error upon parsing error", () => {
-    const isInvalidPayload = (str) =>
-      expect(() => new Decoder().add(str)).to.throwException(
-        /^invalid payload$/
-      );
-
-    isInvalidPayload('442["some","data"');
-    isInvalidPayload('0/admin,"invalid"');
-    isInvalidPayload("0[]");
-    isInvalidPayload("1/admin,{}");
-    isInvalidPayload('2/admin,"invalid');
-    isInvalidPayload("2/admin,{}");
-    isInvalidPayload('2[{"toString":"foo"}]');
-    isInvalidPayload('2[true,"foo"]');
-    isInvalidPayload('2[null,"bar"]');
-    isInvalidPayload('2["connect"]');
-    isInvalidPayload('2["disconnect","123"]');
-
     expect(() => new Decoder().add("999")).to.throwException(
       /^unknown packet type 9$/
     );
@@ -148,4 +131,31 @@ describe("socket.io-parser", () => {
       decoder.add('2["hello"]');
     });
   });
+
+  it("should ensure that a packet is valid", async () => {
+    function decode(str) {
+      return new Promise((resolve) => {
+        const decoder = new Decoder();
+
+        decoder.on("decoded", (data) => {
+          resolve(data);
+        });
+
+        decoder.add(str);
+      });
+    }
+
+    expect(isPacketValid(await decode("0"))).to.eql(true);
+    expect(isPacketValid(await decode('442["some","data"'))).to.eql(false);
+    expect(isPacketValid(await decode('0/admin,"invalid"'))).to.eql(false);
+    expect(isPacketValid(await decode("0[]"))).to.eql(false);
+    expect(isPacketValid(await decode("1/admin,{}"))).to.eql(false);
+    expect(isPacketValid(await decode('2/admin,"invalid'))).to.eql(false);
+    expect(isPacketValid(await decode("2/admin,{}"))).to.eql(false);
+    expect(isPacketValid(await decode('2[{"toString":"foo"}]'))).to.eql(false);
+    expect(isPacketValid(await decode('2[true,"foo"]'))).to.eql(false);
+    expect(isPacketValid(await decode('2[null,"bar"]'))).to.eql(false);
+    expect(isPacketValid(await decode('2["connect"]'))).to.eql(false);
+    expect(isPacketValid(await decode('2["disconnect","123"]'))).to.eql(false);
+  });
 });