fix: Copied internal links in shared documents are incorrect (#7368)

* Add internalUrlBase option for toJSON

* Return correct internal urls for shared data

* test

app/hooks/useEditorClickHandlers.ts

@@ -39,18 +39,22 @@ export default function useEditorClickHandlers({ shareId }: Params) {
           return;
         }
 
-        // If we're navigating to a share link from a non-share link then open it in a new tab
-        if (shareId && navigateTo.startsWith("/s/")) {
-          window.open(href, "_blank");
-          return;
-        }
-
         // If we're navigating to an internal document link then prepend the
         // share route to the URL so that the document is loaded in context
-        if (shareId && navigateTo.includes("/doc/")) {
+        if (
+          shareId &&
+          navigateTo.includes("/doc/") &&
+          !navigateTo.includes(shareId)
+        ) {
           navigateTo = sharedDocumentPath(shareId, navigateTo);
         }
 
+        // If we're navigating to a share link from a non-share link then open it in a new tab
+        if (!shareId && navigateTo.startsWith("/s/")) {
+          window.open(href, "_blank");
+          return;
+        }
+
         if (!isModKey(event) && !event.shiftKey) {
           history.push(navigateTo);
         } else {

server/models/helpers/DocumentHelper.test.ts

@@ -12,6 +12,40 @@ describe("DocumentHelper", () => {
     jest.useRealTimers();
   });
 
+  describe("replaceInternalUrls", () => {
+    it("should replace internal urls", async () => {
+      const document = await buildDocument({
+        text: `[link](/doc/internal-123)`,
+      });
+      const result = await DocumentHelper.toJSON(document, {
+        internalUrlBase: "/s/share-123",
+      });
+      expect(result).toEqual({
+        content: [
+          {
+            content: [
+              {
+                marks: [
+                  {
+                    attrs: {
+                      href: "/s/share-123/doc/internal-123",
+                      title: null,
+                    },
+                    type: "link",
+                  },
+                ],
+                text: "link",
+                type: "text",
+              },
+            ],
+            type: "paragraph",
+          },
+        ],
+        type: "doc",
+      });
+    });
+  });
+
   describe("toJSON", () => {
     it("should return content directly if no transformation required", async () => {
       const document = await buildDocument();

server/models/helpers/DocumentHelper.tsx

@@ -45,7 +45,12 @@ export class DocumentHelper {
    * @param document The document or revision to convert
    * @returns The document content as a Prosemirror Node
    */
-  static toProsemirror(document: Document | Revision | Collection) {
+  static toProsemirror(
+    document: Document | Revision | Collection | ProsemirrorData
+  ) {
+    if ("type" in document && document.type === "doc") {
+      return Node.fromJSON(schema, document);
+    }
     if ("content" in document && document.content) {
       return Node.fromJSON(schema, document.content);
     }
@@ -72,11 +77,13 @@ export class DocumentHelper {
     document: Document | Revision | Collection,
     options?: {
       /** The team context */
-      teamId: string;
+      teamId?: string;
       /** Whether to sign attachment urls, and if so for how many seconds is the signature valid */
-      signedUrls: number;
+      signedUrls?: number;
       /** Marks to remove from the document */
       removeMarks?: string[];
+      /** The base path to use for internal links (will replace /doc/) */
+      internalUrlBase?: string;
     }
   ): Promise<ProsemirrorData> {
     let doc: Node | null;
@@ -84,7 +91,11 @@ export class DocumentHelper {
 
     if ("content" in document && document.content) {
       // Optimized path for documents with content available and no transformation required.
-      if (!options?.removeMarks && !options?.signedUrls) {
+      if (
+        !options?.removeMarks &&
+        !options?.signedUrls &&
+        !options?.internalUrlBase
+      ) {
         return document.content;
       }
       doc = Node.fromJSON(schema, document.content);
@@ -98,7 +109,7 @@ export class DocumentHelper {
       doc = parser.parse(document.text);
     }
 
-    if (doc && options?.signedUrls) {
+    if (doc && options?.signedUrls && options?.teamId) {
       json = await ProsemirrorHelper.signAttachmentUrls(
         doc,
         options.teamId,
@@ -108,6 +119,13 @@ export class DocumentHelper {
       json = doc?.toJSON() ?? {};
     }
 
+    if (options?.internalUrlBase) {
+      json = ProsemirrorHelper.replaceInternalUrls(
+        json,
+        options.internalUrlBase
+      );
+    }
+
     if (options?.removeMarks) {
       json = ProsemirrorHelper.removeMarks(json, options.removeMarks);
     }
@@ -126,8 +144,8 @@ export class DocumentHelper {
     const node = DocumentHelper.toProsemirror(document);
     const textSerializers = Object.fromEntries(
       Object.entries(schema.nodes)
-        .filter(([, node]) => node.spec.toPlainText)
-        .map(([name, node]) => [name, node.spec.toPlainText])
+        .filter(([, n]) => n.spec.toPlainText)
+        .map(([name, n]) => [name, n.spec.toPlainText])
     );
 
     return textBetween(node, 0, node.content.size, textSerializers);
@@ -139,10 +157,12 @@ export class DocumentHelper {
    * @param document The document or revision to convert
    * @returns The document title and content as a Markdown string
    */
-  static toMarkdown(document: Document | Revision | Collection) {
+  static toMarkdown(
+    document: Document | Revision | Collection | ProsemirrorData
+  ) {
     const text = serializer
       .serialize(DocumentHelper.toProsemirror(document))
-      .replace(/\n\\(\n|$)/g, "\n\n")
+      .replace(/(^|\n)\\(\n|$)/g, "\n\n")
       .replace(/“/g, '"')
       .replace(/”/g, '"')
       .replace(/‘/g, "'")
@@ -153,13 +173,17 @@ export class DocumentHelper {
       return text;
     }
 
-    const iconType = determineIconType(document.icon);
+    if (document instanceof Document || document instanceof Revision) {
+      const iconType = determineIconType(document.icon);
 
-    const title = `${iconType === IconType.Emoji ? document.icon + " " : ""}${
-      document.title
-    }`;
+      const title = `${iconType === IconType.Emoji ? document.icon + " " : ""}${
+        document.title
+      }`;
 
-    return `# ${title}\n\n${text}`;
+      return `# ${title}\n\n${text}`;
+    }
+
+    return text;
   }
 
   /**

server/models/helpers/ProsemirrorHelper.tsx

@@ -15,6 +15,7 @@ import light from "@shared/styles/theme";
 import { ProsemirrorData } from "@shared/types";
 import { attachmentRedirectRegex } from "@shared/utils/ProsemirrorHelper";
 import { isRTL } from "@shared/utils/rtl";
+import { isInternalUrl } from "@shared/utils/urls";
 import { schema, parser } from "@server/editor";
 import Logger from "@server/logging/Logger";
 import { trace } from "@server/logging/tracing";
@@ -161,7 +162,9 @@ export class ProsemirrorHelper {
    * @param marks The mark types to remove
    * @returns The content with marks removed
    */
-  static removeMarks(data: ProsemirrorData, marks: string[]) {
+  static removeMarks(doc: Node | ProsemirrorData, marks: string[]) {
+    const json = "toJSON" in doc ? (doc.toJSON() as ProsemirrorData) : doc;
+
     function removeMarksInner(node: ProsemirrorData) {
       if (node.marks) {
         node.marks = node.marks.filter((mark) => !marks.includes(mark.type));
@@ -171,7 +174,7 @@ export class ProsemirrorHelper {
       }
       return node;
     }
-    return removeMarksInner(data);
+    return removeMarksInner(json);
   }
 
   /**
@@ -197,6 +200,44 @@ export class ProsemirrorHelper {
     return replace(data);
   }
 
+  static async replaceInternalUrls(
+    doc: Node | ProsemirrorData,
+    basePath: string
+  ) {
+    const json = "toJSON" in doc ? (doc.toJSON() as ProsemirrorData) : doc;
+
+    if (basePath.endsWith("/")) {
+      throw new Error("internalUrlBase must not end with a slash");
+    }
+
+    function replaceUrl(url: string) {
+      return url.replace(`/doc/`, `${basePath}/doc/`);
+    }
+
+    function replaceInternalUrlsInner(node: ProsemirrorData) {
+      if (typeof node.attrs?.href === "string") {
+        node.attrs.href = replaceUrl(node.attrs.href);
+      } else if (node.marks) {
+        node.marks.forEach((mark) => {
+          if (
+            typeof mark.attrs?.href === "string" &&
+            isInternalUrl(mark.attrs?.href)
+          ) {
+            mark.attrs.href = replaceUrl(mark.attrs.href);
+          }
+        });
+      }
+
+      if (node.content) {
+        node.content.forEach(replaceInternalUrlsInner);
+      }
+
+      return node;
+    }
+
+    return replaceInternalUrlsInner(json);
+  }
+
   /**
    * Returns the document as a plain JSON object with attachment URLs signed.
    *

server/presenters/document.ts

@@ -1,13 +1,14 @@
 import { traceFunction } from "@server/logging/tracing";
 import { Document } from "@server/models";
 import { DocumentHelper } from "@server/models/helpers/DocumentHelper";
-import { TextHelper } from "@server/models/helpers/TextHelper";
 import { APIContext } from "@server/types";
 import presentUser from "./user";
 
 type Options = {
   /** Whether to render the document's public fields. */
   isPublic?: boolean;
+  /** The root share ID when presenting a shared document. */
+  shareId?: string;
   /** Always include the text of the document in the payload. */
   includeText?: boolean;
   /** Always include the data of the document in the payload. */
@@ -25,28 +26,27 @@ async function presentDocument(
   };
 
   const asData = !ctx || Number(ctx?.headers["x-api-version"] ?? 0) >= 3;
-  const text = options.isPublic
-    ? await TextHelper.attachmentsToSignedUrls(document.text, document.teamId)
-    : document.text;
 
-  const data: Record<string, any> = {
+  const data = await DocumentHelper.toJSON(
+    document,
+    options.isPublic
+      ? {
+          signedUrls: 60,
+          teamId: document.teamId,
+          removeMarks: ["comment"],
+          internalUrlBase: `/s/${options.shareId}`,
+        }
+      : undefined
+  );
+
+  const text = DocumentHelper.toMarkdown(data);
+
+  const res: Record<string, any> = {
     id: document.id,
-    url: document.url,
+    url: document.path,
     urlId: document.urlId,
     title: document.title,
-    data:
-      asData || options.includeData
-        ? await DocumentHelper.toJSON(
-            document,
-            options.isPublic
-              ? {
-                  signedUrls: 60,
-                  teamId: document.teamId,
-                  removeMarks: ["comment"],
-                }
-              : undefined
-          )
-        : undefined,
+    data: asData || options?.includeData ? data : undefined,
     text: !asData || options?.includeText ? text : undefined,
     icon: document.icon,
     color: document.color,
@@ -69,22 +69,22 @@ async function presentDocument(
   };
 
   if (!!document.views && document.views.length > 0) {
-    data.lastViewedAt = document.views[0].updatedAt;
+    res.lastViewedAt = document.views[0].updatedAt;
   }
 
   if (!options.isPublic) {
     const source = await document.$get("import");
 
-    data.isCollectionDeleted = await document.isCollectionDeleted();
-    data.collectionId = document.collectionId;
-    data.parentDocumentId = document.parentDocumentId;
-    data.createdBy = presentUser(document.createdBy);
-    data.updatedBy = presentUser(document.updatedBy);
-    data.collaboratorIds = document.collaboratorIds;
-    data.templateId = document.templateId;
-    data.template = document.template;
-    data.insightsEnabled = document.insightsEnabled;
-    data.sourceMetadata = document.sourceMetadata
+    res.isCollectionDeleted = await document.isCollectionDeleted();
+    res.collectionId = document.collectionId;
+    res.parentDocumentId = document.parentDocumentId;
+    res.createdBy = presentUser(document.createdBy);
+    res.updatedBy = presentUser(document.updatedBy);
+    res.collaboratorIds = document.collaboratorIds;
+    res.templateId = document.templateId;
+    res.template = document.template;
+    res.insightsEnabled = document.insightsEnabled;
+    res.sourceMetadata = document.sourceMetadata
       ? {
           importedAt: source?.createdAt ?? document.createdAt,
           importType: source?.format,
@@ -94,7 +94,7 @@ async function presentDocument(
       : undefined;
   }
 
-  return data;
+  return res;
 }
 
 export default traceFunction({

server/routes/api/documents/documents.ts

@@ -441,6 +441,7 @@ router.post(
     const isPublic = cannot(user, "read", document);
     const serializedDocument = await presentDocument(ctx, document, {
       isPublic,
+      shareId,
     });
 
     const team = await document.$get("team");
@@ -891,6 +892,7 @@ router.post(
       results.map(async (result) => {
         const document = await presentDocument(ctx, result.document, {
           isPublic,
+          shareId,
         });
         return { ...result, document };
       })