Add checking for unescaped quotes in a Relnote tag

Because we need the quotes to check for starting and ending
Relnote tags, having unescaped quotes in the release note causes
the parsing to truncate at that unescaped release note.

So the following scenarios will now be disallowed:

Relnote: There are "quotes" in this relnote.
Relnote: "There are "quotes" in this relnote."
Relnote: "There are unescaped quotes in the second half of
this "sentence"."

Bug: 172663867
Test: ./pre-upload.py
Change-Id: Idc3769eab64aff7455453b9d50317e0d0cad2a44

2 files changed, 72 insertions, 6 deletions

diff --git a/rh/hooks.py b/rh/hooks.py
index 31de509..a622bf8 100644
--- a/rh/hooks.py
+++ b/rh/hooks.py
@@ -592,6 +592,18 @@ Single-line Relnote example:
 Relnote: Added a new API `Class#containsData`
 """
 
+RELNOTE_INVALID_QUOTES_MSG = """Commit message contains something that looks
+similar to the "Relnote:" tag but might be malformatted.  If you are using
+quotes that do not mark the start or end of a Relnote, you need to escape them
+with a backslash.
+
+Non-starting/non-ending quote Relnote examples:
+
+Relnote: "Fixed an error with `Class#getBar()` where \"foo\" would be returned
+in edge cases."
+Relnote: Added a new API to handle strings like \"foo\"
+"""
+
 def check_commit_msg_relnote_field_format(project, commit, desc, _diff,
                                           options=None):
     """Check the commit for one correctly formatted 'Relnote:' line.
