From f46921275dca713d25e953c2c7610dec5d030d28 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 17 Feb 2025 14:54:13 -0500 Subject: [PATCH] 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 | 109 +++++++++++-------- plugins/storage/server/api/files.test.ts | 20 +++- plugins/storage/server/api/files.ts | 21 ++-- server/routes/api/attachments/attachments.ts | 2 + server/storage/files/BaseStorage.ts | 9 +- server/utils/fetch.ts | 7 ++ shared/editor/components/Caption.tsx | 1 + 7 files changed, 106 insertions(+), 63 deletions(-) diff --git a/app/editor/extensions/PasteHandler.tsx b/app/editor/extensions/PasteHandler.tsx index e6a3323ca2..73c1c63400 100644 --- a/app/editor/extensions/PasteHandler.tsx +++ b/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(" { 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"); }); }); diff --git a/plugins/storage/server/api/files.ts b/plugins/storage/server/api/files.ts index 904f8c236e..98f29252e6 100644 --- a/plugins/storage/server/api/files.ts +++ b/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); diff --git a/server/routes/api/attachments/attachments.ts b/server/routes/api/attachments/attachments.ts index d2288c2151..120052231b 100644 --- a/server/routes/api/attachments/attachments.ts +++ b/server/routes/api/attachments/attachments.ts @@ -176,6 +176,8 @@ router.post( throw InvalidRequestError(response.error); } + await attachment.reload(); + ctx.body = { data: presentAttachment(attachment), }; diff --git a/server/storage/files/BaseStorage.ts b/server/storage/files/BaseStorage.ts index 339262cc56..9c3850f4aa 100644 --- a/server/storage/files/BaseStorage.ts +++ b/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, }); diff --git a/server/utils/fetch.ts b/server/utils/fetch.ts index 2e712b4015..19b4f1b5d7 100644 --- a/server/utils/fetch.ts +++ b/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 diff --git a/shared/editor/components/Caption.tsx b/shared/editor/components/Caption.tsx index 4550f1fcee..3a8c580657 100644 --- a/shared/editor/components/Caption.tsx +++ b/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;