Fix floating values breaking generic webhooks (#396)

* Convert floats to strings for generic webhook payloads

* document the change

* Create 396.bugfix

changelog.d/396.bugfix

@@ -0,0 +1 @@
+Floats in JSON payloads sent to generic webhooks are now handled properly. See the [documentation](https://matrix-org.github.io/matrix-hookshot/1.8.0/setup/webhooks.html#webhook-handling) for more information.

docs/setup/webhooks.md

@@ -56,6 +56,7 @@ To add a webhook to your room:
 
 ## Webhook Handling
 
+
 Hookshot handles HTTP requests with a method of `GET`, `POST` or `PUT`.
 
 If the request is a `GET` request, the query parameters are assumed to be the body. Otherwise, the body of the request should be a JSON payload.
@@ -69,6 +70,16 @@ If the body *also* contains a `username` key, then the message will be prepended
 
 If the body does NOT contain a `text` field, the full JSON payload will be sent to the room. This can be adapted into a message by creating a **JavaScript transformation function**.
 
+Hookshot will insert the full content of the body into a key under the Matrix event called `uk.half-shot.hookshot.webhook_data`, which may be useful if you have
+other integrations that would like to make use of the raw request body.
+
+<section class="notice">
+Matrix does NOT support floating point values in JSON, so the <code>uk.half-shot.hookshot.webhook_data</code> field will automatically convert any float values
+to a string representation of that value. This change is <strong>not applied</strong> to the JavaScript transformation <code>data</code>
+variable, so it will contain proper float values.
+</section>
+
+
 ## JavaScript Transformations
 
 <section class="notice">

src/Connections/GenericHook.ts

@@ -56,6 +56,8 @@ const log = new LogWrapper("GenericHookConnection");
 const md = new markdownit();
 
 const TRANSFORMATION_TIMEOUT_MS = 500;
+const SANITIZE_MAX_DEPTH = 5;
+const SANITIZE_MAX_BREADTH = 25;
 
 /**
  * Handles rooms connected to a github repo.
@@ -63,6 +65,39 @@ const TRANSFORMATION_TIMEOUT_MS = 500;
 @Connection
 export class GenericHookConnection extends BaseConnection implements IConnection {
 
+    /**
+     * Ensures a JSON payload is compatible with Matrix JSON requirements, such
+     * as disallowing floating point values.
+     * 
+     * If the `depth` exceeds `SANITIZE_MAX_DEPTH`, the value of `data` will be immediately returned.
+     * If the object contains more than `SANITIZE_MAX_BREADTH` entries, the remaining entries will not be checked.
+     * 
+     * @param data The data to santise
+     * @param depth The depth of the current object relative to the root.
+     * @returns 
+     */
+    static sanitiseObjectForMatrixJSON(data: unknown, depth = 0): unknown {
+        if (typeof data === "number" && !Number.isInteger(data)) {
+            return data.toString();
+        }
+        if (depth > SANITIZE_MAX_DEPTH || typeof data !== "object" || data === null) {
+            return data;
+        }
+        if (Array.isArray(data)) {
+            return data.map((d, i) => i > SANITIZE_MAX_BREADTH ? d : this.sanitiseObjectForMatrixJSON(d, depth + 1));
+        }
+        let breadth = 0;
+        const obj: Record<string, unknown> = { ...data };
+        for (const [key, value] of Object.entries(data)) {
+            breadth++;
+            if (breadth > SANITIZE_MAX_BREADTH) {
+                break;
+            }
+            obj[key] = this.sanitiseObjectForMatrixJSON(value, depth + 1);
+        }
+        return obj;
+    }
+
     static validateState(state: Record<string, unknown>, allowJsTransformationFunctions?: boolean): GenericHookConnectionState {
         const {name, transformationFunction} = state;
         let transformationFunctionResult: string|undefined;
@@ -336,12 +371,15 @@ export class GenericHookConnection extends BaseConnection implements IConnection
         const sender = this.getUserId();
         await this.ensureDisplayname();
 
+        // Matrix cannot handle float data, so make sure we parse out any floats.
+        const safeData = GenericHookConnection.sanitiseObjectForMatrixJSON(data);
+        
         await this.messageClient.sendMatrixMessage(this.roomId, {
             msgtype: content.msgtype || "m.notice",
             body: content.plain,
             formatted_body: content.html || md.renderInline(content.plain),
             format: "org.matrix.custom.html",
-            "uk.half-shot.hookshot.webhook_data": data,
+            "uk.half-shot.hookshot.webhook_data": safeData,
         }, 'm.room.message', sender);
         return success;
 

src/MatrixSender.ts

@@ -1,6 +1,5 @@
 import { BridgeConfig } from "./Config/Config";
 import { MessageQueue, createMessageQueue } from "./MessageQueue";
-import { MatrixEventContent, MatrixMessageContent } from "./MatrixEvent";
 import { Appservice, IAppserviceRegistration, MemoryStorageProvider } from "matrix-bot-sdk";
 import LogWrapper from "./LogWrapper";
 import { v4 as uuid } from "uuid";
@@ -11,7 +10,7 @@ export interface IMatrixSendMessage {
     sender: string|null;
     type: string;
     roomId: string;
-    content: MatrixEventContent;
+    content: Record<string, unknown>;
 }
 
 export interface IMatrixSendMessageResponse {
@@ -86,11 +85,11 @@ export class MessageSenderClient {
         return this.sendMatrixMessage(roomId, {
             msgtype,
             body: text,
-        } as MatrixMessageContent, "m.room.message", sender);
+        }, "m.room.message", sender);
     }
 
     public async sendMatrixMessage(roomId: string,
-                                   content: MatrixEventContent, eventType = "m.room.message",
+                                   content: unknown, eventType = "m.room.message",
                                    sender: string|null = null): Promise<string> {
         const result = await this.queue.pushWait<IMatrixSendMessage, IMatrixSendMessageResponse|IMatrixSendMessageFailedResponse>({
             eventName: "matrix.message",
@@ -99,7 +98,7 @@ export class MessageSenderClient {
                 roomId,
                 type: eventType,
                 sender,
-                content,
+                content: content as Record<string, undefined>,
             },
         });
 

tests/connections/GenericHookTest.ts

@@ -2,8 +2,8 @@
 import { expect } from "chai";
 import { BridgeConfigGenericWebhooks, BridgeGenericWebhooksConfigYAML } from "../../src/Config/Config";
 import { GenericHookConnection, GenericHookConnectionState } from "../../src/Connections/GenericHook";
-import { MessageSenderClient } from "../../src/MatrixSender";
-import { createMessageQueue, MessageQueue } from "../../src/MessageQueue";
+import { MessageSenderClient, IMatrixSendMessage } from "../../src/MatrixSender";
+import { LocalMQ } from "../../src/MessageQueue/LocalMQ";
 import { AppserviceMock } from "../utils/AppserviceMock";
 
 const ROOM_ID = "!foo:bar";
@@ -13,25 +13,23 @@ const V2TFFunction = "result = {plain: `The answer to '${data.question}' is ${da
 
 function createGenericHook(state: GenericHookConnectionState = {
     name: "some-name"
-}, config: BridgeGenericWebhooksConfigYAML = { enabled: true, urlPrefix: "https://example.com/webhookurl"}): [GenericHookConnection, MessageQueue] {
-    const mq = createMessageQueue({
-            monolithic: true
-    });
+}, config: BridgeGenericWebhooksConfigYAML = { enabled: true, urlPrefix: "https://example.com/webhookurl"}): [GenericHookConnection, LocalMQ] {
+    const mq = new LocalMQ();
     mq.subscribe('*');
     const messageClient = new MessageSenderClient(mq);
     const connection =  new GenericHookConnection(ROOM_ID, state, "foobar", "foobar", messageClient, new BridgeConfigGenericWebhooks(config), AppserviceMock.create())
     return [connection, mq];
 }
 
-function handleMessage(mq: MessageQueue) {
-    return new Promise(r => mq.on('matrix.message', (msg) => {
+function handleMessage(mq: LocalMQ): Promise<IMatrixSendMessage> {
+    return new Promise(r => mq.once('matrix.message', (msg) => {
         mq.push({
             eventName: 'response.matrix.message',
             messageId: msg.messageId,
             sender: 'TestSender',
             data: { 'eventId': '$foo:bar' },
         });
-        r(msg.data);
+        r(msg.data as IMatrixSendMessage);
     })); 
 }
 
@@ -177,4 +175,42 @@ describe("GenericHookConnection", () => {
             type: 'm.room.message',
         });
     });
+    it("will handle a message containing floats", async () => {
+        const [connection, mq] = createGenericHook();
+        let messagePromise = handleMessage(mq);
+        await connection.onGenericHook({ simple: 1.2345 });
+        let message = await messagePromise;
+        expect(message.roomId).to.equal(ROOM_ID);
+        expect(message.sender).to.equal(connection.getUserId());
+        expect(message.content["uk.half-shot.hookshot.webhook_data"]).to.deep.equal({ simple: "1.2345" });
+
+        messagePromise = handleMessage(mq);
+        await connection.onGenericHook({
+            a: {
+                deep: {
+                    object: {
+                        containing: 1.2345
+                    }
+                }
+            }
+        });
+        message = await messagePromise;
+        expect(message.roomId).to.equal(ROOM_ID);
+        expect(message.sender).to.equal(connection.getUserId());
+        expect(message.content["uk.half-shot.hookshot.webhook_data"]).to.deep.equal({ a: { deep: { object: { containing: "1.2345" }}} });
+
+        messagePromise = handleMessage(mq);
+        await connection.onGenericHook({
+            an_array_of: [1.2345, 6.789],
+            floats: true,
+        });
+        message = await messagePromise;
+        expect(message.roomId).to.equal(ROOM_ID);
+        expect(message.sender).to.equal(connection.getUserId());
+        expect(message.content["uk.half-shot.hookshot.webhook_data"]).to.deep.equal({
+            an_array_of: ["1.2345", "6.789"],
+            floats: true,
+        });
+
+    });
 })