Add XML parser for generic webhook payloads (#410)

* Add support for decoding XML for webhooks

* Ensure all endpoints use the error middleware

* Dependencies

* changelog

* Describe form data in the documentation

* Reorder

changelog.d/410.feature

@@ -0,0 +1 @@
+Added support for decoding XML payloads when handling generic webhooks.

docs/setup/webhooks.md

@@ -64,7 +64,7 @@ Hookshot handles `POST` and `PUT` HTTP requests by default.
 
 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.
+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 supported payload.
 
 If the body contains a `text` key, then that key will be used as a message body in Matrix (aka `body`). This text will be automatically converted from Markdown to HTML (unless
 a `html` key is provided.).
@@ -73,7 +73,19 @@ If the body contains a `html` key, then that key will be used as the HTML messag
 
 If the body *also* contains a `username` key, then the message will be prepended by the given username. This will be prepended to both `text` and `html`.
 
-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**.
+If the body does NOT contain a `text` field, the full payload will be sent to the room. This can be adapted into a message by creating a **JavaScript transformation function**.
+
+### Payload formats
+
+If the request is a `POST`/`PUT`, the body of the request will be decoded and stored inside the event. Currently, Hookshot supports:
+
+- XML, when the `Content-Type` header ends in `/xml` or `+xml`.
+- Web form data, when the `Content-Type` header is `application/x-www-form-urlencoded`.
+- JSON, when the `Content-Type` header is `application/json`.
+- Text, when the `Content-Type` header begins with `text/`.
+
+Decoding is done in the order given above. E.g. `text/xml` would be parsed as XML. Any formats not described above are not
+decoded.
 
 ### GET requests
 
@@ -113,7 +125,7 @@ The script string should be set within the state event under the `transformation
 Transformation scripts have a versioned API. You can check the version of the API that the hookshot instance supports
 at runtime by checking the `HookshotApiVersion` variable. If the variable is undefined, it should be considered `v1`.
 
-The execution environment will contain a `data` variable, which will be the body of the incoming request (JSON will be parsed into an `Object`).
+The execution environment will contain a `data` variable, which will be the body of the incoming request (see [Payload formats](#payload-formats)).
 Scripts are executed synchronously and expect the `result` variable to be set.
 
 If the script contains errors or is otherwise unable to work, the bridge will send an error to the room. You can check the logs of the bridge

package.json

@@ -62,6 +62,7 @@
     "uuid": "^8.3.2",
     "vm2": "^3.9.6",
     "winston": "^3.3.3",
+    "xml2js": "^0.4.23",
     "yaml": "^1.10.2"
   },
   "devDependencies": {
@@ -82,6 +83,7 @@
     "@types/node": "^14",
     "@types/node-emoji": "^1.8.1",
     "@types/uuid": "^8.3.3",
+    "@types/xml2js": "^0.4.11",
     "@typescript-eslint/eslint-plugin": "^5.4.0",
     "@typescript-eslint/parser": "^5.4.0",
     "@uiw/react-codemirror": "^4.5.3",

src/ListenerService.ts

@@ -1,6 +1,7 @@
 import { Server } from "http";
-import { Application, default as expressApp, Router } from "express";
+import { Application, default as expressApp, NextFunction, Request, Response, Router } from "express";
 import LogWrapper from "./LogWrapper";
+import { errorMiddleware } from "./api";
 
 // Appserices can't be handled yet because the bot-sdk maintains control of it.
 // See https://github.com/turt2live/matrix-bot-sdk/issues/191
@@ -71,6 +72,8 @@ export class ListenerService {
             }
             const addr = listener.config.bindAddress || "127.0.0.1";
             listener.server = listener.app.listen(listener.config.port, addr);
+            // Always include the error handler
+            listener.app.use((err: unknown, req: Request, res: Response, next: NextFunction) => errorMiddleware(log)(err, req, res, next));
             log.info(`Listening on http://${addr}:${listener.config.port} for ${listener.config.resources.join(', ')}`)
         }
     }

src/generic/Router.ts

@@ -3,6 +3,7 @@ import express, { NextFunction, Request, Response, Router } from "express";
 import LogWrapper from "../LogWrapper";
 import { ApiError, ErrCode } from "../api";
 import { GenericWebhookEvent, GenericWebhookEventResult } from "./types";
+import * as xml from "xml2js";
 
 const WEBHOOK_RESPONSE_TIMEOUT = 5000;
 
