From 58e44df72cf604e19771254e05a8b9e156d98751 Mon Sep 17 00:00:00 2001 From: Himanshu Kapoor <hkapoor@gitlab.com> Date: Tue, 15 Nov 2022 11:27:31 +0700 Subject: [PATCH] Add support for rendering comments in content editor Add support for rendering and editing HTML comments in the comments in the content editor. Changelog: added --- .../components/toolbar_more_dropdown.vue | 3 ++ .../content_editor/extensions/comment.js | 49 +++++++++++++++++++ .../services/create_content_editor.js | 2 + .../services/gl_api_markdown_deserializer.js | 5 +- .../services/markdown_serializer.js | 3 ++ .../services/serialization_helpers.js | 7 +++ app/assets/javascripts/lib/utils/dom_utils.js | 21 ++++++++ .../components/content_editor.scss | 12 +++++ app/controllers/concerns/preview_markdown.rb | 8 ++- .../output_example_snapshots/html.yml | 24 +++++---- glfm_specification/output_spec/spec.html | 4 +- lib/api/markdown.rb | 5 ++ lib/banzai/filter/sanitization_filter.rb | 2 + .../markdown_golden_master_examples.yml | 10 ++++ .../content_editor/extensions/comment_spec.js | 30 ++++++++++++ .../gl_api_markdown_deserializer_spec.js | 12 +++-- .../services/markdown_serializer_spec.js | 14 ++++++ spec/frontend/content_editor/test_utils.js | 2 + spec/frontend/lib/utils/dom_utils_spec.js | 18 +++++++ 19 files changed, 214 insertions(+), 17 deletions(-) create mode 100644 app/assets/javascripts/content_editor/extensions/comment.js create mode 100644 spec/frontend/content_editor/extensions/comment_spec.js diff --git a/app/assets/javascripts/content_editor/components/toolbar_more_dropdown.vue b/app/assets/javascripts/content_editor/components/toolbar_more_dropdown.vue index 6bb122153ef51..93b31ea7d205f 100644 --- a/app/assets/javascripts/content_editor/components/toolbar_more_dropdown.vue +++ b/app/assets/javascripts/content_editor/components/toolbar_more_dropdown.vue @@ -58,6 +58,9 @@ export default { right lazy > + <gl-dropdown-item @click="insert('comment')"> + {{ __('Comment') }} + </gl-dropdown-item> <gl-dropdown-item @click="insert('codeBlock')"> {{ __('Code block') }} </gl-dropdown-item> diff --git a/app/assets/javascripts/content_editor/extensions/comment.js b/app/assets/javascripts/content_editor/extensions/comment.js new file mode 100644 index 0000000000000..8e247e552a397 --- /dev/null +++ b/app/assets/javascripts/content_editor/extensions/comment.js @@ -0,0 +1,49 @@ +import { Node, textblockTypeInputRule } from '@tiptap/core'; + +export const commentInputRegex = /^<!--[\s\n]$/; + +export default Node.create({ + name: 'comment', + content: 'text*', + marks: '', + group: 'block', + code: true, + isolating: true, + defining: true, + + parseHTML() { + return [ + { + tag: 'comment', + preserveWhitespace: 'full', + getContent(element, schema) { + const node = schema.node('paragraph', {}, [ + schema.text( + element.textContent.replace(/&#x([0-9A-F]{2,4});/gi, (_, code) => + String.fromCharCode(parseInt(code, 16)), + ) || ' ', + ), + ]); + return node.content; + }, + }, + ]; + }, + + renderHTML() { + return [ + 'pre', + { class: 'gl-p-0 gl-border-0 gl-bg-transparent gl-text-gray-300' }, + ['span', { class: 'content-editor-comment' }, 0], + ]; + }, + + addInputRules() { + return [ + textblockTypeInputRule({ + find: commentInputRegex, + type: this.type, + }), + ]; + }, +}); diff --git a/app/assets/javascripts/content_editor/services/create_content_editor.js b/app/assets/javascripts/content_editor/services/create_content_editor.js index ba9ce705c62cf..61c6be574d07b 100644 --- a/app/assets/javascripts/content_editor/services/create_content_editor.js +++ b/app/assets/javascripts/content_editor/services/create_content_editor.js @@ -10,6 +10,7 @@ import BulletList from '../extensions/bullet_list'; import Code from '../extensions/code'; import CodeBlockHighlight from '../extensions/code_block_highlight'; import ColorChip from '../extensions/color_chip'; +import Comment from '../extensions/comment'; import DescriptionItem from '../extensions/description_item'; import DescriptionList from '../extensions/description_list'; import Details from '../extensions/details'; @@ -100,6 +101,7 @@ export const createContentEditor = ({ BulletList, Code, ColorChip, + Comment, CodeBlockHighlight, DescriptionItem, DescriptionList, diff --git a/app/assets/javascripts/content_editor/services/gl_api_markdown_deserializer.js b/app/assets/javascripts/content_editor/services/gl_api_markdown_deserializer.js index fa46bd9ff8153..796dc06ad938e 100644 --- a/app/assets/javascripts/content_editor/services/gl_api_markdown_deserializer.js +++ b/app/assets/javascripts/content_editor/services/gl_api_markdown_deserializer.js @@ -1,4 +1,5 @@ import { DOMParser as ProseMirrorDOMParser } from 'prosemirror-model'; +import { replaceCommentsWith } from '~/lib/utils/dom_utils'; export default ({ render }) => { /** @@ -22,7 +23,9 @@ export default ({ render }) => { if (!html) return {}; const parser = new DOMParser(); - const { body } = parser.parseFromString(html, 'text/html'); + const { body } = parser.parseFromString(`<body>${html}</body>`, 'text/html'); + + replaceCommentsWith(body, 'comment'); // append original source as a comment that nodes can access body.append(document.createComment(markdown)); diff --git a/app/assets/javascripts/content_editor/services/markdown_serializer.js b/app/assets/javascripts/content_editor/services/markdown_serializer.js index 958c27c281a80..4e29f85004b79 100644 --- a/app/assets/javascripts/content_editor/services/markdown_serializer.js +++ b/app/assets/javascripts/content_editor/services/markdown_serializer.js @@ -12,6 +12,7 @@ import DescriptionItem from '../extensions/description_item'; import DescriptionList from '../extensions/description_list'; import Details from '../extensions/details'; import DetailsContent from '../extensions/details_content'; +import Comment from '../extensions/comment'; import Diagram from '../extensions/diagram'; import Emoji from '../extensions/emoji'; import Figure from '../extensions/figure'; @@ -50,6 +51,7 @@ import Text from '../extensions/text'; import Video from '../extensions/video'; import WordBreak from '../extensions/word_break'; import { + renderComment, renderCodeBlock, renderHardBreak, renderTable, @@ -130,6 +132,7 @@ const defaultSerializerConfig = { }), [BulletList.name]: preserveUnchanged(renderBulletList), [CodeBlockHighlight.name]: preserveUnchanged(renderCodeBlock), + [Comment.name]: renderComment, [Diagram.name]: preserveUnchanged(renderCodeBlock), [DescriptionList.name]: renderHTMLNode('dl', true), [DescriptionItem.name]: (state, node, parent, index) => { diff --git a/app/assets/javascripts/content_editor/services/serialization_helpers.js b/app/assets/javascripts/content_editor/services/serialization_helpers.js index 5c0cb21075a8c..5ee9d66def166 100644 --- a/app/assets/javascripts/content_editor/services/serialization_helpers.js +++ b/app/assets/javascripts/content_editor/services/serialization_helpers.js @@ -324,6 +324,13 @@ export function renderPlayable(state, node) { renderImage(state, node); } +export function renderComment(state, node) { + state.text('<!--'); + state.text(node.textContent); + state.text('-->'); + state.closeBlock(node); +} + export function renderCodeBlock(state, node) { state.write(`\`\`\`${node.attrs.language || ''}\n`); state.text(node.textContent, false); diff --git a/app/assets/javascripts/lib/utils/dom_utils.js b/app/assets/javascripts/lib/utils/dom_utils.js index cafee641174a3..317c401e404dc 100644 --- a/app/assets/javascripts/lib/utils/dom_utils.js +++ b/app/assets/javascripts/lib/utils/dom_utils.js @@ -118,3 +118,24 @@ export const getContentWrapperHeight = (contentWrapperClass) => { const wrapperEl = document.querySelector(contentWrapperClass); return wrapperEl ? `${wrapperEl.offsetTop}px` : ''; }; + +/** + * Replaces comment nodes in a DOM tree with a different element + * containing the text of the comment. + * + * @param {*} el + * @param {*} tagName + */ +export const replaceCommentsWith = (el, tagName) => { + const iterator = document.createNodeIterator(el, NodeFilter.SHOW_COMMENT); + let commentNode = iterator.nextNode(); + + while (commentNode) { + const newNode = document.createElement(tagName); + newNode.textContent = commentNode.textContent; + + commentNode.parentNode.replaceChild(newNode, commentNode); + + commentNode = iterator.nextNode(); + } +}; diff --git a/app/assets/stylesheets/components/content_editor.scss b/app/assets/stylesheets/components/content_editor.scss index 1b6a0208ca75c..44b06c0ff12c0 100644 --- a/app/assets/stylesheets/components/content_editor.scss +++ b/app/assets/stylesheets/components/content_editor.scss @@ -130,6 +130,18 @@ background-color: var(--gl-color-chip-color); } +.content-editor-comment { + &::before { + content: '<!--'; + } + + &::after { + content: '-->'; + } +} + + + .bubble-menu-form { width: 320px; } diff --git a/app/controllers/concerns/preview_markdown.rb b/app/controllers/concerns/preview_markdown.rb index 7af114313a165..a7655efe7a932 100644 --- a/app/controllers/concerns/preview_markdown.rb +++ b/app/controllers/concerns/preview_markdown.rb @@ -45,7 +45,13 @@ def markdown_context_params when 'projects' then projects_filter_params when 'timeline_events' then timeline_events_filter_params else {} - end.merge(requested_path: params[:path], ref: params[:ref]) + end.merge( + requested_path: params[:path], + ref: params[:ref], + # Disable comments in markdown for IE browsers because comments in IE + # could allow script execution. + allow_comments: !browser.ie? + ) end # rubocop:enable Gitlab/ModuleWithInstanceVariables diff --git a/glfm_specification/output_example_snapshots/html.yml b/glfm_specification/output_example_snapshots/html.yml index bdd0777ce17ee..99c844c8794aa 100644 --- a/glfm_specification/output_example_snapshots/html.yml +++ b/glfm_specification/output_example_snapshots/html.yml @@ -1911,7 +1911,7 @@ <!-- foo -->*bar* <p><em>baz</em></p> static: |- - *bar* + <!-- foo -->*bar* <p data-sourcepos="2:1-2:5" dir="auto"><em>baz</em></p> wysiwyg: |- <p>*bar* @@ -1933,8 +1933,11 @@ bar baz --> <p>okay</p> - static: |2- + static: |- + <!-- Foo + bar + baz --> <p data-sourcepos="5:1-5:4" dir="auto">okay</p> wysiwyg: |- <p>okay</p> @@ -1988,10 +1991,12 @@ <!-- foo --> <pre><code><!-- foo --> </code></pre> - static: " \n<div class=\"gl-relative markdown-code-block js-markdown-code\">\n<pre - data-sourcepos=\"3:5-3:16\" lang=\"plaintext\" class=\"code highlight js-syntax-highlight - language-plaintext\" data-canonical-lang=\"\" v-pre=\"true\"><code><span id=\"LC1\" - class=\"line\" lang=\"plaintext\"><!-- foo --></span></code></pre>\n<copy-code></copy-code>\n</div>" + static: |2- + <!-- foo --> + <div class="gl-relative markdown-code-block js-markdown-code"> + <pre data-sourcepos="3:5-3:16" lang="plaintext" class="code highlight js-syntax-highlight language-plaintext" data-canonical-lang="" v-pre="true"><code><span id="LC1" class="line" lang="plaintext"><!-- foo --></span></code></pre> + <copy-code></copy-code> + </div> wysiwyg: |- <pre class="content-editor-code-block undefined code highlight"><code><!-- foo --></code></pre> 04_06_00__leaf_blocks__html_blocks__036: @@ -4311,7 +4316,7 @@ <li data-sourcepos="1:1-1:5">foo</li> <li data-sourcepos="2:1-3:0">bar</li> </ul> - + <!-- --> <ul data-sourcepos="6:1-7:5" dir="auto"> <li data-sourcepos="6:1-6:5">baz</li> <li data-sourcepos="7:1-7:5">bim</li> @@ -4343,7 +4348,7 @@ <p data-sourcepos="5:5-5:7">foo</p> </li> </ul> - + <!-- --> <div class="gl-relative markdown-code-block js-markdown-code"> <pre data-sourcepos="9:5-9:8" lang="plaintext" class="code highlight js-syntax-highlight language-plaintext" data-canonical-lang="" v-pre="true"><code><span id="LC1" class="line" lang="plaintext">code</span></code></pre> <copy-code></copy-code> @@ -7382,7 +7387,8 @@ <p>foo <!-- this is a comment - with hyphen --></p> static: |- - <p data-sourcepos="1:1-2:25" dir="auto">foo </p> + <p data-sourcepos="1:1-2:25" dir="auto">foo <!-- this is a + comment - with hyphen --></p> wysiwyg: |- <p>foo </p> 06_11_00__inlines__raw_html__014: diff --git a/glfm_specification/output_spec/spec.html b/glfm_specification/output_spec/spec.html index 525c21a6ad1ff..2a4d03a3fdd2a 100644 --- a/glfm_specification/output_spec/spec.html +++ b/glfm_specification/output_spec/spec.html @@ -262,7 +262,7 @@ for a complete list of all examples, which are a superset of examples from:</p> <li data-sourcepos="14:1-14:88">GitLab Flavored Markdown Official Specification (the same ones from this specifiation)</li> <li data-sourcepos="15:1-16:0">GitLab Flavored Markdown Internal Extensions.</li> </ul> - +<!-- BEGIN TESTS --> <h1 data-sourcepos="18:1-18:40" dir="auto"> <a id="user-content-gitlab-official-specification-markdown" class="anchor" href="#gitlab-official-specification-markdown" aria-hidden="true"></a>GitLab Official Specification Markdown</h1> <p data-sourcepos="20:1-23:104" dir="auto">Currently, only some of the GitLab-specific markdown features are @@ -594,7 +594,7 @@ line.</p> <copy-code></copy-code> </div> </div> - +<!-- END TESTS --> </body> </html> diff --git a/lib/api/markdown.rb b/lib/api/markdown.rb index 276560f343359..f348e20cc0b02 100644 --- a/lib/api/markdown.rb +++ b/lib/api/markdown.rb @@ -35,6 +35,11 @@ class Markdown < ::API::Base context[:skip_project_check] = true end + # Disable comments in markdown for IE browsers because comments in IE + # could allow script execution. + browser = Browser.new(headers['User-Agent']) + context[:allow_comments] = !browser.ie? + present({ html: Banzai.render_and_post_process(params[:text], context) }, with: Entities::Markdown) end end diff --git a/lib/banzai/filter/sanitization_filter.rb b/lib/banzai/filter/sanitization_filter.rb index fe189b1b0c993..de9b846efe9be 100644 --- a/lib/banzai/filter/sanitization_filter.rb +++ b/lib/banzai/filter/sanitization_filter.rb @@ -10,6 +10,8 @@ class SanitizationFilter < Banzai::Filter::BaseSanitizationFilter TABLE_ALIGNMENT_PATTERN = /text-align: (?<alignment>center|left|right)/.freeze def customize_allowlist(allowlist) + allowlist[:allow_comments] = context[:allow_comments] + # Allow table alignment; we allow specific text-align values in a # transformer below allowlist[:attributes]['th'] = %w[style] diff --git a/spec/fixtures/markdown/markdown_golden_master_examples.yml b/spec/fixtures/markdown/markdown_golden_master_examples.yml index 0c7e6ab5cd212..9e7de030a29d6 100644 --- a/spec/fixtures/markdown/markdown_golden_master_examples.yml +++ b/spec/fixtures/markdown/markdown_golden_master_examples.yml @@ -347,6 +347,16 @@ <li data-sourcepos="9:1-9:25"><code>HSLA(540,70%,50%,0.3)<span class="gfm-color_chip"><span style="background-color: HSLA(540,70%,50%,0.3);"></span></span></code></li> </ul> +- name: comment + markdown: |- + <!-- this is a + multiline markdown + comment --> + html: |- + <!-- this is a + multiline markdown + comment --> + - name: description_list markdown: |- <dl> diff --git a/spec/frontend/content_editor/extensions/comment_spec.js b/spec/frontend/content_editor/extensions/comment_spec.js new file mode 100644 index 0000000000000..7d8ff28e4d7d7 --- /dev/null +++ b/spec/frontend/content_editor/extensions/comment_spec.js @@ -0,0 +1,30 @@ +import Comment from '~/content_editor/extensions/comment'; +import { createTestEditor, createDocBuilder, triggerNodeInputRule } from '../test_utils'; + +describe('content_editor/extensions/comment', () => { + let tiptapEditor; + let doc; + let comment; + + beforeEach(() => { + tiptapEditor = createTestEditor({ extensions: [Comment] }); + ({ + builders: { doc, comment }, + } = createDocBuilder({ + tiptapEditor, + names: { + comment: { nodeType: Comment.name }, + }, + })); + }); + + describe('when typing the comment input rule', () => { + it('inserts a comment node', () => { + const expectedDoc = doc(comment()); + + triggerNodeInputRule({ tiptapEditor, inputRuleText: '<!-- ' }); + + expect(tiptapEditor.getJSON()).toEqual(expectedDoc.toJSON()); + }); + }); +}); diff --git a/spec/frontend/content_editor/services/gl_api_markdown_deserializer_spec.js b/spec/frontend/content_editor/services/gl_api_markdown_deserializer_spec.js index 5458a42532f06..90d83820c7025 100644 --- a/spec/frontend/content_editor/services/gl_api_markdown_deserializer_spec.js +++ b/spec/frontend/content_editor/services/gl_api_markdown_deserializer_spec.js @@ -1,5 +1,6 @@ import createMarkdownDeserializer from '~/content_editor/services/gl_api_markdown_deserializer'; import Bold from '~/content_editor/extensions/bold'; +import Comment from '~/content_editor/extensions/comment'; import { createTestEditor, createDocBuilder } from '../test_utils'; describe('content_editor/services/gl_api_markdown_deserializer', () => { @@ -7,19 +8,21 @@ describe('content_editor/services/gl_api_markdown_deserializer', () => { let doc; let p; let bold; + let comment; let tiptapEditor; beforeEach(() => { tiptapEditor = createTestEditor({ - extensions: [Bold], + extensions: [Bold, Comment], }); ({ - builders: { doc, p, bold }, + builders: { doc, p, bold, comment }, } = createDocBuilder({ tiptapEditor, names: { bold: { markType: Bold.name }, + comment: { nodeType: Comment.name }, }, })); renderMarkdown = jest.fn(); @@ -33,7 +36,7 @@ describe('content_editor/services/gl_api_markdown_deserializer', () => { const deserializer = createMarkdownDeserializer({ render: renderMarkdown }); renderMarkdown.mockResolvedValueOnce( - `<p><strong>${text}</strong></p><pre lang="javascript"></pre>`, + `<p><strong>${text}</strong></p><pre lang="javascript"></pre><!-- some comment -->`, ); result = await deserializer.deserialize({ @@ -41,8 +44,9 @@ describe('content_editor/services/gl_api_markdown_deserializer', () => { schema: tiptapEditor.schema, }); }); + it('transforms HTML returned by render function to a ProseMirror document', async () => { - const document = doc(p(bold(text))); + const document = doc(p(bold(text)), comment(' some comment ')); expect(result.document.toJSON()).toEqual(document.toJSON()); }); diff --git a/spec/frontend/content_editor/services/markdown_serializer_spec.js b/spec/frontend/content_editor/services/markdown_serializer_spec.js index 1bf2341505210..a29678ff1bcf4 100644 --- a/spec/frontend/content_editor/services/markdown_serializer_spec.js +++ b/spec/frontend/content_editor/services/markdown_serializer_spec.js @@ -3,6 +3,7 @@ import Bold from '~/content_editor/extensions/bold'; import BulletList from '~/content_editor/extensions/bullet_list'; import Code from '~/content_editor/extensions/code'; import CodeBlockHighlight from '~/content_editor/extensions/code_block_highlight'; +import Comment from '~/content_editor/extensions/comment'; import DescriptionItem from '~/content_editor/extensions/description_item'; import DescriptionList from '~/content_editor/extensions/description_list'; import Details from '~/content_editor/extensions/details'; @@ -50,6 +51,7 @@ const { bulletList, code, codeBlock, + comment, details, detailsContent, div, @@ -89,6 +91,7 @@ const { bulletList: { nodeType: BulletList.name }, code: { markType: Code.name }, codeBlock: { nodeType: CodeBlockHighlight.name }, + comment: { nodeType: Comment.name }, details: { nodeType: Details.name }, detailsContent: { nodeType: DetailsContent.name }, descriptionItem: { nodeType: DescriptionItem.name }, @@ -169,6 +172,17 @@ describe('markdownSerializer', () => { ); }); + it('correctly serializes a comment node', () => { + expect(serialize(paragraph('hi'), comment(' this is a\ncomment '))).toBe( + ` +hi + +<!-- this is a +comment --> + `.trim(), + ); + }); + it('correctly serializes a line break', () => { expect(serialize(paragraph('hello', hardBreak(), 'world'))).toBe('hello\\\nworld'); }); diff --git a/spec/frontend/content_editor/test_utils.js b/spec/frontend/content_editor/test_utils.js index 46240c4ac9488..0fa0e65cd2635 100644 --- a/spec/frontend/content_editor/test_utils.js +++ b/spec/frontend/content_editor/test_utils.js @@ -11,6 +11,7 @@ import Bold from '~/content_editor/extensions/bold'; import BulletList from '~/content_editor/extensions/bullet_list'; import Code from '~/content_editor/extensions/code'; import CodeBlockHighlight from '~/content_editor/extensions/code_block_highlight'; +import Comment from '~/content_editor/extensions/comment'; import DescriptionItem from '~/content_editor/extensions/description_item'; import DescriptionList from '~/content_editor/extensions/description_list'; import Details from '~/content_editor/extensions/details'; @@ -212,6 +213,7 @@ export const createTiptapEditor = (extensions = []) => BulletList, Code, CodeBlockHighlight, + Comment, DescriptionItem, DescriptionList, Details, diff --git a/spec/frontend/lib/utils/dom_utils_spec.js b/spec/frontend/lib/utils/dom_utils_spec.js index d6bac93597052..172f8972653a2 100644 --- a/spec/frontend/lib/utils/dom_utils_spec.js +++ b/spec/frontend/lib/utils/dom_utils_spec.js @@ -10,6 +10,7 @@ import { getParents, getParentByTagName, setAttributes, + replaceCommentsWith, } from '~/lib/utils/dom_utils'; const TEST_MARGIN = 5; @@ -263,4 +264,21 @@ describe('DOM Utils', () => { expect(getContentWrapperHeight('.does-not-exist')).toBe(''); }); }); + + describe('replaceCommentsWith', () => { + let div; + beforeEach(() => { + div = document.createElement('div'); + }); + + it('replaces the comments in a DOM node with an element', () => { + div.innerHTML = '<h1> hi there <!-- some comment --> <p> <!-- another comment -->'; + + replaceCommentsWith(div, 'comment'); + + expect(div.innerHTML).toBe( + '<h1> hi there <comment> some comment </comment> <p> <comment> another comment </comment></p></h1>', + ); + }); + }); }); -- GitLab