Merge pull request #622 from kyrias/commonmark-fix-escaping

Fix escaping markdown by rendering plaintext
matrix-org · Jan 19, 2017 · 89fa47d · 89fa47d
2 parents 9e0c7a1 + 9c1c657
commit 89fa47d
Show file tree

Hide file tree

Showing 4 changed files with 134 additions and 8 deletions.
diff --git a/src/Markdown.js b/src/Markdown.js
@@ -23,7 +23,9 @@ import commonmark from 'commonmark';
  */
 export default class Markdown {
     constructor(input) {
-        this.input = input
+        this.input = input;
+        this.parser = new commonmark.Parser();
+        this.renderer = new commonmark.HtmlRenderer({safe: false});
     }
 
     isPlainText() {
@@ -48,6 +50,7 @@ export default class Markdown {
         }
         // text and paragraph are just text
         dummy_renderer.text = function(t) { return t; }
+        dummy_renderer.softbreak = function(t) { return t; }
         dummy_renderer.paragraph = function(t) { return t; }
 
         const dummy_parser = new commonmark.Parser();
@@ -57,11 +60,9 @@ export default class Markdown {
     }
 
     toHTML() {
-        const parser = new commonmark.Parser();
+        const real_paragraph = this.renderer.paragraph;
 
-        const renderer = new commonmark.HtmlRenderer({safe: true});
-        const real_paragraph = renderer.paragraph;
-        renderer.paragraph = function(node, entering) {
+        this.renderer.paragraph = function(node, entering) {
             // If there is only one top level node, just return the
             // bare text: it's a single line of text and so should be
             // 'inline', rather than unnecessarily wrapped in its own
@@ -76,7 +77,48 @@ export default class Markdown {
             }
         }
 
-        var parsed = parser.parse(this.input);
-        return renderer.render(parsed);
+        var parsed = this.parser.parse(this.input);
+        var rendered = this.renderer.render(parsed);
+
+        this.renderer.paragraph = real_paragraph;
+
+        return rendered;
+    }
+
+    toPlaintext() {
+        const real_paragraph = this.renderer.paragraph;
+
+        // The default `out` function only sends the input through an XML
+        // escaping function, which causes messages to be entity encoded,
+        // which we don't want in this case.
+        this.renderer.out = function(s) {
+            // The `lit` function adds a string literal to the output buffer.
+            this.lit(s);
+        }
+
+        this.renderer.paragraph = function(node, entering) {
+            // If there is only one top level node, just return the
+            // bare text: it's a single line of text and so should be
+            // 'inline', rather than unnecessarily wrapped in its own
+            // p tag. If, however, we have multiple nodes, each gets
+            // its own p tag to keep them as separate paragraphs.
+            var par = node;
+            while (par.parent) {
+                node = par;
+                par = par.parent;
+            }
+            if (node != par.lastChild) {
+                if (!entering) {
+                    this.lit('\n\n');
+                }
+            }
+        }
+
+        var parsed = this.parser.parse(this.input);
+        var rendered = this.renderer.render(parsed);
+
+        this.renderer.paragraph = real_paragraph;
+
+        return rendered;
     }
 }
diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js
@@ -523,7 +523,9 @@ export default class MessageComposerInput extends React.Component {
             );
         } else {
             const md = new Markdown(contentText);
-            if (!md.isPlainText()) {
+            if (md.isPlainText()) {
+                contentText = md.toPlaintext();
+            } else {
                 contentHTML = md.toHTML();
             }
         }

diff --git a/src/components/views/rooms/MessageComposerInputOld.js b/src/components/views/rooms/MessageComposerInputOld.js
@@ -331,6 +331,7 @@ module.exports = React.createClass({
                 MatrixClientPeg.get().sendHtmlMessage(this.props.room.roomId, contentText, htmlText);
         }
         else {
+            const contentText = mdown.toPlaintext();
             sendMessagePromise = isEmote ?
                 MatrixClientPeg.get().sendEmoteMessage(this.props.room.roomId, contentText) :
                 MatrixClientPeg.get().sendTextMessage(this.props.room.roomId, contentText);

diff --git a/test/components/views/rooms/MessageComposerInput-test.js b/test/components/views/rooms/MessageComposerInput-test.js
@@ -158,4 +158,85 @@ describe('MessageComposerInput', () => {
         expect(['__', '**']).toContain(spy.args[0][1]);
     });
 
+    it('should not entity-encode " in Markdown mode', () => {
+        const spy = sinon.spy(client, 'sendTextMessage');
+        mci.enableRichtext(false);
+        addTextToDraft('"');
+        mci.handleReturn(sinon.stub());
+
+        expect(spy.calledOnce).toEqual(true);
+        expect(spy.args[0][1]).toEqual('"');
+    });
+
+    it('should escape characters without other markup in Markdown mode', () => {
+        const spy = sinon.spy(client, 'sendTextMessage');
+        mci.enableRichtext(false);
+        addTextToDraft('\\*escaped\\*');
+        mci.handleReturn(sinon.stub());
+
+        expect(spy.calledOnce).toEqual(true);
+        expect(spy.args[0][1]).toEqual('*escaped*');
+    });
+
+    it('should escape characters with other markup in Markdown mode', () => {
+        const spy = sinon.spy(client, 'sendHtmlMessage');
+        mci.enableRichtext(false);
+        addTextToDraft('\\*escaped\\* *italic*');
+        mci.handleReturn(sinon.stub());
+
+        expect(spy.calledOnce).toEqual(true);
+        expect(spy.args[0][1]).toEqual('\\*escaped\\* *italic*');
+        expect(spy.args[0][2]).toEqual('*escaped* <em>italic</em>');
+    });
+
+    it('should not convert -_- into a horizontal rule in Markdown mode', () => {
+        const spy = sinon.spy(client, 'sendTextMessage');
+        mci.enableRichtext(false);
+        addTextToDraft('-_-');
+        mci.handleReturn(sinon.stub());
+
+        expect(spy.calledOnce).toEqual(true);
+        expect(spy.args[0][1]).toEqual('-_-');
+    });
+
+    it('should not strip <del> tags in Markdown mode', () => {
+        const spy = sinon.spy(client, 'sendHtmlMessage');
+        mci.enableRichtext(false);
+        addTextToDraft('<del>striked-out</del>');
+        mci.handleReturn(sinon.stub());
+
+        expect(spy.calledOnce).toEqual(true);
+        expect(spy.args[0][1]).toEqual('<del>striked-out</del>');
+        expect(spy.args[0][2]).toEqual('<del>striked-out</del>');
+    });
+
+    it('should not strike-through ~~~ in Markdown mode', () => {
+        const spy = sinon.spy(client, 'sendTextMessage');
+        mci.enableRichtext(false);
+        addTextToDraft('~~~striked-out~~~');
+        mci.handleReturn(sinon.stub());
+
+        expect(spy.calledOnce).toEqual(true);
+        expect(spy.args[0][1]).toEqual('~~~striked-out~~~');
+    });
+
+    it('should not mark single unmarkedup paragraphs as HTML in Markdown mode', () => {
+        const spy = sinon.spy(client, 'sendTextMessage');
+        mci.enableRichtext(false);
+        addTextToDraft('Lorem ipsum dolor sit amet, consectetur adipiscing elit.');
+        mci.handleReturn(sinon.stub());
+
+        expect(spy.calledOnce).toEqual(true);
+        expect(spy.args[0][1]).toEqual('Lorem ipsum dolor sit amet, consectetur adipiscing elit.');
+    });
+
+    it('should not mark two unmarkedup paragraphs as HTML in Markdown mode', () => {
+        const spy = sinon.spy(client, 'sendTextMessage');
+        mci.enableRichtext(false);
+        addTextToDraft('Lorem ipsum dolor sit amet, consectetur adipiscing elit.\n\nFusce congue sapien sed neque molestie volutpat.');
+        mci.handleReturn(sinon.stub());
+
+        expect(spy.calledOnce).toEqual(true);
+        expect(spy.args[0][1]).toEqual('Lorem ipsum dolor sit amet, consectetur adipiscing elit.\n\nFusce congue sapien sed neque molestie volutpat.');
+    });
 });