From 75c9f6d4ae440df3c00625f79ea20079628a8d5f Mon Sep 17 00:00:00 2001
From: Adam Williamson <awilliam@redhat.com>
Date: Feb 22 2018 09:20:15 +0000
Subject: [PATCH 1/2] Remove some funky bits from inline pattern regexes


The bits removed here are *very* weird. They are inverted sets
which will match anything but a literal pipe or any character in
the \w class. The following bit, `(?<!\w)`, is a negative
lookbehind assertion which means 'match unless the previous
character is in the \w class'. So these two actually apply to the
same character and are almost entirely redundant.

I think the weird set was *meant* to be something like (^|w),
and the approximate idea here was to match 'start of string
or any non-word character followed by a #'. If so, then in fact
just removing the wacky inverted set is all we need to do, as
negative lookbehind assertions are allowed to match at the start
of the string. (There is actually a whole hidden complication
here where markdown, behind the scenes, adds some more bits to
the pattern we feed it to form a complete regex, but it happens
that everything works OK with that). The tests should suffice to
demonstrate that these regexes still behave as we expect. These
regexes originally came from @ralph, who says he's OK with this
change.

The changes to `handleMatch()` are all to do with the details of
what markdown actually does with these pattern classes. In short
markdown expects that the pattern's `handleMatch()` method spits
out something that corresponds to all the characters the class'
regex pattern consumes. The weird sets we remove in this commit
actually consume a character immediately before the `#`, and the
space in these `text` values is intended to replace that
character. The modified regexes no longer consume that character,
so `handleMatch()` no longer needs to replace it.

This actually fixes a subtle bug, because the character matched
by the set was *not* in fact always a space. This meant that if
you wrote something like 'Issue.#23' in a comment (for some
reason...), it would be turned into 'Issue #23', with the '#23'
linkified - the '.' was turned into a space. With this change,
that bug no longer happens.

Signed-off-by: Adam Williamson <awilliam@redhat.com>

---

diff --git a/pagure/pfmarkdown.py b/pagure/pfmarkdown.py
index 4c569c7..363e94b 100644
--- a/pagure/pfmarkdown.py
+++ b/pagure/pfmarkdown.py
@@ -51,8 +51,8 @@ COMMIT_LINK_RE = \
     '([a-zA-Z0-9_-]*?/)?'\
     '([a-zA-Z0-9_-]+)'\
     '#(?P<id>[\w]{40})'
-IMPLICIT_ISSUE_RE = r'[^|\w](?<!\w)#([0-9]+)'
-IMPLICIT_PR_RE = r'[^|\w](?<!\w)PR#([0-9]+)'
+IMPLICIT_ISSUE_RE = r'(?<!\w)#([0-9]+)'
+IMPLICIT_PR_RE = r'(?<!\w)PR#([0-9]+)'
 IMPLICIT_COMMIT_RE = r'(?<![<\w#])([a-f0-9]{7,40})'
 STRIKE_THROUGH_RE = r'~~(.*?)~~'
 
@@ -157,7 +157,7 @@ class ImplicitIssuePattern(markdown.inlinepatterns.Pattern):
     def handleMatch(self, m):
         """ When the pattern matches, update the text. """
         idx = markdown.util.AtomicString(m.group(2))
-        text = ' #%s' % idx
+        text = '#%s' % idx
         try:
             idx = int(idx)
         except (ValueError, TypeError):
@@ -185,7 +185,7 @@ class ImplicitPRPattern(markdown.inlinepatterns.Pattern):
     def handleMatch(self, m):
         """ When the pattern matches, update the text. """
         idx = markdown.util.AtomicString(m.group(2))
-        text = ' PR#%s' % idx
+        text = 'PR#%s' % idx
         try:
             idx = int(idx)
         except (ValueError, TypeError):

