fix: copy pasting the content from some medium into outline does not get the images (#8472)

* fix: Files from local storage provider sometimes returned with incorrect content type

* fix: attachments.createFromUrl response values incorrect for successful upload

* fix: Reduce liklihood of image download requests being blocked on server

* fix: Content with HTML images should never be considered as markdown

* fix: Image caption sometimes uncentered

* test

app/editor/extensions/PasteHandler.tsx

@@ -25,52 +25,6 @@ import { isDocumentUrl, isUrl } from "@shared/utils/urls";
 import stores from "~/stores";
 import PasteMenu from "../components/PasteMenu";
 
-/**
- * Checks if the HTML string is likely coming from Dropbox Paper.
- *
- * @param html The HTML string to check.
- * @returns True if the HTML string is likely coming from Dropbox Paper.
- */
-function isDropboxPaper(html: string): boolean {
-  return html?.includes("usually-unique-id");
-}
-
-function sliceSingleNode(slice: Slice) {
-  return slice.openStart === 0 &&
-    slice.openEnd === 0 &&
-    slice.content.childCount === 1
-    ? slice.content.firstChild
-    : null;
-}
-
-/**
- * Parses the text contents of an HTML string and returns the src of the first
- * iframe if it exists.
- *
- * @param text The HTML string to parse.
- * @returns The src of the first iframe if it exists, or undefined.
- */
-function parseSingleIframeSrc(html: string) {
-  try {
-    const parser = new DOMParser();
-    const doc = parser.parseFromString(html, "text/html");
-
-    if (
-      doc.body.children.length === 1 &&
-      doc.body.firstElementChild?.tagName === "IFRAME"
-    ) {
-      const iframe = doc.body.firstElementChild;
-      const src = iframe.getAttribute("src");
-      if (src) {
-        return src;
-      }
-    }
-  } catch (e) {
-    // Ignore the million ways parsing could fail.
-  }
-  return undefined;
-}
-
 export default class PasteHandler extends Extension {
   state: {
     open: boolean;
@@ -261,9 +215,12 @@ export default class PasteHandler extends Extension {
             // If the text on the clipboard looks like Markdown OR there is no
             // html on the clipboard then try to parse content as Markdown
             if (
-              (isMarkdown(text) && !isDropboxPaper(html)) ||
+              (isMarkdown(text) &&
+                !isDropboxPaper(html) &&
+                !isContainingImage(html)) ||
               pasteCodeLanguage === "markdown" ||
-              this.shiftKey
+              this.shiftKey ||
+              !html
             ) {
               event.preventDefault();
 
@@ -475,3 +432,59 @@ export default class PasteHandler extends Extension {
     />
   );
 }
+
+/**
+ * Checks if the HTML string is likely coming from Dropbox Paper.
+ *
+ * @param html The HTML string to check.
+ * @returns True if the HTML string is likely coming from Dropbox Paper.
+ */
+function isDropboxPaper(html: string): boolean {
+  return html?.includes("usually-unique-id");
+}
+
+/**
+ * Checks if the HTML string contains an image.
+ *
+ * @param html The HTML string to check.
+ * @returns True if the HTML string contains an image.
+ */
+function isContainingImage(html: string): boolean {
+  return html?.includes("<img");
+}
+
+function sliceSingleNode(slice: Slice) {
+  return slice.openStart === 0 &&
+    slice.openEnd === 0 &&
+    slice.content.childCount === 1
+    ? slice.content.firstChild
+    : null;
+}
+
+/**
+ * Parses the text contents of an HTML string and returns the src of the first
+ * iframe if it exists.
+ *
+ * @param text The HTML string to parse.
+ * @returns The src of the first iframe if it exists, or undefined.
+ */
+function parseSingleIframeSrc(html: string) {
+  try {
+    const parser = new DOMParser();
+    const doc = parser.parseFromString(html, "text/html");
+
+    if (
+      doc.body.children.length === 1 &&
+      doc.body.firstElementChild?.tagName === "IFRAME"
+    ) {
+      const iframe = doc.body.firstElementChild;
+      const src = iframe.getAttribute("src");
+      if (src) {
+        return src;
+      }
+    }
+  } catch (e) {
+    // Ignore the million ways parsing could fail.
+  }
+  return undefined;
+}

plugins/storage/server/api/files.test.ts

@@ -236,7 +236,16 @@ describe("#files.get", () => {
   it("should succeed with status 200 ok when file is requested using signature", async () => {
     const user = await buildUser();
     const fileName = "images.docx";
-    const key = path.join("uploads", user.id, uuidV4(), fileName);
+    const { key } = await buildAttachment(
+      {
+        teamId: user.teamId,
+        userId: user.id,
+        contentType:
+          "application/vnd.openxmlformats-officedocument.wordprocessingml.document",
+        acl: "private",
+      },
+      fileName
+    );
     const signedUrl = await FileStorage.getSignedUrl(key);
 
     ensureDirSync(
@@ -262,6 +271,13 @@ describe("#files.get", () => {
   it("should succeed with status 200 ok when avatar is requested using key", async () => {
     const user = await buildUser();
     const key = path.join("avatars", user.id, uuidV4());
+    await buildAttachment({
+      key,
+      teamId: user.teamId,
+      userId: user.id,
+      contentType: "image/jpg",
+      acl: "public-read",
+    });
 
     ensureDirSync(
       path.dirname(path.join(env.FILE_STORAGE_LOCAL_ROOT_DIR, key))
@@ -274,7 +290,7 @@ describe("#files.get", () => {
 
     const res = await server.get(`/api/files.get?key=${key}`);
     expect(res.status).toEqual(200);
-    expect(res.headers.get("Content-Type")).toEqual("application/octet-stream");
+    expect(res.headers.get("Content-Type")).toEqual("image/jpg");
     expect(res.headers.get("Content-Disposition")).toEqual("attachment");
   });
 });

plugins/storage/server/api/files.ts

@@ -77,19 +77,20 @@ router.get(
     const { isPublicBucket, fileName } = AttachmentHelper.parseKey(key);
     const skipAuthorize = isPublicBucket || isSignedRequest;
     const cacheHeader = "max-age=604800, immutable";
-    let contentType =
+
+    const attachment = await Attachment.findOne({
+      where: { key },
+      rejectOnEmpty: true,
+    });
+    if (!skipAuthorize) {
+      authorize(actor, "read", attachment);
+    }
+
+    const contentType =
+      attachment.contentType ||
       (fileName ? mime.lookup(fileName) : undefined) ||
       "application/octet-stream";
 
-    if (!skipAuthorize) {
-      const attachment = await Attachment.findOne({
-        where: { key },
-        rejectOnEmpty: true,
-      });
-      authorize(actor, "read", attachment);
-      contentType = attachment.contentType;
-    }
-
     ctx.set("Accept-Ranges", "bytes");
     ctx.set("Cache-Control", cacheHeader);
     ctx.set("Content-Type", contentType);

server/routes/api/attachments/attachments.ts

@@ -176,6 +176,8 @@ router.post(
       throw InvalidRequestError(response.error);
     }
 
+    await attachment.reload();
+
     ctx.body = {
       data: presentAttachment(attachment),
     };

server/storage/files/BaseStorage.ts

@@ -1,10 +1,10 @@
 import { Blob } from "buffer";
 import { Readable } from "stream";
 import { PresignedPost } from "@aws-sdk/s3-presigned-post";
-import { isBase64Url } from "@shared/utils/urls";
+import { isBase64Url, isInternalUrl } from "@shared/utils/urls";
 import env from "@server/env";
 import Logger from "@server/logging/Logger";
-import fetch, { RequestInit } from "@server/utils/fetch";
+import fetch, { chromeUserAgent, RequestInit } from "@server/utils/fetch";
 
 export default abstract class BaseStorage {
   /** The default number of seconds until a signed URL expires. */
@@ -149,7 +149,7 @@ export default abstract class BaseStorage {
     const endpoint = this.getUploadUrl(true);
 
     // Early return if url is already uploaded to the storage provider
-    if (url.startsWith("/api") || url.startsWith(endpoint)) {
+    if (url.startsWith(endpoint) || isInternalUrl(url)) {
       return;
     }
 
@@ -168,6 +168,9 @@ export default abstract class BaseStorage {
             options?.maxUploadSize ?? Infinity,
             env.FILE_STORAGE_UPLOAD_MAX_SIZE
           ),
+          headers: {
+            "User-Agent": chromeUserAgent,
+          },
           timeout: 10000,
           ...init,
         });

server/utils/fetch.ts

@@ -7,6 +7,13 @@ import Logger from "@server/logging/Logger";
 
 export type { RequestInit } from "node-fetch";
 
+/**
+ * Fake Chrome user agent string for use in fetch requests to
+ * improve reliability.
+ */
+export const chromeUserAgent =
+  "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/133.0.0.0 Safari/537.36";
+
 /**
  * Wrapper around fetch that uses the request-filtering-agent in cloud hosted
  * environments to filter malicious requests, and the fetch-with-proxy library

shared/editor/components/Caption.tsx

@@ -71,6 +71,7 @@ const Content = styled.p<{ $width: number; $isSelected: boolean }>`
   cursor: text;
   width: ${(props) => props.$width}px;
   min-width: 200px;
+  max-width: 100%;
 
   ${breakpoint("tablet")`
     font-size: 13px;