Remove json assumption from postMessage handler (#190)

Incorrect assumption that all messages received are from desktopJS and are serialized JSON. Remove use of JSON completely so no parse is required and we can look at source to determine whether they are within scope of desktopJS message bus.
morganstanley · Sep 18, 2018 · 8ed38bf · 8ed38bf
1 parent 0b0c123
commit 8ed38bf
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 7 deletions.
diff --git a/src/Default/default.ts b/src/Default/default.ts
@@ -133,7 +133,7 @@ export class DefaultMessageBus implements MessageBus {
                 }
 
                 // Make sure topic received matches the one that was subscribed
-                const { source, "topic": receivedTopic, message } = JSON.parse(event.data);
+                const { source, "topic": receivedTopic, message } = event.data;
 
                 if (source === DefaultMessageBus.messageSource && topic === receivedTopic) {
                     listener({ topic: topic }, message);
@@ -173,7 +173,7 @@ export class DefaultMessageBus implements MessageBus {
                     continue;
                 }
 
-                win.postMessage(JSON.stringify({ source: DefaultMessageBus.messageSource, topic: topic, message: message }), this.container.globalWindow.location.origin);
+                win.postMessage({ source: DefaultMessageBus.messageSource, topic: topic, message: message }, this.container.globalWindow.location.origin);
             }
         }
 

diff --git a/tests/unit/Default/default.spec.ts b/tests/unit/Default/default.spec.ts
@@ -430,9 +430,15 @@ describe("DefaultMessageBus", () => {
     });
 
     it("listener callback attached", (done) => {
-        bus.subscribe("topic", callback).then((subscriber) => {
-            subscriber.listener({ origin: "origin", data: JSON.stringify({ source: "desktopJS", topic: "topic", message: "message" }) });
-        }).then(done);
+        const handler = (e, data) => {
+            expect(e.topic).toEqual("topic");
+            expect(data).toEqual("message");
+            done();
+        };
+
+        bus.subscribe("topic", handler).then((subscriber) => {
+            subscriber.listener({ origin: "origin", data: { source: "desktopJS", topic: "topic", message: "message" } });
+        });
     });
 
     it("unsubscribe invokes underlying unsubscribe", (done) => {
@@ -444,8 +450,9 @@ describe("DefaultMessageBus", () => {
     it("publish invokes underling publish", (done) => {
         let message: any = { data: "data" };
         spyOn(mockWindow, "postMessage").and.callThrough();
-        bus.publish("topic", message).then(done);
-        expect(mockWindow.postMessage).toHaveBeenCalledWith(JSON.stringify({ source: "desktopJS", topic: "topic", message: message }), "origin");
+        bus.publish("topic", message).then(() => {
+            expect(mockWindow.postMessage).toHaveBeenCalledWith({ source: "desktopJS", topic: "topic", message: message }, "origin");
+        }).then(done);
     });
 
     it("publish with non matching optional name does not invoke underling send", (done) => {