@@ -11,7 +12,6 @@ export class GenericWebhooksRouter {
     constructor(private readonly queue: MessageQueue, private readonly deprecatedPath = false, private readonly allowGet: boolean) { }
 
     private onWebhook(req: Request<{hookId: string}, unknown, unknown, unknown>, res: Response<{ok: true}|{ok: false, error: string}>, next: NextFunction) {
-
         if (req.method === "GET" && !this.allowGet) {
             throw new ApiError("Invalid Method. Expecting PUT or POST", ErrCode.MethodNotAllowed);
         }
@@ -55,13 +55,34 @@ export class GenericWebhooksRouter {
         });
     }
 
+    private static xmlHandler(req: Request, res: Response, next: NextFunction) {
+        express.text({ type: ["*/xml", "+xml"] })(req, res, (err) => {
+            if (err) {
+                next(err);
+                return;
+            }
+            if (typeof req.body !== 'string') {
+                next();
+                return;
+            }
+            xml.parseStringPromise(req.body).then(xmlResult => {
+                req.body = xmlResult;
+                next();
+            }).catch(e => {
+                res.statusCode = 400;
+                next(e);
+            });
+        });
+    }
+
     public getRouter() {
         const router = Router();
         router.all(
             '/:hookId',
-            express.text({ type: 'text/*'}),
+            GenericWebhooksRouter.xmlHandler,
             express.urlencoded({ extended: false }),
             express.json(),
+            express.text({ type: 'text/*'}),
             this.onWebhook.bind(this),
         );
         return router;

src/provisioning/provisioner.ts

@@ -3,8 +3,8 @@ import { Router, default as express, NextFunction, Request, Response } from "exp
 import { ConnectionManager } from "../ConnectionManager";
 import LogWrapper from "../LogWrapper";
 import { assertUserPermissionsInRoom, GetConnectionsResponseItem, GetConnectionTypeResponseItem } from "./api";
-import { ApiError, ErrCode, errorMiddleware } from "../api";
-import { Intent, MembershipEventContent, PowerLevelsEvent, PowerLevelsEventContent } from "matrix-bot-sdk";
+import { ApiError, ErrCode } from "../api";
+import { Intent } from "matrix-bot-sdk";
 import Metrics from "../Metrics";
 
 const log = new LogWrapper("Provisioner");
@@ -70,7 +70,6 @@ export class Provisioner {
             (...args) => this.checkUserPermission("write", ...args),
             this.deleteConnection.bind(this),
         );
-        this.expressRouter.use((err: unknown, req: Request, res: Response, next: NextFunction) => errorMiddleware(log)(err, req, res, next));
     }
 
     private checkAuth(req: Request, _res: Response, next: NextFunction) {

yarn.lock

@@ -1299,6 +1299,13 @@
   resolved "https://registry.yarnpkg.com/@types/uuid/-/uuid-8.3.3.tgz#c6a60686d953dbd1b1d45e66f4ecdbd5d471b4d0"
   integrity sha512-0LbEEx1zxrYB3pgpd1M5lEhLcXjKJnYghvhTRgaBeUivLHMDM1TzF3IJ6hXU2+8uA4Xz+5BA63mtZo5DjVT8iA==
 
+"@types/xml2js@^0.4.11":
+  version "0.4.11"
+  resolved "https://registry.yarnpkg.com/@types/xml2js/-/xml2js-0.4.11.tgz#bf46a84ecc12c41159a7bd9cf51ae84129af0e79"
+  integrity sha512-JdigeAKmCyoJUiQljjr7tQG3if9NkqGUgwEUqBvV0N7LM4HyQk7UXCnusRa1lnvXAEYJ8mw8GtZWioagNztOwA==
+  dependencies:
+    "@types/node" "*"
+
 "@typescript-eslint/eslint-plugin@^5.4.0":
   version "5.8.0"
   resolved "https://registry.yarnpkg.com/@typescript-eslint/eslint-plugin/-/eslint-plugin-5.8.0.tgz#52cd9305ceef98a5333f9492d519e6c6c7fe7d43"
@@ -5528,7 +5535,7 @@ wrappy@1:
   resolved "https://registry.yarnpkg.com/wrappy/-/wrappy-1.0.2.tgz#b5243d8f3ec1aa35f1364605bc0d1036e30ab69f"
   integrity sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=
 
-xml2js@^0.4.19:
+xml2js@^0.4.19, xml2js@^0.4.23:
   version "0.4.23"
   resolved "https://registry.yarnpkg.com/xml2js/-/xml2js-0.4.23.tgz#a0c69516752421eb2ac758ee4d4ccf58843eac66"
   integrity sha512-ySPiMjM0+pLDftHgXY4By0uswI3SPKLDw/i3UXbnO8M/p28zqexCUoPmQFrYD+/1BzhGJSs2i1ERWKJAtiLrug==