From fcb535efee545401659b10d55058abfee9ead14f Mon Sep 17 00:00:00 2001
From: mike12345567 <me@michaeldrury.co.uk>
Date: Wed, 15 May 2024 14:03:31 +0100
Subject: [PATCH] Adding test cases for content-disposition hacks.

---
 packages/server/src/integrations/rest.ts      | 19 ++++------
 .../src/integrations/tests/restUtils.spec.ts  | 38 +++++++++++++++++++
 .../src/integrations/utils/restUtils.ts       | 26 +++++++++++++
 3 files changed, 72 insertions(+), 11 deletions(-)
 create mode 100644 packages/server/src/integrations/tests/restUtils.spec.ts
 create mode 100644 packages/server/src/integrations/utils/restUtils.ts

diff --git a/packages/server/src/integrations/rest.ts b/packages/server/src/integrations/rest.ts
index deff7c81d0..6ed8e4e4ec 100644
--- a/packages/server/src/integrations/rest.ts
+++ b/packages/server/src/integrations/rest.ts
@@ -16,6 +16,7 @@ import get from "lodash/get"
 import * as https from "https"
 import qs from "querystring"
 import fetch from "node-fetch"
+import type { Response } from "node-fetch"
 import { formatBytes } from "../utilities"
 import { performance } from "perf_hooks"
 import FormData from "form-data"
@@ -25,6 +26,7 @@ import { handleFileResponse, handleXml } from "./utils"
 import { parse } from "content-disposition"
 import path from "path"
 import { Builder as XmlBuilder } from "xml2js"
+import { getAttachmentHeaders } from "./utils/restUtils"
 
 enum BodyType {
   NONE = "none",
@@ -130,25 +132,20 @@ class RestIntegration implements IntegrationBase {
     this.config = config
   }
 
-  async parseResponse(response: any, pagination: PaginationConfig | null) {
+  async parseResponse(response: Response, pagination: PaginationConfig | null) {
     let data: any[] | string | undefined,
       raw: string | undefined,
-      headers: Record<string, string> = {},
+      headers: Record<string, string[] | string> = {},
       filename: string | undefined
 
-    const contentType = response.headers.get("content-type") || ""
-    let contentDisposition = response.headers.get("content-disposition") || ""
+    const { contentType, contentDisposition } = getAttachmentHeaders(
+      response.headers
+    )
     if (
       contentDisposition.includes("filename") ||
       contentDisposition.includes("attachment") ||
       contentDisposition.includes("form-data")
     ) {
-      // the API does not follow the requirements of https://www.ietf.org/rfc/rfc2183.txt
-      // all content-disposition headers should be format disposition-type; parameters
-      // but some APIs do not provide a type, causing the parse below to fail - add one to fix this
-      if (!contentDisposition.includes("; ")) {
-        contentDisposition = `attachment; ${contentDisposition}`
-      }
       filename =
         path.basename(parse(contentDisposition).parameters?.filename) || ""
     }
@@ -178,7 +175,7 @@ class RestIntegration implements IntegrationBase {
       throw `Failed to parse response body: ${err}`
     }
 
-    let contentLength: string = response.headers.get("content-length")
+    let contentLength = response.headers.get("content-length")
     if (!contentLength && raw) {
       contentLength = Buffer.byteLength(raw, "utf8").toString()
     }
diff --git a/packages/server/src/integrations/tests/restUtils.spec.ts b/packages/server/src/integrations/tests/restUtils.spec.ts
new file mode 100644
index 0000000000..cdcaaec489
--- /dev/null
+++ b/packages/server/src/integrations/tests/restUtils.spec.ts
@@ -0,0 +1,38 @@
+import { getAttachmentHeaders } from "../utils/restUtils"
+import type { Headers } from "node-fetch"
+
+function headers(dispositionValue: string) {
+  return {
+    get: (name: string) => {
+      if (name.toLowerCase() === "content-disposition") {
+        return dispositionValue
+      } else {
+        return "application/pdf"
+      }
+    },
+    set: () => {},
+  } as unknown as Headers
+}
+
+describe("getAttachmentHeaders", () => {
+  it("should be able to correctly handle a broken content-disposition", () => {
+    const { contentDisposition } = getAttachmentHeaders(
+      headers(`filename="report.pdf"`)
+    )
+    expect(contentDisposition).toBe(`attachment; filename="report.pdf"`)
+  })
+
+  it("should be able to correctly with a filename that could cause problems", () => {
+    const { contentDisposition } = getAttachmentHeaders(
+      headers(`filename="report;.pdf"`)
+    )
+    expect(contentDisposition).toBe(`attachment; filename="report;.pdf"`)
+  })
+
+  it("should not touch a valid content-disposition", () => {
+    const { contentDisposition } = getAttachmentHeaders(
+      headers(`inline; filename="report.pdf"`)
+    )
+    expect(contentDisposition).toBe(`inline; filename="report.pdf"`)
+  })
+})
diff --git a/packages/server/src/integrations/utils/restUtils.ts b/packages/server/src/integrations/utils/restUtils.ts
new file mode 100644
index 0000000000..42c8e939eb
--- /dev/null
+++ b/packages/server/src/integrations/utils/restUtils.ts
@@ -0,0 +1,26 @@
+import type { Headers } from "node-fetch"
+
+export function getAttachmentHeaders(headers: Headers) {
+  const contentType = headers.get("content-type") || ""
+  let contentDisposition = headers.get("content-disposition") || ""
+
+  // the API does not follow the requirements of https://www.ietf.org/rfc/rfc2183.txt
+  // all content-disposition headers should be format disposition-type; parameters
+  // but some APIs do not provide a type, causing the parse below to fail - add one to fix this
+  const quotesRegex = /"(?:[^"\\]|\\.)*"|;/g
+  let match: RegExpMatchArray | null = null,
+    found = false
+  while ((match = quotesRegex.exec(contentDisposition)) !== null) {
+    if (match[0] === ";") {
+      found = true
+    }
+  }
+  if (!found) {
+    return {
+      contentDisposition: `attachment; ${contentDisposition}`,
+      contentType,
+    }
+  }
+
+  return { contentDisposition, contentType }
+}