Escape HTML in markdown. (#955)

* Escape HTML in markdown. * Escape variables for message history. * Rename escape method.
Squidex · Dec 13, 2022 · cf4efc5 · cf4efc5
1 parent 16fc031
commit cf4efc5
Show file tree

Hide file tree

Showing 23 changed files with 315 additions and 113 deletions.
diff --git a/frontend/src/app/declarations.d.ts b/frontend/src/app/declarations.d.ts
@@ -18,3 +18,7 @@ declare module 'sortablejs' {
 
     export function create(element: any, options: any): Ref;
 }
+
+declare namespace marked {
+    export function escape(input: string): string;
+}
diff --git a/frontend/src/app/features/rules/pages/rule/rule-page.component.ts b/frontend/src/app/features/rules/pages/rule/rule-page.component.ts
@@ -10,7 +10,7 @@ import { AbstractControl } from '@angular/forms';
 import { ActivatedRoute, Router } from '@angular/router';
 import { debounceTime, Subscription } from 'rxjs';
 import { ActionForm, ALL_TRIGGERS, MessageBus, ResourceOwner, RuleDto, RuleElementDto, RulesService, RulesState, SchemasState, TriggerForm, value$ } from '@app/shared';
-import { RuleConfigured } from '../messages';
+import { RuleConfigured } from './../messages';
 
 @Component({
     selector: 'sqx-rule-page',

diff --git a/frontend/src/app/features/rules/pages/simulator/rule-simulator-page.component.ts b/frontend/src/app/features/rules/pages/simulator/rule-simulator-page.component.ts
@@ -8,7 +8,7 @@
 import { Component, OnInit } from '@angular/core';
 import { ActivatedRoute } from '@angular/router';
 import { MessageBus, ResourceOwner, RuleSimulatorState, SimulatedRuleEventDto } from '@app/shared';
-import { RuleConfigured } from '../messages';
+import { RuleConfigured } from './../messages';
 
 @Component({
     selector: 'sqx-simulator-events-page',

diff --git a/frontend/src/app/features/rules/shared/actions/generic-action.component.html b/frontend/src/app/features/rules/shared/actions/generic-action.component.html
@@ -47,7 +47,7 @@
             </ng-container>
 
             <sqx-form-hint>
-                <span [sqxMarkdown]="property.description" [inline]="true" [html]="true"></span>
+                <span [sqxMarkdown]="property.description" [inline]="true"></span>
 
                 <div *ngIf="property.isFormattable">
                     {{ 'rules.advancedFormattingHint' | sqxTranslate }}: <a tabindex="-1" href="https://docs.squidex.io/concepts/rules#3-formatting" sqxExternalLink>{{ 'common.documentation' | sqxTranslate }}</a>

diff --git a/frontend/src/app/framework/angular/markdown.directive.spec.ts b/frontend/src/app/framework/angular/markdown.directive.spec.ts
@@ -0,0 +1,89 @@
+/*
+ * Squidex Headless CMS
+ *
+ * @license
+ * Copyright (c) Squidex UG (haftungsbeschränkt). All rights reserved.
+ */
+
+import { Renderer2 } from '@angular/core';
+import { IMock, It, Mock, Times } from 'typemoq';
+import { MarkdownDirective } from './markdown.directive';
+
+describe('MarkdownDirective', () => {
+    let renderer: IMock<Renderer2>;
+    let markdownElement = {};
+    let markdownDirective: MarkdownDirective;
+
+    beforeEach(() => {
+        renderer = Mock.ofType<Renderer2>();
+
+        markdownElement = {};
+        markdownDirective = new MarkdownDirective(markdownElement as any, renderer.object);
+    });
+
+    it('should render empty text as text', () => {
+        markdownDirective.markdown = '';
+        markdownDirective.ngOnChanges();
+
+        verifyTextRender('');
+    });
+
+    it('should render as text if result has no tags', () => {
+        markdownDirective.inline = true;
+        markdownDirective.markdown = 'markdown';
+        markdownDirective.ngOnChanges();
+
+        verifyTextRender('markdown');
+    });
+
+    it('should render as text if optional', () => {
+        markdownDirective.optional = true;
+        markdownDirective.markdown = '**bold**';
+        markdownDirective.ngOnChanges();
+
+        verifyTextRender('**bold**');
+    });
+
+    it('should render if optional with exclamation', () => {
+        markdownDirective.optional = true;
+        markdownDirective.markdown = '!**bold**';
+        markdownDirective.ngOnChanges();
+
+        verifyHtmlRender('<strong>bold</strong>');
+    });
+
+    it('should render as HTML if allowed', () => {
+        markdownDirective.inline = false;
+        markdownDirective.markdown = '**bold**';
+        markdownDirective.ngOnChanges();
+
+        verifyHtmlRender('<p><strong>bold</strong></p>\n');
+    });
+
+    it('should render as inline HTML if allowed', () => {
+        markdownDirective.markdown = '!**bold**';
+        markdownDirective.ngOnChanges();
+
+        verifyHtmlRender('<strong>bold</strong>');
+    });
+
+    it('should render HTML escaped', () => {
+        markdownDirective.inline = false;
+        markdownDirective.markdown = '<h1>Header</h1>';
+        markdownDirective.ngOnChanges();
+
+        verifyHtmlRender('<p>&lt;h1&gt;Header&lt;/h1&gt;</p>\n');
+    });
+
+    function verifyTextRender(text: string) {
+        renderer.verify(x => x.setProperty(It.isAny(), 'textContent', text), Times.once());
+
+        expect().nothing();
+    }
+
+    function verifyHtmlRender(text: string) {
+        renderer.verify(x => x.setProperty(It.isAny(), 'innerHTML', text), Times.once());
+
+        expect().nothing();
+    }
+});
diff --git a/frontend/src/app/framework/angular/markdown.directive.ts b/frontend/src/app/framework/angular/markdown.directive.ts
@@ -6,24 +6,7 @@
  */
 
 import { Directive, ElementRef, Input, OnChanges, Renderer2 } from '@angular/core';
-import { marked } from 'marked';
-
-const RENDERER_DEFAULT = new marked.Renderer();
-const RENDERER_INLINE = new marked.Renderer();
-
-RENDERER_DEFAULT.link = (href, _, text) => {
-    if (href && href.startsWith('mailto')) {
-        return text;
-    } else {
-        return `<a href="${href}" target="_blank", rel="noopener">${text} <i class="icon-external-link"></i></a>`;
-    }
-};
-
-RENDERER_INLINE.paragraph = (text) => {
-    return text;
-};
-
-RENDERER_INLINE.link = RENDERER_DEFAULT.link;
+import { renderMarkdown } from '@app/framework/internal';
 
 @Directive({
     selector: '[sqxMarkdown]',
@@ -35,9 +18,6 @@ export class MarkdownDirective implements OnChanges {
     @Input()
     public inline = true;
 
-    @Input()
-    public html = false;
-
     @Input()
     public optional = false;
 
@@ -50,22 +30,28 @@ export class MarkdownDirective implements OnChanges {
     public ngOnChanges() {
         let html = '';
 
-        const markdown = this.markdown;
+        let markdown = this.markdown;
+
+        const hasExclamation = markdown.indexOf('!') === 0;
+
+        if (hasExclamation) {
+            markdown = markdown.substring(1);
+        }
 
         if (!markdown) {
             html = markdown;
-        } else if (this.optional && markdown.indexOf('!') !== 0) {
+        } else if (this.optional && !hasExclamation) {
             html = markdown;
         } else if (this.markdown) {
-            const renderer = this.inline ? RENDERER_INLINE : RENDERER_DEFAULT;
-
-            html = marked(this.markdown, { renderer });
+            html = renderMarkdown(markdown, this.inline);
         }
 
-        if (!this.html && (!html || html === this.markdown || html.indexOf('<') < 0)) {
-            this.renderer.setProperty(this.element.nativeElement, 'textContent', html);
-        } else {
+        const hasHtml = html.indexOf('<') >= 0;
+
+        if (hasHtml) {
             this.renderer.setProperty(this.element.nativeElement, 'innerHTML', html);
+        } else {
+            this.renderer.setProperty(this.element.nativeElement, 'textContent', html);
         }
     }
 }
diff --git a/frontend/src/app/framework/angular/pager.component.ts b/frontend/src/app/framework/angular/pager.component.ts
@@ -6,7 +6,7 @@
  */
 
 import { ChangeDetectionStrategy, Component, EventEmitter, Input, OnChanges, Output } from '@angular/core';
-import { PagingInfo } from '../state';
+import { PagingInfo } from './../state';
 
 export const PAGE_SIZES: ReadonlyArray<number> = [10, 20, 30, 50];
 

diff --git a/frontend/src/app/framework/angular/pipes/markdown.pipe.spec.ts b/frontend/src/app/framework/angular/pipes/markdown.pipe.spec.ts
@@ -8,43 +8,59 @@
 import { MarkdownInlinePipe, MarkdownPipe } from './markdown.pipe';
 
 describe('MarkdownInlinePipe', () => {
+    const pipe = new MarkdownInlinePipe();
+
     it('should convert link to html', () => {
-        const actual = new MarkdownInlinePipe().transform('[link-name](link-url)');
+        const actual = pipe.transform('[link-name](link-url)');
 
         expect(actual).toBe('<a href="link-url" target="_blank", rel="noopener">link-name <i class="icon-external-link"></i></a>');
     });
 
     it('should convert markdown to html', () => {
-        const actual = new MarkdownInlinePipe().transform('*bold*');
+        const actual = pipe.transform('*bold*');
 
         expect(actual).toBe('<em>bold</em>');
     });
 
+    it('should escape input html', () => {
+        const actual = pipe.transform('<h1>Header</h1>');
+
+        expect(actual).toBe('&lt;h1&gt;Header&lt;/h1&gt;');
+    });
+
     [null, undefined, ''].forEach(x => {
         it('should return empty string for invalid value', () => {
-            const actual = new MarkdownInlinePipe().transform(x);
+            const actual = pipe.transform(x);
 
             expect(actual).toBe('');
         });
     });
 });
 
 describe('MarkdownPipe', () => {
+    const pipe = new MarkdownPipe();
+
     it('should convert link to html', () => {
-        const actual = new MarkdownPipe().transform('[link-name](link-url)');
+        const actual = pipe.transform('[link-name](link-url)');
 
         expect(actual).toBe('<p><a href="link-url" target="_blank", rel="noopener">link-name <i class="icon-external-link"></i></a></p>\n');
     });
 
     it('should convert markdown to html', () => {
-        const actual = new MarkdownPipe().transform('*bold*');
+        const actual = pipe.transform('*bold*');
 
         expect(actual).toBe('<p><em>bold</em></p>\n');
     });
 
+    it('should escape input html', () => {
+        const actual = pipe.transform('<h1>Header</h1>');
+
+        expect(actual).toBe('<p>&lt;h1&gt;Header&lt;/h1&gt;</p>\n');
+    });
+
     [null, undefined, ''].forEach(x => {
         it('should return empty string for invalid value', () => {
-            const actual = new MarkdownPipe().transform(x);
+            const actual = pipe.transform(x);
 
             expect(actual).toBe('');
         });

diff --git a/frontend/src/app/framework/angular/pipes/markdown.pipe.ts b/frontend/src/app/framework/angular/pipes/markdown.pipe.ts
@@ -6,37 +6,15 @@
  */
 
 import { Pipe, PipeTransform } from '@angular/core';
-import { marked } from 'marked';
-
-const renderer = new marked.Renderer();
-
-renderer.link = (href, _, text) => {
-    if (href && href.startsWith('mailto')) {
-        return text;
-    } else {
-        return `<a href="${href}" target="_blank", rel="noopener">${text} <i class="icon-external-link"></i></a>`;
-    }
-};
-
-const inlinerRenderer = new marked.Renderer();
-
-inlinerRenderer.paragraph = (text) => {
-    return text;
-};
-
-inlinerRenderer.link = renderer.link;
+import { renderMarkdown } from '@app/framework/internal';
 
 @Pipe({
     name: 'sqxMarkdown',
     pure: true,
 })
 export class MarkdownPipe implements PipeTransform {
     public transform(text: string | undefined | null): string {
-        if (text) {
-            return marked(text, { renderer });
-        } else {
-            return '';
-        }
+        return renderMarkdown(text, false);
     }
 }
 
@@ -46,10 +24,6 @@ export class MarkdownPipe implements PipeTransform {
 })
 export class MarkdownInlinePipe implements PipeTransform {
     public transform(text: string | undefined | null): string {
-        if (text) {
-            return marked(text, { renderer: inlinerRenderer });
-        } else {
-            return '';
-        }
+        return renderMarkdown(text, true);
     }
-}
+}
diff --git a/frontend/src/app/framework/internal.ts b/frontend/src/app/framework/internal.ts
@@ -33,6 +33,7 @@ export * from './utils/hateos';
 export * from './utils/interpolator';
 export * from './utils/keys';
 export * from './utils/math-helper';
+export * from './utils/markdown';
 export * from './utils/modal-positioner';
 export * from './utils/modal-view';
 export * from './utils/picasso';

diff --git a/frontend/src/app/framework/services/localizer.service.ts b/frontend/src/app/framework/services/localizer.service.ts
@@ -6,7 +6,7 @@
  */
 
 import { Injectable } from '@angular/core';
-import { compareStrings } from '../utils/array-helper';
+import { compareStrings } from './../utils/array-helper';
 
 @Injectable()
 export class LocalizerService {

diff --git a/frontend/src/app/framework/utils/date-time.spec.ts b/frontend/src/app/framework/utils/date-time.spec.ts
@@ -5,7 +5,7 @@
  * Copyright (c) Squidex UG (haftungsbeschränkt). All rights reserved.
  */
 
-import { DateHelper } from '..';
+import { DateHelper } from './..';
 import { DateTime } from './date-time';
 
 describe('DateTime', () => {