@@ -600,6 +612,8 @@ def check_commit_msg_relnote_field_format(project, commit, desc, _diff,
     (1) Checks for possible misspellings of the 'Relnote:' tag.
     (2) Ensures that multiline release notes are properly formatted with a
     starting quote and an endling quote.
+    (3) Checks that release notes that contain non-starting or non-ending
+    quotes are escaped with a backslash.
     """
     field = 'Relnote'
     regex_relnote = r'^%s:.*$' % (field,)
@@ -676,16 +690,53 @@ def check_commit_msg_relnote_field_format(project, commit, desc, _diff,
             first_quote_found = True
             # A single-line Relnote containing a start and ending quote
             # is valid as well.
-            if cur_line.count('"') == 2:
+            if cur_line.count('"') - cur_line.count('\\"') == 2:
                 second_quote_found = True
                 break
-
     if first_quote_found != second_quote_found:
         ret.append(
             rh.results.HookResult(('commit msg: "%s:" '
                                    'tag missing closing quote') % (field,),
                                   project, commit,
                                   error=RELNOTE_MISSING_QUOTES_MSG))
+
+    # Check 4: Check that non-starting or non-ending quotes are escaped with a
+    # backslash.
+    line_needs_checking = False
+    uses_invalide_quotes = False
+    for cur_line in desc_lines:
+        if check_re_other_fields.findall(cur_line):
+            line_needs_checking = False
+        on_relnote_line = check_re_relnote.match(cur_line)
+        # Determine if we are parsing the base `Relnote:` line.
+        if on_relnote_line and '"' in cur_line:
+            line_needs_checking = True
+        if line_needs_checking:
+            stripped_line = re.sub('^%s:' % field, '', cur_line,
+                                   flags=re.IGNORECASE).strip()
+            for i, character in enumerate(stripped_line):
+                # Case 1: Valid quote at the beginning of the
+                # base `Relnote:` line.
+                if on_relnote_line and i == 0:
+                    continue
+                # Case 2: Invalid quote at the beginning of following lines.
+                if not on_relnote_line and i == 0 and character == '"':
+                    uses_invalide_quotes = True
+                    break
+                # Case 3: Check all other cases.
+                if (character == '"'
+                        and 0 < i < len(stripped_line) - 1
+                        and stripped_line[i-1] != "\""
+                        and stripped_line[i-1] != "\\"):
+                    uses_invalide_quotes = True
+                    break
+
+    if uses_invalide_quotes:
+        ret.append(rh.results.HookResult(('commit msg: "%s:" '
+                                          'tag using unescaped '
+                                          'quotes') % (field,),
+                                         project, commit,
+                                         error=RELNOTE_INVALID_QUOTES_MSG))
     return ret
 
 
diff --git a/rh/hooks_unittest.py b/rh/hooks_unittest.py
index 33b911d..74971b8 100755
--- a/rh/hooks_unittest.py
+++ b/rh/hooks_unittest.py
@@ -525,12 +525,13 @@ class BuiltinHooksTests(unittest.TestCase):
             True,
             (
                 'subj',
-                'subj\n\nTest: i did done dood it\n',
-                'subj\n\nMore content\n\nTest: i did done dood it\n',
-                'subj\n\nRelnote: This is a release note\n',
-                'subj\n\nRelnote:This is a release note\n',
+                'subj\n\nTest: i did done dood it\nBug: 1234',
+                'subj\n\nMore content\n\nTest: i did done dood it\nBug: 1234',
+                'subj\n\nRelnote: This is a release note\nBug: 1234',
+                'subj\n\nRelnote:This is a release note\nBug: 1234',
                 'subj\n\nRelnote: This is a release note.\nBug: 1234',
                 'subj\n\nRelnote: "This is a release note."\nBug: 1234',
+                'subj\n\nRelnote: "This is a \\"release note\\"."\n\nBug: 1234',
                 'subj\n\nRelnote: This is a release note.\nChange-Id: 1234',
                 'subj\n\nRelnote: This is a release note.\n\nChange-Id: 1234',
                 ('subj\n\nRelnote: "This is a release note."\n\n'
@@ -546,6 +547,10 @@ class BuiltinHooksTests(unittest.TestCase):
                  'It contains a correct second line.\n'
                  'And even a third line."\n'
                  'Bug: 1234'),
+                ('subj\n\nRelnote: "This is a release note.\n'
+                 'It contains a correct second line.\n'
+                 '\\"Quotes\\" are even used on the third line."\n'
+                 'Bug: 1234'),
                 ('subj\n\nRelnote: This is release note 1.\n'
                  'Relnote: This is release note 2.\n'
                  'Bug: 1234'),
@@ -558,6 +563,10 @@ class BuiltinHooksTests(unittest.TestCase):
                  'Relnote: "This is release note 2, and it\n'
                  'contains a correctly formatted second line."\n'
                  'Bug: 1234'),
+                ('subj\n\nRelnote: "This is a release note with\n'
+                 'a correctly formatted second line."\n\n'
+                 'Bug: 1234'
+                 'Here is some extra "quoted" content.'),
             ))
 
         # Check some bad messages.
@@ -570,6 +579,8 @@ class BuiltinHooksTests(unittest.TestCase):
                 'subj\n\nRel-note: This is a release note.\n',
                 'subj\n\nrelnoTes: This is a release note.\n',
                 'subj\n\nrel-Note: This is a release note.\n',
+                'subj\n\nRelnote: "This is a "release note"."\nBug: 1234',
+                'subj\n\nRelnote: This is a "release note".\nBug: 1234',
                 ('subj\n\nRelnote: This is a release note.\n'
                  'It contains an incorrect second line.'),
                 ('subj\n\nRelnote: "This is a release note.\n'
@@ -592,6 +603,10 @@ class BuiltinHooksTests(unittest.TestCase):
                  'Relnote: This is release note 2, and it\n'
                  'contains an incorrectly formatted second line.\n'
                  'Bug: 1234'),
+                ('subj\n\nRelnote: "This is a release note.\n'
+                 'It contains a correct second line.\n'
+                 'But incorrect "quotes" on the third line."\n'
+                 'Bug: 1234'),
             ))
 
     def test_commit_msg_relnote_for_current_txt(self, _mock_check, _mock_run):