From c9fff89c4e0c69789ac049999773ab62c7e05262 Mon Sep 17 00:00:00 2001
From: Adam Williamson <awilliam@redhat.com>
Date: Feb 22 2018 09:20:15 +0000
Subject: [PATCH 2/2] Handle implicit issue link at start of line (#2987)


This treats a line starting with #123 - where 123 is a valid
issue or pull request ID - as an implicit link, rather than as
a header line.

Doing this is a bit complicated, as markdown does header line
processing early, in the block processors (that's before the
inline patterns we usually use). So we can't do this entirely
with inline patterns. We have to add a preprocessor that runs
before the block processors, and mungs the relevant strings so
the block processor doesn't turn them into header HTML. Then
we adjust the ImplicitIssuePattern to handle these modified
strings as well.

This also adds tests for implicit links, enabled by mocking out
enough bits of flask that `_get_ns_repo_user` is happy. There
are tests not only for implicit links at the start of a line but
also other cases, including the case of an implicit link not
surrounded by whitespace that was fixed in the previous commit.

Fixes https://pagure.io/pagure/issue/2987

Signed-off-by: Adam Williamson <awilliam@redhat.com>

---

diff --git a/pagure/pfmarkdown.py b/pagure/pfmarkdown.py
index 363e94b..9fe6ddf 100644
--- a/pagure/pfmarkdown.py
+++ b/pagure/pfmarkdown.py
@@ -22,8 +22,10 @@ Author: Ralph Bean <rbean@redhat.com>
 import flask
 
 import markdown.inlinepatterns
+import markdown.preprocessors
 import markdown.util
 import pygit2
+import re
 
 import pagure.lib
 from pagure.config import config as pagure_config
@@ -51,7 +53,10 @@ COMMIT_LINK_RE = \
     '([a-zA-Z0-9_-]*?/)?'\
     '([a-zA-Z0-9_-]+)'\
     '#(?P<id>[\w]{40})'
-IMPLICIT_ISSUE_RE = r'(?<!\w)#([0-9]+)'
+# PREPROCIMPLLINK is used by ImplicitIssuePreprocessor to replace the
+# '#' when a line starts with an implicit issue link, to prevent
+# markdown parsing it as a header; we have to handle it here
+IMPLICIT_ISSUE_RE = r'(?<!\w)(?:PREPROCIMPLLINK|#)([0-9]+)'
 IMPLICIT_PR_RE = r'(?<!\w)PR#([0-9]+)'
 IMPLICIT_COMMIT_RE = r'(?<![<\w#])([a-f0-9]{7,40})'
 STRIKE_THROUGH_RE = r'~~(.*?)~~'
@@ -151,6 +156,51 @@ class CommitLinkPattern(markdown.inlinepatterns.Pattern):
         return text
 
 
+class ImplicitIssuePreprocessor(markdown.preprocessors.Preprocessor):
+    """
+    Preprocessor which handles lines starting with an implicit
+    link. We have to modify these so that markdown doesn't interpret
+    them as headers.
+    """
+    def run(self, lines):
+        """
+        If a line starts with an implicit issue link like #152,
+        we replace the # with PREPROCIMPLLINK. This prevents markdown
+        parsing the line as a header. ImplicitIssuePattern will catch
+        and parse the text later. Otherwise, we change nothing.
+        """
+        # match a # character, then any number of digits
+        regex = re.compile(r'#([0-9]+)')
+        new_lines = []
+        for line in lines:
+            # avoid calling the regex if line doesn't start with #
+            if line.startswith('#'):
+                match = regex.match(line)
+                if match:
+                    idx = int(match.group(1))
+                    # we have to check if this is a real issue or PR now.
+                    # we can't just 'tag' the text somehow and leave it to
+                    # the pattern to check, as if it's *not* one we want
+                    # the line treated as a header, so we need the block
+                    # processor to see it unmodified.
+                    try:
+                        namespace, repo, user = _get_ns_repo_user()
+                    except RuntimeError:
+                        # non-match path, keep original line
+                        new_lines.append(line)
+                        continue
+                    if (
+                        _issue_exists(user, namespace, repo, idx) or
+                        _pr_exists(user, namespace, repo, idx)
+                    ):
+                        # tweak the text
+                        new_lines.append('PREPROCIMPLLINK' + line[1:])
+                        continue
+            # this is a non-match path, keep original line
+            new_lines.append(line)
+            continue
+        return new_lines
+
 class ImplicitIssuePattern(markdown.inlinepatterns.Pattern):
     """ Implicit issue pattern. """
 
@@ -275,6 +325,8 @@ class PagureExtension(markdown.extensions.Extension):
         ])
         markdown.inlinepatterns.AUTOLINK_RE = AUTOLINK_RE
 
