Repository: nhn/tui.editor
Pull request #2267 (2022-02-04 17:54:30)

fix: wrong link, image escape when converting
<!-- EDIT TITLE PLEASE -->
<!-- It should be one of them
  <ISSUE TYPE>: Short Description (<CLOSING TYPE> #<ISSUE NUMBERS>)
  ex)
  feat: add new feature (close #111)
  fix: wrong behavior (fix #111)
  chore: change build tool (ref #111)
-->

<!-- SPECIFY A ISSUE TYPE AT HEAD
  feat: A new feature
  fix: A bug fix
  docs: Documentation only changes
  style: Changes that do not affect the meaning of the code (white-space, formatting etc)
  refactor: A code change that neither fixes a bug or adds a feature
  perf: A code change that improves performance
  test: Adding missing tests
  chore: Changes to the build process or auxiliary tools and libraries such as documentation generation
-->

<!-- ADD CLOSING TYPE AND ISSUE NUMBER AT TAIL
  (<CLOSING TYPE> #<ISSUE NUMBERS>)
  close: resolve not a bug(feature, docs, etc) completely
  fix: resolve a bug completely
  ref: not fully resolved or related to
-->

### Please check if the PR fulfills these requirements
- [x] It's the right issue type on the title
- [ ] When resolving a specific issue, it's referenced in the PR's title (e.g. `fix #xxx[,#xxx]`, where "xxx" is the issue number)
- [x] The commit message follows our guidelines
- [x] Tests for the changes have been added (for bug fixes/features)
- [ ] Docs have been added/updated (for bug fixes/features)
- [ ] It does not introduce a breaking change or has a description of the breaking change

### Description
* related issues
  * #2176
  * #2168


---
Thank you for your contribution to TOAST UI product. 🎉 😘 ✨



Patch:
diff --git a/apps/editor/src/convertors/toMarkdown/toMdConvertors.ts b/apps/editor/src/convertors/toMarkdown/toMdConvertors.ts
index 0833c468..978e9939 100644
--- a/apps/editor/src/convertors/toMarkdown/toMdConvertors.ts
+++ b/apps/editor/src/convertors/toMarkdown/toMdConvertors.ts
@@ -4,7 +4,7 @@ import isUndefined from 'tui-code-snippet/type/isUndefined';
 
 import { nodeTypeWriters, write } from './toMdNodeTypeWriters';
 
-import { repeat, escape, quote, escapeXml } from '@/utils/common';
+import { repeat, quote, escapeXml, escapeTextForLink } from '@/utils/common';
 
 import {
   ToMdConvertorMap,
@@ -167,14 +167,14 @@ export const toMdConvertors: ToMdConvertorMap = {
 
   image({ node }) {
     const { attrs } = node;
-    const altText = escape(attrs.altText || '');
-    const imageUrl = escapeXml(attrs.imageUrl);
-    const altAttr = altText ? ` alt="${altText}"` : '';
+    const { rawHTML, altText } = attrs;
+    const imageUrl = attrs.imageUrl.replace(/&amp;/g, '&');
+    const altAttr = altText ? ` alt="${escapeXml(altText)}"` : '';
 
     return {
-      rawHTML: attrs.rawHTML ? `<${attrs.rawHTML} src="${imageUrl}"${altAttr}>` : null,
+      rawHTML: rawHTML ? `<${rawHTML} src="${escapeXml(imageUrl)}"${altAttr}>` : null,
       attrs: {
-        altText,
+        altText: escapeTextForLink(altText || ''),
         imageUrl,
       },
     };
@@ -237,20 +237,19 @@ export const toMdConvertors: ToMdConvertorMap = {
 
   link({ node }, { entering }) {
     const { attrs } = node;
-    const linkUrl = escapeXml(attrs.linkUrl);
-    const { rawHTML } = attrs;
+    const { title, rawHTML } = attrs;
+    const linkUrl = attrs.linkUrl.replace(/&amp;/g, '&');
+    const titleAttr = title ? ` title="${escapeXml(title)}"` : '';
 
     if (entering) {
       return {
         delim: '[',
-        rawHTML: rawHTML ? `<${rawHTML} href="${linkUrl}">` : null,
+        rawHTML: rawHTML ? `<${rawHTML} href="${escapeXml(linkUrl)}"${titleAttr}>` : null,
       };
     }
 
-    const linkText = attrs.title ? ` ${quote(attrs.linkText)}` : '';
-
     return {
-      delim: `](${linkText}${linkUrl})`,
+      delim: `](${linkUrl}${title ? ` ${quote(escapeTextForLink(title))}` : ''})`,
       rawHTML: getCloseRawHTML(rawHTML),
     };
   },
diff --git a/apps/editor/src/convertors/toMarkdown/toMdNodeTypeWriters.ts b/apps/editor/src/convertors/toMarkdown/toMdNodeTypeWriters.ts
index bd4fb453..8de60e57 100644
--- a/apps/editor/src/convertors/toMarkdown/toMdNodeTypeWriters.ts
+++ b/apps/editor/src/convertors/toMarkdown/toMdNodeTypeWriters.ts
@@ -2,7 +2,7 @@ import { ProsemirrorNode } from 'prosemirror-model';
 
 import inArray from 'tui-code-snippet/array/inArray';
 
-import { repeat } from '@/utils/common';
+import { escapeTextForLink, repeat } from '@/utils/common';
 
 import {
   ToMdNodeTypeWriterMap,
@@ -60,7 +60,13 @@ function createTableHeadDelim(textContent: string, columnAlign: ColumnAlign) {
 
 export const nodeTypeWriters: ToMdNodeTypeWriterMap = {
   text(state, { node }) {
-    state.text(node.text ?? '');
+    const text = node.text ?? '';
+
+    if ((node.marks || []).some((mark) => mark.type.name === 'link')) {
+      state.text(escapeTextForLink(text), false);
+    } else {
+      state.text(text);
+    }
   },
 
   paragraph(state, { node, parent, index = 0 }) {
diff --git a/apps/editor/src/convertors/toWysiwyg/toWwConvertors.ts b/apps/editor/src/convertors/toWysiwyg/toWwConvertors.ts
index e32672d8..1a0c274c 100644
--- a/apps/editor/src/convertors/toWysiwyg/toWwConvertors.ts
+++ b/apps/editor/src/convertors/toWysiwyg/toWwConvertors.ts
@@ -175,7 +175,7 @@ const toWwConvertors: ToWwConvertorMap = {
     if (entering) {
       const attrs = {
         linkUrl: destination,
-        ...(title && { linkText: title }),
+        title,
         ...customAttrs,
       };
 
diff --git a/apps/editor/src/markdown/marks/link.ts b/apps/editor/src/markdown/marks/link.ts
index fa54d3e0..2c5f9a73 100644
--- a/apps/editor/src/markdown/marks/link.ts
+++ b/apps/editor/src/markdown/marks/link.ts
@@ -1,8 +1,8 @@
 import { DOMOutputSpecArray, Mark as ProsemirrorMark } from 'prosemirror-model';
 import { EditorCommand } from '@t/spec';
 import { clsWithMdPrefix } from '@/utils/dom';
+import { escapeTextForLink } from '@/utils/common';
 import Mark from '@/spec/mark';
-import { encodeMarkdownText, escapeMarkdownText } from '@/utils/encoder';
 import { createTextNode } from '@/helper/manipulation';
 import { resolveSelectionPos } from '../helper/pos';
 
@@ -56,8 +56,7 @@ export class Link extends Mark {
         syntax = '!';
       }
 
-      text = escapeMarkdownText(text);
-      url = encodeMarkdownText(url);
+      text = escapeTextForLink(text);
       syntax += `[${text}](${url})`;
 
       dispatch!(tr.replaceWith(from, to, createTextNode(schema, syntax)));
diff --git a/apps/editor/src/utils/common.ts b/apps/editor/src/utils/common.ts
index f5ef7b43..709ff21d 100644
--- a/apps/editor/src/utils/common.ts
+++ b/apps/editor/src/utils/common.ts
@@ -11,6 +11,8 @@ const reEscapeChars = /[>(){}[\]+-.!#|]/g;
 const reEscapeHTML = /<([a-zA-Z_][a-zA-Z0-9\-._]*)(\s|[^\\/>])*\/?>|<(\/)([a-zA-Z_][a-zA-Z0-9\-._]*)\s*\/?>|<!--[^-]+-->|<([a-zA-Z_][a-zA-Z0-9\-.:/]*)>/g;
 const reEscapeBackSlash = /\\[!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~\\]/g;
 const reEscapePairedChars = /[*_~`]/g;
+const reMdImageSyntax = /!\[.*\]\(.*\)/g;
+const reEscapedCharInLinkSyntax = /[[\]]/g; //
 
 const XMLSPECIAL = '[&<>"]';
 const reXmlSpecial = new RegExp(XMLSPECIAL, 'g');
@@ -99,6 +101,22 @@ function isNeedEscapeText(text: string) {
   return needEscape;
 }
 
+export function escapeTextForLink(text: string) {
+  const imageSyntaxRanges: [number, number][] = [];
+  let result = reMdImageSyntax.exec(text);
+
+  while (result) {
+    imageSyntaxRanges.push([result.index, result.index + result[0].length]);
+    result = reMdImageSyntax.exec(text);
+  }
+
+  return text.replace(reEscapedCharInLinkSyntax, (matched, offset) => {
+    const isDelimiter = imageSyntaxRanges.some((range) => offset > range[0] && offset < range[1]);
+
+    return isDelimiter ? matched : `\\${matched}`;
+  });
+}
+
 export function escape(text: string) {
   const replacer = (matched: string) => `\\${matched}`;
 
diff --git a/apps/editor/src/utils/encoder.ts b/apps/editor/src/utils/encoder.ts
deleted file mode 100644
index 55ab13bb..00000000
--- a/apps/editor/src/utils/encoder.ts
+++ /dev/null
@@ -1,45 +0,0 @@
-const encoderList = [
-  {
-    regExp: /\(/g,
-    encoded: '%28',
-    escaped: '\\(',
-  },
-  {
-    regExp: /\)/g,
-    encoded: '%29',
-    escaped: '\\)',
-  },
-  {
-    regExp: /\[/g,
-    encoded: '%5B',
-    escaped: '\\[',
-  },
-  {
-    regExp: /\]/g,
-    encoded: '%5D',
-    escaped: '\\]',
-  },
-  {
-    regExp: /</g,
-    encoded: '%3C',
-    escaped: '\\<',
-  },
-  {
-    regExp: />/g,
-    encoded: '%3E',
-    escaped: '\\>',
-  },
-  {
-    regExp: / /g,
-    encoded: '%20',
-    escaped: ' ',
-  },
-];
-
-export function escapeMarkdownText(text: string) {
-  return encoderList.reduce((result, { regExp, escaped }) => result.replace(regExp, escaped), text);
-}
-
-export function encodeMarkdownText(text: string) {
-  return encoderList.reduce((result, { regExp, encoded }) => result.replace(regExp, encoded), text);
-}
diff --git a/apps/editor/src/wysiwyg/adaptor/mdLikeNode.ts b/apps/editor/src/wysiwyg/adaptor/mdLikeNode.ts
index c3b7f12b..4a55b218 100644
--- a/apps/editor/src/wysiwyg/adaptor/mdLikeNode.ts
+++ b/apps/editor/src/wysiwyg/adaptor/mdLikeNode.ts
@@ -43,7 +43,7 @@ export function createMdLikeNode(node: ProsemirrorNode | Mark): MdLikeNode {
 
   const nodeTypeMap = {
     heading: { level: attrs.level },
-    link: { destination: attrs.linkUrl, title: attrs.linkText },
+    link: { destination: attrs.linkUrl, title: attrs.title },
     image: { destination: attrs.imageUrl },
     codeBlock: { info: attrs.language },
     bulletList: { type: 'list', listData: { type: 'bullet' } },
diff --git a/apps/editor/src/wysiwyg/marks/link.ts b/apps/editor/src/wysiwyg/marks/link.ts
index 5d7c7de6..f6a4d587 100644
--- a/apps/editor/src/wysiwyg/marks/link.ts
+++ b/apps/editor/src/wysiwyg/marks/link.ts
@@ -2,7 +2,6 @@ import { Mark as ProsemirrorMark, DOMOutputSpecArray } from 'prosemirror-model';
 import { toggleMark } from 'prosemirror-commands';
 
 import Mark from '@/spec/mark';
-import { escapeMarkdownText } from '@/utils/encoder';
 import { escapeXml } from '@/utils/common';
 import { sanitizeHTML } from '@/sanitizer/htmlSanitizer';
 import { createTextNode } from '@/helper/manipulation';
@@ -27,7 +26,7 @@ export class Link extends Mark {
     return {
       attrs: {
         linkUrl: { default: '' },
-        linkText: { default: null },
+        title: { default: null },
         rawHTML: { default: null },
         ...getDefaultCustomAttrs(),
       },
@@ -39,11 +38,12 @@ export class Link extends Mark {
             const sanitizedDOM = sanitizeHTML<DocumentFragment>(dom, { RETURN_DOM_FRAGMENT: true })
               .firstChild as HTMLElement;
             const href = sanitizedDOM.getAttribute('href') || '';
+            const title = sanitizedDOM.getAttribute('title') || '';
             const rawHTML = sanitizedDOM.getAttribute('data-raw-html');
 
             return {
               linkUrl: href,
-              linkText: sanitizedDOM.textContent,
+              title,
               ...(rawHTML && { rawHTML }),
             };
           },
@@ -52,7 +52,7 @@ export class Link extends Mark {
       toDOM: ({ attrs }: ProsemirrorMark): DOMOutputSpecArray => [
         attrs.rawHTML || 'a',
         {
-          href: attrs.linkUrl,
+          href: escapeXml(attrs.linkUrl),
           ...(this.linkAttributes as DOMOutputSpecArray),
           ...getCustomAttrs(attrs),
         },
@@ -67,10 +67,7 @@ export class Link extends Mark {
       const { empty, from, to } = selection;
 
       if (from && to && linkUrl) {
-        const attrs = {
-          linkUrl: escapeXml(linkUrl),
-          linkText: escapeMarkdownText(linkText),
-        };
+        const attrs = { linkUrl };
         const mark = schema.mark('link', attrs);
 
         if (empty && linkText) {
diff --git a/apps/editor/src/wysiwyg/nodes/image.ts b/apps/editor/src/wysiwyg/nodes/image.ts
index bcc67b62..6be15e87 100644
--- a/apps/editor/src/wysiwyg/nodes/image.ts
+++ b/apps/editor/src/wysiwyg/nodes/image.ts
@@ -45,7 +45,7 @@ export class Image extends NodeSchema {
         return [
           attrs.rawHTML || 'img',
           {
-            src: attrs.imageUrl,
+            src: escapeXml(attrs.imageUrl),
             ...(attrs.altText && { alt: attrs.altText }),
             ...getCustomAttrs(attrs),
           },
@@ -63,7 +63,7 @@ export class Image extends NodeSchema {
       }
 
       const node = schema.nodes.image.createAndFill({
-        imageUrl: escapeXml(imageUrl),
+        imageUrl,
         ...(altText && { altText }),
       });
 


Test patch:
diff --git a/apps/editor/src/__test__/unit/convertor.spec.ts b/apps/editor/src/__test__/unit/convertor.spec.ts
index a86a6fe5..2a44a548 100644
--- a/apps/editor/src/__test__/unit/convertor.spec.ts
+++ b/apps/editor/src/__test__/unit/convertor.spec.ts
@@ -157,11 +157,13 @@ describe('Convertor', () => {
         ![](imgUrl)
         ![altText](imgUrl)
         ![altText](img*Url)
-      `;
+        ![altText](url?key=abc&attribute=abc)
+        `;
       const expected = source`
         ![](imgUrl)
         ![altText](imgUrl)
         ![altText](img*Url)
+        ![altText](url?key=abc&attribute=abc)
     	`;
 
       assertConverting(markdown, expected);
@@ -173,12 +175,16 @@ describe('Convertor', () => {
         [text](url)
         [text](ur*l)
         [Editor](https://github.com/nhn_test/tui.editor)
-    	`;
+        [this.is_a_test_link.com](this.is_a_test_link.com)
+        [text](url?key=abc&attribute=abc)
+        `;
       const expected = source`
         foo
         [text](url)
         [text](ur*l)
         [Editor](https://github.com/nhn_test/tui.editor)
+        [this.is_a_test_link.com](this.is_a_test_link.com)
+        [text](url?key=abc&attribute=abc)
     	`;
 
       assertConverting(markdown, expected);
diff --git a/apps/editor/src/__test__/unit/markdown/mdCommand.spec.ts b/apps/editor/src/__test__/unit/markdown/mdCommand.spec.ts
index 5b9b268f..50ec12f9 100644
--- a/apps/editor/src/__test__/unit/markdown/mdCommand.spec.ts
+++ b/apps/editor/src/__test__/unit/markdown/mdCommand.spec.ts
@@ -281,7 +281,7 @@ describe('addImage command', () => {
       imageUrl: 'https://picsum.photos/200',
     });
 
-    expect(getTextContent(mde)).toBe('![mytext \\(\\)\\[\\]\\<\\>](https://picsum.photos/200)');
+    expect(getTextContent(mde)).toBe('![mytext ()\\[\\]<>](https://picsum.photos/200)');
   });
 
   it('should encode image url', () => {
@@ -290,7 +290,7 @@ describe('addImage command', () => {
       imageUrl: 'myurl ()[]<>',
     });
 
-    expect(getTextContent(mde)).toBe('![image](myurl%20%28%29%5B%5D%3C%3E)');
+    expect(getTextContent(mde)).toBe('![image](myurl ()[]<>)');
   });
 
   it('should not decode url which is already encoded', () => {
@@ -318,16 +318,7 @@ describe('addLink command', () => {
       linkUrl: 'https://ui.toast.com',
     });
 
-    expect(getTextContent(mde)).toBe('[mytext \\(\\)\\[\\]\\<\\>](https://ui.toast.com)');
-  });
-
-  it('should encode link url', () => {
-    cmd.exec('addLink', {
-      linkText: 'TOAST UI',
-      linkUrl: 'myurl ()[]<>',
-    });
-
-    expect(getTextContent(mde)).toBe('[TOAST UI](myurl%20%28%29%5B%5D%3C%3E)');
+    expect(getTextContent(mde)).toBe('[mytext ()\\[\\]<>](https://ui.toast.com)');
   });
 
   it('should not decode url which is already encoded', () => {
diff --git a/apps/editor/src/__test__/unit/wysiwyg/wwToDOMAdaptor.spec.ts b/apps/editor/src/__test__/unit/wysiwyg/wwToDOMAdaptor.spec.ts
index 48d17830..9cb4e7e0 100644
--- a/apps/editor/src/__test__/unit/wysiwyg/wwToDOMAdaptor.spec.ts
+++ b/apps/editor/src/__test__/unit/wysiwyg/wwToDOMAdaptor.spec.ts
@@ -212,8 +212,8 @@ describe('mdLikeNode', () => {
       type: 'link',
       literal: null,
       wysiwygNode: true,
-      title: 'myLinkText',
       destination: 'myLinkUrl',
+      title: null,
     });
   });