Merge pull request #13694 from Budibase/fix/invalid-content-dispositions

Handle invalid content-dispositions, still allow REST download

packages/server/__mocks__/node-fetch.ts

@@ -17,8 +17,10 @@ module FetchMock {
           raw: () => {
             return { "content-type": ["application/json"] }
           },
-          get: () => {
+          get: (name: string) => {
-            return ["application/json"]
+            if (name.toLowerCase() === "content-type") {
+              return ["application/json"]
+            }
           },
         },
         json: async () => {

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,14 +132,15 @@ 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") || ""
+    const { contentType, contentDisposition } = getAttachmentHeaders(
-    const contentDisposition = response.headers.get("content-disposition") || ""
+      response.headers
+    )
     if (
       contentDisposition.includes("filename") ||
       contentDisposition.includes("attachment") ||
@@ -172,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()
     }

packages/server/src/integrations/tests/rest.spec.ts

@@ -4,7 +4,11 @@ jest.mock("node-fetch", () => {
       raw: () => {
         return { "content-type": ["application/json"] }
       },
-      get: () => ["application/json"],
+      get: (name: string) => {
+        if (name.toLowerCase() === "content-type") {
+          return ["application/json"]
+        }
+      },
     },
     json: jest.fn(() => ({
       my_next_cursor: 123,
@@ -211,7 +215,16 @@ describe("REST Integration", () => {
         json: json ? async () => json : undefined,
         text: text ? async () => text : undefined,
         headers: {
-          get: (key: any) => (key === "content-length" ? 100 : header),
+          get: (key: string) => {
+            switch (key.toLowerCase()) {
+              case "content-length":
+                return 100
+              case "content-type":
+                return header
+              default:
+                return ""
+            }
+          },
           raw: () => ({ "content-type": header }),
         },
       }

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"`)
+  })
+})

packages/server/src/integrations/utils/restUtils.ts

@@ -0,0 +1,28 @@
+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
+  if (contentDisposition) {
+    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 }
+}