+        md.preprocessors['implicit_issue'] = ImplicitIssuePreprocessor()
+
         md.inlinePatterns['mention'] = MentionPattern(MENTION_RE)
 
         md.inlinePatterns['implicit_commit'] = ImplicitCommitPattern(
diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py
index 654b1ef..8d20c89 100644
--- a/tests/test_pagure_lib.py
+++ b/tests/test_pagure_lib.py
@@ -4078,9 +4078,13 @@ class PagureLibtests(tests.Modeltests):
         watch_list = [obj.name for obj in watch_list_objs]
         self.assertEqual(watch_list, [])
 
+    @patch('flask.request.url', 'http://pagure.org/test/issue/69')
+    @patch('flask.request.url_root', 'http://pagure.org/')
+    @patch('flask.request.args.get', return_value=None)
+    @patch('flask.request')
     @patch('flask.g')
     @patch('pagure.lib.notify.send_email', MagicMock(return_value=True))
-    def test_text2markdown(self, g):
+    def test_text2markdown(self, g, req, reqget):
         ''' Test the test2markdown method in pagure.lib. '''
         pagure.config.config['TESTING'] = True
         pagure.config.config['SERVER_NAME'] = 'pagure.org'
@@ -4093,8 +4097,22 @@ class PagureLibtests(tests.Modeltests):
         # PR#1 to project test
         self.test_new_pull_request()
 
-        # create PR#2 to project pingou/test
+        # create an issue (will be #2) in 'test' project
         repo = pagure.lib._get_project(self.session, 'test')
+        iss = pagure.lib.new_issue(
+            issue_id=2,
+            session=self.session,
+            repo=repo,
+            title='test issue',
+            content='content test issue',
+            user='pingou',
+            ticketfolder=None,
+        )
+        self.session.commit()
+        self.assertEqual(iss.id, 2)
+        self.assertEqual(iss.title, 'test issue')
+
+        # create PR#2 to project pingou/test
         forked_repo = pagure.lib._get_project(self.session, 'test', user='pingou')
         req = pagure.lib.new_pull_request(
             requestid=2,
@@ -4197,6 +4215,12 @@ class PagureLibtests(tests.Modeltests):
             'test#1 bazinga!',
             'pingou opened the PR forks/pingou/test#2',
             'fork/pingou/ns/test#8 is private',
+            'implicit link to #1',
+            'implicit link .#1. with non-whitespace, non-word characters',
+            '#2 - implicit link at start of line',
+            '#2. implicit link at start of line with no whitespace after',
+            '#regular header',
+            '#34 looks like an implicit link, but no issue 34',
             'pingou committed on test#9364354a4555ba17aa60f0dc844d70b74eb1aecd',
             'irc://pagure.io',
             'ircs://pagure.io',
@@ -4245,6 +4269,18 @@ class PagureLibtests(tests.Modeltests):
             # 'fork/pingou/ns/test#8 is private',
             '<p><a href="http://pagure.org/fork/pingou/ns/test/issue/8" '
             'title="Private issue">pingou/ns/test#8</a> is private</p>',
+            # 'implicit link to #1',
+            '<p>implicit link to <a href="http://pagure.org/test/pull-request/1" title="[Open] test pull-request">#1</a></p>',
+            # 'implicit link .#1. with non-whitespace, non-word characters',
+            '<p>implicit link .<a href="http://pagure.org/test/pull-request/1" title="[Open] test pull-request">#1</a>. with non-whitespace, non-word characters</p>',
+            # '#2 - implicit link at start of line',
+            '<p><a href="http://pagure.org/test/issue/2" title="[Open] test issue">#2</a> - implicit link at start of line</p>',
+            # '#2. implicit link at start of line with no whitespace after',
+            '<p><a href="http://pagure.org/test/issue/2" title="[Open] test issue">#2</a>. implicit link at start of line with no whitespace after</p>',
+            # '#regular header',
+            '<h1>regular header</h1>',
+            # '#34 looks like an implicit link, but no issue 34',
+            '<h1>34 looks like an implicit link, but no issue 34</h1>',
             # 'pingou committed on test#9364354a4555ba17aa60f0dc844d70b74eb1aecd',
             '<p>pingou committed on <a href="http://pagure.org/'
             'test/c/9364354a4555ba17aa60f0dc844d70b74eb1aecd" '