From 3583e33dab3556d0f7a032cd190798878f692460 Mon Sep 17 00:00:00 2001 From: jaesung-lee Date: Tue, 6 Jul 2021 16:27:37 +0900 Subject: [PATCH] fix: encode link url unnecessarily (#1641) * fix: decode url when executing addImage, addLink command * chore: add test case(addImage, addLink) * chore: apply code review * refactor: assertToContainHTML function * chore: add test case(converting the html block which has "=" attr value) --- src/__test__/unit/convertor.spec.ts | 7 ++ src/__test__/unit/markdown/mdCommand.spec.ts | 22 ++++++ src/__test__/unit/wysiwyg/wwCommand.spec.ts | 30 ++++---- src/__test__/unit/wysiwyg/wwEditor.spec.ts | 14 ++-- src/markdown/marks/link.ts | 6 +- src/utils/encoder.ts | 72 +++++++++++--------- src/wysiwyg/marks/link.ts | 6 +- src/wysiwyg/nodes/html.ts | 6 +- src/wysiwyg/nodes/image.ts | 6 +- 9 files changed, 106 insertions(+), 63 deletions(-) diff --git a/src/__test__/unit/convertor.spec.ts b/src/__test__/unit/convertor.spec.ts index 2e50ced36d..0f929d340a 100644 --- a/src/__test__/unit/convertor.spec.ts +++ b/src/__test__/unit/convertor.spec.ts @@ -828,6 +828,13 @@ describe('Convertor', () => { assertConverting(markdown, markdown); }); + it('should convert html block element which has "=" character as the attribute value', () => { + const markdown = + ''; + + assertConverting(markdown, markdown); + }); + it('should convert html inline node', () => { const markdown = 'content'; diff --git a/src/__test__/unit/markdown/mdCommand.spec.ts b/src/__test__/unit/markdown/mdCommand.spec.ts index 4fb9bc0f49..aeb81cef87 100644 --- a/src/__test__/unit/markdown/mdCommand.spec.ts +++ b/src/__test__/unit/markdown/mdCommand.spec.ts @@ -292,6 +292,17 @@ describe('addImage command', () => { expect(getTextContent(mde)).toBe('![image](myurl%20%28%29%5B%5D%3C%3E)'); }); + + it('should not decode url which is already encoded', () => { + cmd.exec('addImage', { + altText: 'image', + imageUrl: 'https://firebasestorage.googleapis.com/images%2Fimage.png?alt=media', + }); + + expect(getTextContent(mde)).toBe( + '![image](https://firebasestorage.googleapis.com/images%2Fimage.png?alt=media)' + ); + }); }); describe('addLink command', () => { @@ -318,6 +329,17 @@ describe('addLink command', () => { expect(getTextContent(mde)).toBe('[TOAST UI](myurl%20%28%29%5B%5D%3C%3E)'); }); + + it('should not decode url which is already encoded', () => { + cmd.exec('addLink', { + linkText: 'TOAST UI', + linkUrl: 'https://firebasestorage.googleapis.com/links%2Fimage.png?alt=media', + }); + + expect(getTextContent(mde)).toBe( + '[TOAST UI](https://firebasestorage.googleapis.com/links%2Fimage.png?alt=media)' + ); + }); }); describe('heading command', () => { diff --git a/src/__test__/unit/wysiwyg/wwCommand.spec.ts b/src/__test__/unit/wysiwyg/wwCommand.spec.ts index 73bf45daac..3b6bcb6119 100644 --- a/src/__test__/unit/wysiwyg/wwCommand.spec.ts +++ b/src/__test__/unit/wysiwyg/wwCommand.spec.ts @@ -484,14 +484,14 @@ describe('wysiwyg commands', () => { expect(wwe.getHTML()).toBe('


'); }); - it('should decode attribute and encode wrong markdown charactors', () => { + it('should not decode url which is already encoded', () => { cmd.exec('addImage', { - imageUrl: 'foo %D1%88%D0%B5%D0%BB%D0%BB%D1%8B ()[]<>', - altText: 'foo ()[]<>', + imageUrl: 'https://firebasestorage.googleapis.com/images%2Fimage.png?alt=media', + altText: 'foo', }); expect(wwe.getHTML()).toBe( - '

foo \\(\\)\\[\\]\\<\\>

' + '

foo

' ); }); }); @@ -520,17 +520,6 @@ describe('wysiwyg commands', () => { expect(wwe.getHTML()).toBe('


'); }); - it('should decode attribute and encode wrong markdown charactors', () => { - cmd.exec('addLink', { - linkUrl: 'foo %D1%88%D0%B5%D0%BB%D0%BB%D1%8B ()[]<>', - linkText: 'foo ()[]<>', - }); - - expect(wwe.getHTML()).toBe( - '

foo ()[]<>

' - ); - }); - it('should change link url in selection', () => { cmd.exec('addLink', { linkUrl: '#', @@ -554,6 +543,17 @@ describe('wysiwyg commands', () => { expect(wwe.getHTML()).toBe(expected); }); + + it('should not decode url which is already encoded', () => { + cmd.exec('addLink', { + linkUrl: 'https://firebasestorage.googleapis.com/links%2Fimage.png?alt=media', + linkText: 'foo', + }); + + expect(wwe.getHTML()).toBe( + '

foo

' + ); + }); }); describe(`addLink command with 'linkAttributes' option`, () => { diff --git a/src/__test__/unit/wysiwyg/wwEditor.spec.ts b/src/__test__/unit/wysiwyg/wwEditor.spec.ts index 8ddb1dc051..7ace3f6d15 100644 --- a/src/__test__/unit/wysiwyg/wwEditor.spec.ts +++ b/src/__test__/unit/wysiwyg/wwEditor.spec.ts @@ -14,6 +14,10 @@ jest.useFakeTimers(); describe('WysiwygEditor', () => { let wwe: WysiwygEditor, em: EventEmitter, el: HTMLElement; + function assertToContainHTML(html: string) { + expect(wwe.view.dom).toContainHTML(html); + } + function setContent(content: string) { const wrapper = document.createElement('div'); @@ -81,7 +85,7 @@ describe('WysiwygEditor', () => { it('setPlaceholder() attach placeholder element', () => { wwe.setPlaceholder('placeholder text'); - expect(wwe.getHTML()).toBe(oneLineTrim` + assertToContainHTML(oneLineTrim`

placeholder text
@@ -128,7 +132,7 @@ describe('WysiwygEditor', () => { wwe.setSelection(3, 7); wwe.replaceSelection('new foo\nnew bar'); - expect(wwe.getHTML()).toBe(oneLineTrim` + assertToContainHTML(oneLineTrim`

fonew foo

new barar

`); @@ -172,7 +176,7 @@ describe('WysiwygEditor', () => { '' ); - expect(wwe.getHTML()).toBe( + assertToContainHTML( '' ); }); @@ -180,13 +184,13 @@ describe('WysiwygEditor', () => { it('should display html inline element properly', () => { setContent('text'); - expect(wwe.getHTML()).toBe('

text

'); + assertToContainHTML('

text

'); }); it('should sanitize html element', () => { setContent(''); - expect(wwe.getHTML()).toBe( + assertToContainHTML( '' ); }); diff --git a/src/markdown/marks/link.ts b/src/markdown/marks/link.ts index 99e32d80c7..fa54d3e04e 100644 --- a/src/markdown/marks/link.ts +++ b/src/markdown/marks/link.ts @@ -2,7 +2,7 @@ import { DOMOutputSpecArray, Mark as ProsemirrorMark } from 'prosemirror-model'; import { EditorCommand } from '@t/spec'; import { clsWithMdPrefix } from '@/utils/dom'; import Mark from '@/spec/mark'; -import { decodeURIGraceful, encodeMarkdownText } from '@/utils/encoder'; +import { encodeMarkdownText, escapeMarkdownText } from '@/utils/encoder'; import { createTextNode } from '@/helper/manipulation'; import { resolveSelectionPos } from '../helper/pos'; @@ -56,8 +56,8 @@ export class Link extends Mark { syntax = '!'; } - text = encodeMarkdownText(text, false); - url = encodeMarkdownText(decodeURIGraceful(url), true); + text = escapeMarkdownText(text); + url = encodeMarkdownText(url); syntax += `[${text}](${url})`; dispatch!(tr.replaceWith(from, to, createTextNode(schema, syntax))); diff --git a/src/utils/encoder.ts b/src/utils/encoder.ts index d6347bb624..55ab13bb0b 100644 --- a/src/utils/encoder.ts +++ b/src/utils/encoder.ts @@ -1,35 +1,45 @@ -const reURL = /^(https?:\/\/)?([\da-z.-]+)\.([a-z.]{2,6})(\/([^\s]*))?$/g; -const encodingRegExps = [/\(/g, /\)/g, /\[/g, /\]/g, //g]; -const encodedList = ['%28', '%29', '%5B', '%5D', '%3C', '%3E']; -const escapedList = ['\\(', '\\)', '\\[', '\\]', '\\<', '\\>']; +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: '%3E', + escaped: '\\>', + }, + { + regExp: / /g, + encoded: '%20', + escaped: ' ', + }, +]; -export function decodeURIGraceful(uri: string) { - const uriList = uri.split(' '); - - return uriList - .reduce((decodedURIList, targetUri) => { - let decodedURI = ''; - - try { - decodedURI = decodeURIComponent(targetUri); - } catch (e) { - decodedURI = targetUri; - } - - return decodedURIList.concat(decodedURI); - }, []) - .join('%20'); -} - -export function encodeMarkdownText(text: string, encode: boolean) { - const expectedValues = encode ? encodedList : escapedList; - - return encodingRegExps.reduce( - (result, regExp, index) => result.replace(regExp, expectedValues[index]), - text - ); +export function escapeMarkdownText(text: string) { + return encoderList.reduce((result, { regExp, escaped }) => result.replace(regExp, escaped), text); } -export function decodeURL(text: string) { - return text.replace(reURL, (matched) => encodeMarkdownText(decodeURIGraceful(matched), true)); +export function encodeMarkdownText(text: string) { + return encoderList.reduce((result, { regExp, encoded }) => result.replace(regExp, encoded), text); } diff --git a/src/wysiwyg/marks/link.ts b/src/wysiwyg/marks/link.ts index 0fc0a5f8a9..b87e4cae9f 100644 --- a/src/wysiwyg/marks/link.ts +++ b/src/wysiwyg/marks/link.ts @@ -2,7 +2,7 @@ import { Mark as ProsemirrorMark, DOMOutputSpecArray } from 'prosemirror-model'; import { toggleMark } from 'prosemirror-commands'; import Mark from '@/spec/mark'; -import { decodeURIGraceful, encodeMarkdownText } from '@/utils/encoder'; +import { encodeMarkdownText, escapeMarkdownText } from '@/utils/encoder'; import { sanitizeXSSAttributeValue } from '@/sanitizer/htmlSanitizer'; import { createTextNode } from '@/helper/manipulation'; import { getCustomAttrs, getDefaultCustomAttrs } from '@/wysiwyg/helper/node'; @@ -65,8 +65,8 @@ export class Link extends Mark { if (from && to && linkUrl) { const attrs = { - linkUrl: encodeMarkdownText(decodeURIGraceful(linkUrl), true), - linkText: encodeMarkdownText(linkText, false), + linkUrl: encodeMarkdownText(linkUrl), + linkText: escapeMarkdownText(linkText), }; const mark = schema.mark('link', attrs); diff --git a/src/wysiwyg/nodes/html.ts b/src/wysiwyg/nodes/html.ts index 3900304817..de382861b2 100644 --- a/src/wysiwyg/nodes/html.ts +++ b/src/wysiwyg/nodes/html.ts @@ -24,10 +24,10 @@ export function getHTMLAttrsByHTMLString(html: string) { return attrs ? attrs.reduce>((acc, attr) => { - const [name, ...value] = attr.trim().split('='); + const [name, ...values] = attr.trim().split('='); - if (value.length > 0) { - acc[name] = value.join('=').replace(/'|"/g, '').trim(); + if (values.length) { + acc[name] = values.join('=').replace(/'|"/g, '').trim(); } return acc; diff --git a/src/wysiwyg/nodes/image.ts b/src/wysiwyg/nodes/image.ts index 13321a740c..5780dd5152 100644 --- a/src/wysiwyg/nodes/image.ts +++ b/src/wysiwyg/nodes/image.ts @@ -1,7 +1,7 @@ import { Node as ProsemirrorNode, DOMOutputSpecArray } from 'prosemirror-model'; import NodeSchema from '@/spec/node'; -import { decodeURIGraceful, encodeMarkdownText } from '@/utils/encoder'; +import { encodeMarkdownText } from '@/utils/encoder'; import { sanitizeXSSAttributeValue } from '@/sanitizer/htmlSanitizer'; import { EditorCommand } from '@t/spec'; @@ -60,8 +60,8 @@ export class Image extends NodeSchema { } const node = schema.nodes.image.createAndFill({ - imageUrl: encodeMarkdownText(decodeURIGraceful(imageUrl), true), - ...(altText && { altText: encodeMarkdownText(altText, false) }), + imageUrl: encodeMarkdownText(imageUrl), + ...(altText && { altText }), }); dispatch!(tr.replaceSelectionWith(node!).scrollIntoView());