kallithea Changeset - 638ac4e65365

Changeset - 638ac4e65365

Parent rev.

Child rev.

[Not reviewed]

default

0 2 0

Thomas De Schampheleire - 8 years ago 2018-02-24 21:18:24
thomas.de_schampheleire@nokia.com

issues: gracefully handle invalid issue patterns

issue_pat is provided by the admin and can be invalid due to bad syntax.
In this case, a page load by a user could cause 500 Internal Server Error.

Instead, add a try..except clause around the compilation of issue_pat, and
skip invalid patterns.

2 files changed with 11 insertions and 3 deletions:

kallithea/lib/helpers.py

kallithea/tests/other/test_libs.py

0 comments (0 inline, 0 general)

kallithea/lib/helpers.py

➞

Show inline comments

@@ @@ -1096,104 +1096,108 @@ def urlify_text(s, repo_name=None, link_ @@
     # (This is not done in one step in html_escape, because character codes like
     # &#123; risk to be seen as an issue reference due to the presence of '#'.)
     s = s.replace("&apos;", "&#39;")
     return literal(s)
 def linkify_others(t, l):
     """Add a default link to html with links.
     HTML doesn't allow nesting of links, so the outer link must be broken up
     in pieces and give space for other links.
     """
     urls = re.compile(r'(\<a.*?\<\/a\>)',)
     links = []
     for e in urls.split(t):
         if e.strip() and not urls.match(e):
             links.append('<a class="message-link" href="%s">%s</a>' % (l, e))
         else:
             links.append(e)
     return ''.join(links)
 # Global variable that will hold the actual urlify_issues function body.
 # Will be set on first use when the global configuration has been read.
 _urlify_issues_f = None
 def urlify_issues(newtext, repo_name):
     """Urlify issue references according to .ini configuration"""
     global _urlify_issues_f
     if _urlify_issues_f is None:
         from kallithea import CONFIG
         from kallithea.model.db import URL_SEP
         assert CONFIG['sqlalchemy.url'] # make sure config has been loaded
         # Build chain of urlify functions, starting with not doing any transformation
         tmp_urlify_issues_f = lambda s: s
         issue_pat_re = re.compile(r'issue_pat(.*)')
         for k in CONFIG.keys():
             # Find all issue_pat* settings that also have corresponding server_link and prefix configuration
             m = issue_pat_re.match(k)
             if m is None:
                 continue
             suffix = m.group(1)
             issue_pat = CONFIG.get(k)
             issue_server_link = CONFIG.get('issue_server_link%s' % suffix)
             issue_prefix = CONFIG.get('issue_prefix%s' % suffix)
             if issue_pat and issue_server_link and issue_prefix is not None: # issue_prefix can be empty but should be present
                 log.debug('issue pattern %r: %r -> %r %r', suffix, issue_pat, issue_server_link, issue_prefix)
             else:
             if not issue_pat or not issue_server_link or issue_prefix is None: # issue_prefix can be empty but should be present
                 log.error('skipping incomplete issue pattern %r: %r -> %r %r', suffix, issue_pat, issue_server_link, issue_prefix)
                 continue
             # Wrap tmp_urlify_issues_f with substitution of this pattern, while making sure all loop variables (and compiled regexpes) are bound
             try:
             issue_re = re.compile(issue_pat)
             except re.error as e:
                 log.error('skipping invalid issue pattern %r: %r -> %r %r. Error: %s', suffix, issue_pat, issue_server_link, issue_prefix, str(e))
                 continue
             log.debug('issue pattern %r: %r -> %r %r', suffix, issue_pat, issue_server_link, issue_prefix)
             def issues_replace(match_obj,
                                issue_server_link=issue_server_link, issue_prefix=issue_prefix):
                 leadingspace = ' ' if match_obj.group().startswith(' ') else ''
                 issue_id = ''.join(match_obj.groups())
                 issue_url = issue_server_link.replace('{id}', issue_id)
                 issue_url = issue_url.replace('{repo}', repo_name)
                 issue_url = issue_url.replace('{repo_name}', repo_name.split(URL_SEP)[-1])
                 return (
                     '%(leadingspace)s<a class="issue-tracker-link" href="%(url)s">'
                     '%(issue-prefix)s%(id-repr)s'
                     '</a>'
                     ) % {
                      'leadingspace': leadingspace,
                      'url': issue_url,
                      'id-repr': issue_id,
                      'issue-prefix': issue_prefix,
                      'serv': issue_server_link,
+                    }
             tmp_urlify_issues_f = (lambda s,
                                           issue_re=issue_re, issues_replace=issues_replace, chain_f=tmp_urlify_issues_f:
                                    issue_re.sub(issues_replace, chain_f(s)))
         # Set tmp function globally - atomically
         _urlify_issues_f = tmp_urlify_issues_f
     return _urlify_issues_f(newtext)
 def render_w_mentions(source, repo_name=None):
     """
     Render plain text with revision hashes and issue references urlified
     and with @mention highlighting.
     """
     s = safe_unicode(source)
     s = urlify_text(s, repo_name=repo_name)
     return literal('<div class="formatted-fixed">%s</div>' % s)
 def short_ref(ref_type, ref_name):
     if ref_type == 'rev':
         return short_id(ref_name)
     return ref_name
 def link_to_ref(repo_name, ref_type, ref_name, rev=None):
     """
     Return full markup for a href to changeset_home for a changeset.

kallithea/tests/other/test_libs.py

➞

Show inline comments

@@ @@ -415,96 +415,100 @@ class TestLibs(TestController): @@
             'issue #123 and issue#456',
             """issue <a class="issue-tracker-link" href="http://foo/repo_name/issue/123">#123</a> and """
             """issue<a class="issue-tracker-link" href="http://foo/repo_name/issue/456">#456</a>"""),
         (r'\bPR(\d+)', 'http://foo/{repo}/issue/{id}', '#',
             'issue PR123 and issuePR456',
             """issue <a class="issue-tracker-link" href="http://foo/repo_name/issue/123">#123</a> and """
             """issuePR456"""),
         # following test case shows that \b does not work well in combination with '#': the expectations
         # are reversed from what is actually happening.
         (r'\b#(\d+)', 'http://foo/{repo}/issue/{id}', '#',
             'issue #123 and issue#456',
             """issue #123 and """
             """issue<a class="issue-tracker-link" href="http://foo/repo_name/issue/456">#456</a>"""),
         (r'[ \t]#(\d+)', 'http://foo/{repo}/issue/{id}', '#',
             'issue #123 and issue#456',
             """issue <a class="issue-tracker-link" href="http://foo/repo_name/issue/123">#123</a> and """
             """issue#456"""),
         (r'(?:pullrequest|pull request|PR|pr) ?#?(\d+)', 'http://foo/{repo}/issue/{id}', 'PR#',
             'fixed with pullrequest #1, pull request#2, PR 3, pr4',
             """fixed with <a class="issue-tracker-link" href="http://foo/repo_name/issue/1">PR#1</a>, """
             """<a class="issue-tracker-link" href="http://foo/repo_name/issue/2">PR#2</a>, """
             """<a class="issue-tracker-link" href="http://foo/repo_name/issue/3">PR#3</a>, """
             """<a class="issue-tracker-link" href="http://foo/repo_name/issue/4">PR#4</a>"""),
         (r'#(\d+)', 'http://foo/{repo}/issue/{id}', 'PR',
             'interesting issue #123',
             """interesting issue <a class="issue-tracker-link" href="http://foo/repo_name/issue/123">PR123</a>"""),
         (r'BUG\d{5}', 'https://bar/{repo}/{id}', 'BUG',
             'silly me, I did not parenthesize the {id}, BUG12345.',
             """silly me, I did not parenthesize the {id}, <a class="issue-tracker-link" href="https://bar/repo_name/">BUG</a>."""),
         (r'BUG(\d{5})', 'https://bar/{repo}/', 'BUG',
             'silly me, the URL does not contain {id}, BUG12345.',
             """silly me, the URL does not contain {id}, <a class="issue-tracker-link" href="https://bar/repo_name/">BUG12345</a>."""),
         (r'(PR-\d+)', 'http://foo/{repo}/issue/{id}', '',
             'interesting issue #123, err PR-56',
             """interesting issue #123, err <a class="issue-tracker-link" href="http://foo/repo_name/issue/PR-56">PR-56</a>"""),
         (r'#(\d+)', 'http://foo/{repo}/issue/{id}', '#',
             "some 'standard' text with apostrophes",
             """some &#39;standard&#39; text with apostrophes"""),
         (r'#(\d+)', 'http://foo/{repo}/issue/{id}', '#',
             "some 'standard' issue #123",
             """some &#39;standard&#39; issue <a class="issue-tracker-link" href="http://foo/repo_name/issue/123">#123</a>"""),
         (r'#(\d+)', 'http://foo/{repo}/issue/{id}', '#',
             'an issue   #123       with extra whitespace',
             """an issue   <a class="issue-tracker-link" href="http://foo/repo_name/issue/123">#123</a>       with extra whitespace"""),
         # Note: whitespace is squashed
         (r'(?:\s*#)(\d+)', 'http://foo/{repo}/issue/{id}', '#',
             'an issue   #123       with extra whitespace',
             """an issue <a class="issue-tracker-link" href="http://foo/repo_name/issue/123">#123</a>       with extra whitespace"""),
         # invalid issue pattern
         (r'(PR\d+', 'http://foo/{repo}/issue/{id}', '',
             'PR135',
             """PR135"""),
     ])
     def test_urlify_issues(self, issue_pat, issue_server, issue_prefix, sample, expected):
         from kallithea.lib.helpers import urlify_text
         config_stub = {
             'sqlalchemy.url': 'foo',
             'issue_pat': issue_pat,
             'issue_server_link': issue_server,
             'issue_prefix': issue_prefix,
+        }
         # force recreation of lazy function
         with mock.patch('kallithea.lib.helpers._urlify_issues_f', None):
             with mock.patch('kallithea.CONFIG', config_stub):
                 assert urlify_text(sample, 'repo_name') == expected
     @parametrize('sample,expected', [
         ('abc X5', 'abc <a class="issue-tracker-link" href="http://main/repo_name/main/5/">#5</a>'),
         ('abc pullrequest #6 xyz', 'abc <a class="issue-tracker-link" href="http://pr/repo_name/pr/6">PR#6</a> xyz'),
         ('pull request7 #', '<a class="issue-tracker-link" href="http://pr/repo_name/pr/7">PR#7</a> #'),
         ('look PR9 and pr #11', 'look <a class="issue-tracker-link" href="http://pr/repo_name/pr/9">PR#9</a> and <a class="issue-tracker-link" href="http://pr/repo_name/pr/11">PR#11</a>'),
         ('pullrequest#10 solves issue 9', '<a class="issue-tracker-link" href="http://pr/repo_name/pr/10">PR#10</a> solves <a class="issue-tracker-link" href="http://bug/repo_name/bug/9">bug#9</a>'),
         ('issue FAIL67', 'issue <a class="issue-tracker-link" href="http://fail/repo_name/67">67</a>'),
         ('issue FAILMORE89', 'issue FAILMORE89'), # no match because absent prefix
     ])
     def test_urlify_issues_multiple_issue_patterns(self, sample, expected):
         from kallithea.lib.helpers import urlify_text
         config_stub = {
             'sqlalchemy.url': 'foo',
             'issue_pat': 'X(\d+)',
             'issue_server_link': 'http://main/{repo}/main/{id}/',
             'issue_prefix': '#',
             'issue_pat_pr': '(?:pullrequest|pull request|PR|pr) ?#?(\d+)',
             'issue_server_link_pr': 'http://pr/{repo}/pr/{id}',
             'issue_prefix_pr': 'PR#',
             'issue_pat_bug': '(?:BUG|bug|issue) ?#?(\d+)',
             'issue_server_link_bug': 'http://bug/{repo}/bug/{id}',
             'issue_prefix_bug': 'bug#',
             'issue_pat_empty_prefix': 'FAIL(\d+)',
             'issue_server_link_empty_prefix': 'http://fail/{repo}/{id}',
             'issue_prefix_empty_prefix': '',
             'issue_pat_absent_prefix': 'FAILMORE(\d+)',
             'issue_server_link_absent_prefix': 'http://failmore/{repo}/{id}',
+        }
         # force recreation of lazy function
         with mock.patch('kallithea.lib.helpers._urlify_issues_f', None):
             with mock.patch('kallithea.CONFIG', config_stub):
                 assert urlify_text(sample, 'repo_name') == expected
     @parametrize('test,expected', [

0 comments (0 inline, 0 general)