Merge pull request #103 from threema-ch/issue-86

Sanitizing text would cause some non-HTML text to disappear (see #86). Instead, we escape HTML, so that it looks exactly like pasted.

Fixes #86.

src/directives/compose_area.ts

@@ -26,7 +26,6 @@ export default [
     '$translate',
     '$mdDialog',
     '$filter',
-    '$sanitize',
     '$log',
     function(browserService: threema.BrowserService,
              stringService: threema.StringService,
@@ -34,7 +33,6 @@ export default [
              $translate: ng.translate.ITranslateService,
              $mdDialog: ng.material.IDialogService,
              $filter: ng.IFilterService,
-             $sanitize: ng.sanitize.ISanitizeService,
              $log: ng.ILogService) {
         return {
             restrict: 'EA',
@@ -279,12 +277,6 @@ export default [
                     });
                 }
 
-                function applyFilters(text: string): string {
-                    const emojify = $filter('emojify') as (a: string, b?: boolean) => string;
-                    const parseNewLine = $filter('nlToBr') as (a: string, b?: boolean) => string;
-                    return parseNewLine(emojify(text, true), true);
-                }
-
                 // Handle pasting
                 function onPaste(ev: ClipboardEvent) {
                     ev.preventDefault();
@@ -348,17 +340,19 @@ export default [
                     } else if (textIdx !== null) {
                         const text = ev.clipboardData.getData('text/plain');
 
-                        // Apply filters (emojify, convert newline, etc)
-                        const formatted = applyFilters(text);
+                        // Look up some filter functions
+                        const escapeHtml = $filter('escapeHtml') as (a: string) => string;
+                        const emojify = $filter('emojify') as (a: string, b?: boolean) => string;
+                        const nlToBr = $filter('nlToBr') as (a: string, b?: boolean) => string;
 
-                        // Replace HTML formatting with ASCII counterparts
-                        const htmlToAsciiMarkup = $filter('htmlToAsciiMarkup') as (a: string) => string;
+                        // Escape HTML markup
+                        const escaped = escapeHtml(text);
 
-                        // Sanitize
-                        const sanitized = $sanitize(htmlToAsciiMarkup(formatted));
+                        // Apply filters (emojify, convert newline, etc)
+                        const formatted = nlToBr(emojify(escaped, true), true);
 
-                        // Insert HTML
-                        document.execCommand('insertHTML', false, sanitized);
+                        // Insert resulting HTML
+                        document.execCommand('insertHTML', false, formatted);
 
                         cleanupComposeContent();
                         updateView();
@@ -421,7 +415,7 @@ export default [
                 function onEmojiChosen(ev: MouseEvent): void {
                     ev.stopPropagation();
                     const emoji = this.textContent; // Unicode character
-                    const formatted = applyFilters(emoji);
+                    const formatted = ($filter('emojify') as any)(emoji, true);
 
                     // Firefox inserts a <br> after editing content editable fields.
                     // Remove the last <br> to fix this.

src/filters.ts

@@ -30,7 +30,12 @@ angular.module('3ema.filters', [])
         '"': '"',
         "'": ''',
     };
-    return (text) => (text !== undefined && text !== null ? text : '').replace(/[&<>"']/g, (m) => map[m]);
+    return (text: string) => {
+        if (text === undefined || text === null) {
+            text = '';
+        }
+        return text.replace(/[&<>"']/g, (m) => map[m]);
+    };
 })
 
 /**
@@ -45,29 +50,6 @@ angular.module('3ema.filters', [])
     };
 })
 
-/**
- * Replace formatting HTML with ASCII alternatives.
- */
-.filter('htmlToAsciiMarkup', function() {
-    return (text) => {
-        let tags = [
-            ['b', '*'],
-            ['strong', '*'],
-            ['i', '_'],
-            ['em', '_'],
-            ['strike', '~'],
-            ['del', '~'],
-            ['s', '~'],
-        ];
-        let out = text;
-        for (let tag of tags) {
-            out = out.replace(new RegExp('<\\s*' + tag[0] + '(\\s[^>]*|\\s*)>', 'gi'), tag[1]);
-            out = out.replace(new RegExp('<\\s*\/\\s*' + tag[0] + '\\s*>', 'gi'), tag[1]);
-        }
-        return out;
-    };
-})
-
 /**
  * Convert links in text to <a> tags.
  */

tests/filters.js

@@ -16,16 +16,18 @@ describe('Filters', function() {
 
     });
 
+    function testPatterns(filterName, cases) {
+        const filter = $filter(filterName);
+        for (let testcase of cases) {
+            const input = testcase[0];
+            const expected = testcase[1];
+            expect(filter(input)).toEqual(expected);
+        };
+    };
+
     describe('markify', function() {
 
-        this.testPatterns = (cases) => {
-            const filter = $filter('markify');
-            for (let testcase of cases) {
-                const input = testcase[0];
-                const expected = testcase[1];
-                expect(filter(input)).toEqual(expected);
-            };
-        };
+        this.testPatterns = (cases) => testPatterns('markify', cases);
 
         it('detects bold text', () => {
             this.testPatterns([
@@ -96,59 +98,18 @@ describe('Filters', function() {
 
     });
 
-    describe('htmlToAsciiMarkup', function() {
+    describe('escapeHtml', function() {
 
-        this.testPatterns = (cases) => {
-            const filter = $filter('htmlToAsciiMarkup');
-            for (let testcase of cases) {
-                const input = testcase[0];
-                const expected = testcase[1];
-                expect(filter(input)).toEqual(expected);
-            };
-        };
+        this.testPatterns = (cases) => testPatterns('escapeHtml', cases);
 
-        it('converts bold text', () => {
+        it('escapes html tags', () => {
             this.testPatterns([
-                ['<b>bold</b>', '*bold*'],
-                ['< 	b >bold</b>', '*bold*'],
-                ['<B>bold</b>', '*bold*'],
-                ['<b class="asdf">bold</b>', '*bold*'],
-                ['<strong>bold</strong>', '*bold*'],
-                ['<b><b>bold</b></b>', '**bold**'],
-                ['<b><strong>bold</strong></b>', '**bold**'],
-            ]);
-        });
-
-        it('converts italic text', () => {
-            this.testPatterns([
-                ['<i>italic</i>', '_italic_'],
-                ['<i onclick="alert(1)">italic</i>', '_italic_'],
-                ['<em>italic</em>', '_italic_'],
-                ['<i><em>italic</em></i>', '__italic__'],
-            ]);
-        });
-
-        it('converts strikethrough text', () => {
-            this.testPatterns([
-                ['<strike>strikethrough</strike>', '~strikethrough~'],
-                ['<del>strikethrough</del>', '~strikethrough~'],
-                ['<del href="/">strikethrough</del>', '~strikethrough~'],
-                ['<s>strikethrough</s>', '~strikethrough~'],
-                ['<strike><del><s>strikethrough</s></del></strike>', '~~~strikethrough~~~'],
-            ]);
-        });
-
-        it('does not affect other tags', () => {
-            this.testPatterns([
-                ['<script>alert("pho soup time")</script>', '<script>alert("pho soup time")</script>'],
-            ]);
-        });
-
-        it('combination of all', () => {
-            this.testPatterns([
-                ['<b><em><del>foo</del></em></b>', '*_~foo~_*'],
+                ['<h1>heading</h1>', '&lt;h1&gt;heading&lt;/h1&gt;'],
+                ['<b>< script >foo&ndash;</b>< script>', '&lt;b&gt;&lt; script &gt;foo&amp;ndash;&lt;/b&gt;&lt; script&gt;'],
+                ['<a href="/">a</a>', '&lt;a href=&quot;/&quot;&gt;a&lt;/a&gt;'],
             ]);
         });
 
     });
+
 });