From 67b16f7ea3a616875b45d07215197b7e4a08eba4 Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Wed, 17 Apr 2019 12:52:25 +0100
Subject: Only use backslash escapes in autocomplete when needed

Autocompletion for references happens on the frontend. Those references
are turned into actual references on the backend, but only after
Markdown processing has happened. That means that if a reference
contains a character that Markdown might consume, it won't render
correctly. So we need to do some escaping on the frontend.

We have these potential problem characters:
https://docs.gitlab.com/ee/user/markdown.html#emphasis

1. ~ - this is ~~strikethrough~~, but only when doubled.
2. _ - used for _emphasis_, doubled is __bold__.
3. * - also used for *emphasis*, doubled is **bold** also.
4. ` - used for `code spans`, any number works.

We don't need to escape `-` any more. When it comes to being inside a
word:

1. a~~b~~ has strikethrough, so it needs to be escaped everywhere.
2. a_b_ has no emphasis (see [a]) so it only needs to be escaped at the
   start and end of words.
3. a*b* has emphasis, so it needs to be escaped everywhere.
4. a`b` has a code span, so it needs to be escaped everywhere.

Or, in code terms:

1. Always escape ~~, *, and ` when being inserted by autocomplete.
2. Escape _ when it's either at the beginning or the end of a word.

[a]: https://docs.gitlab.com/ee/user/markdown.html#multiple-underscores-in-words
---
 app/assets/javascripts/gfm_auto_complete.js         |  5 ++++-
 .../unreleased/markdown-autocomplete-escaping.yml   |  5 +++++
 spec/frontend/gfm_auto_complete_spec.js             | 21 +++++++++++++++------
 3 files changed, 24 insertions(+), 7 deletions(-)
 create mode 100644 changelogs/unreleased/markdown-autocomplete-escaping.yml

diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js
index c81e754df4c..f1e26cdfa21 100644
--- a/app/assets/javascripts/gfm_auto_complete.js
+++ b/app/assets/javascripts/gfm_auto_complete.js
@@ -461,7 +461,10 @@ class GfmAutoComplete {
         // We can ignore this for quick actions because they are processed
         // before Markdown.
         if (!this.setting.skipMarkdownCharacterTest) {
-          withoutAt = withoutAt.replace(/([~\-_*`])/g, '\\$&');
+          withoutAt = withoutAt
+            .replace(/(~~|`|\*)/g, '\\$1')
+            .replace(/(\b)(_+)/g, '$1\\$2') // only escape underscores at the start
+            .replace(/(_+)(\b)/g, '\\$1$2'); // or end of words
         }
 
         return `${at}${withoutAt}`;
diff --git a/changelogs/unreleased/markdown-autocomplete-escaping.yml b/changelogs/unreleased/markdown-autocomplete-escaping.yml
new file mode 100644
index 00000000000..0ea034b14ee
--- /dev/null
+++ b/changelogs/unreleased/markdown-autocomplete-escaping.yml
@@ -0,0 +1,5 @@
+---
+title: Only escape Markdown emphasis characters in autocomplete when necessary
+merge_request: 27457
+author:
+type: changed
diff --git a/spec/frontend/gfm_auto_complete_spec.js b/spec/frontend/gfm_auto_complete_spec.js
index ed12af925f1..841aff0d7ff 100644
--- a/spec/frontend/gfm_auto_complete_spec.js
+++ b/spec/frontend/gfm_auto_complete_spec.js
@@ -94,7 +94,7 @@ describe('GfmAutoComplete', () => {
     });
 
     it('should quote if value contains any non-alphanumeric characters', () => {
-      expect(beforeInsert(atwhoInstance, '~label-20')).toBe('~"label\\-20"');
+      expect(beforeInsert(atwhoInstance, '~label-20')).toBe('~"label-20"');
       expect(beforeInsert(atwhoInstance, '~label 20')).toBe('~"label 20"');
     });
 
@@ -102,12 +102,21 @@ describe('GfmAutoComplete', () => {
       expect(beforeInsert(atwhoInstance, '~1234')).toBe('~"1234"');
     });
 
-    it('should escape Markdown emphasis characters, except in the first character', () => {
-      expect(beforeInsert(atwhoInstance, '@_group')).toEqual('@\\_group');
-      expect(beforeInsert(atwhoInstance, '~_bug')).toEqual('~\\_bug');
+    it('escapes Markdown strikethroughs when needed', () => {
+      expect(beforeInsert(atwhoInstance, '~a~bug')).toEqual('~"a~bug"');
+      expect(beforeInsert(atwhoInstance, '~a~~bug~~')).toEqual('~"a\\~~bug\\~~"');
+    });
+
+    it('escapes Markdown emphasis when needed', () => {
+      expect(beforeInsert(atwhoInstance, '~a_bug_')).toEqual('~a_bug\\_');
+      expect(beforeInsert(atwhoInstance, '~a _bug_')).toEqual('~"a \\_bug\\_"');
+      expect(beforeInsert(atwhoInstance, '~a*bug*')).toEqual('~"a\\*bug\\*"');
+      expect(beforeInsert(atwhoInstance, '~a *bug*')).toEqual('~"a \\*bug\\*"');
+    });
+
+    it('escapes Markdown code spans when needed', () => {
+      expect(beforeInsert(atwhoInstance, '~a`bug`')).toEqual('~"a\\`bug\\`"');
       expect(beforeInsert(atwhoInstance, '~a `bug`')).toEqual('~"a \\`bug\\`"');
-      expect(beforeInsert(atwhoInstance, '~a ~bug')).toEqual('~"a \\~bug"');
-      expect(beforeInsert(atwhoInstance, '~a **bug')).toEqual('~"a \\*\\*bug"');
     });
   });
 
-- 
cgit v1.2.1