Bug 1328830 - [manifestparser] Check line continuation before looking for next key, r=jmaher
authorAndrew Halberstadt <ahalberstadt@mozilla.com>
Wed, 19 Jul 2017 14:48:01 -0400
changeset 642657 8d64da85644e03c63a3cf9aa62b33af1975bd35a
parent 642656 3e13ecea8235e6d356843e868e40e8098649e5bc
child 642658 fbf29cfa4717abfd141cfa7503d69a0ccee1fc2a
push id72833
push userbmo:emilio+bugs@crisal.io
push dateTue, 08 Aug 2017 16:50:16 +0000
reviewersjmaher
bugs1328830
milestone57.0a1
Bug 1328830 - [manifestparser] Check line continuation before looking for next key, r=jmaher Currently manifestparser will only look for line continuations *after* looking for a key. This means that line continuations cannot contain key separators. For example, this: [test] foo= bar=baz gets treated as: {'name': 'test', 'foo': '', 'bar': 'baz'} Here, bar=baz will be treated as a new key/value pair despite the indentation. This patch switches the order around, so we look for a continuation first. Now, it is only treated as a continuation if the indent is greater than the indent of the preceding key. So this manifest: [test] foo=bar baz=fleem is a continuation and results in: {'name': 'test', 'foo': 'bar\nbaz=fleem'} But this manifest: [test] foo=bar baz=fleem is not a continuation, and yields: {'name': 'test', 'foo': 'bar', 'baz': 'fleem'} MozReview-Commit-ID: FAMP5TUIo9q
dom/manifest/test/mochitest.ini
testing/mozbase/manifestparser/manifestparser/ini.py
testing/mozbase/manifestparser/tests/test_read_ini.py
testing/mozharness/manifestparser/ini.py
--- a/dom/manifest/test/mochitest.ini
+++ b/dom/manifest/test/mochitest.ini
@@ -1,13 +1,13 @@
 [DEFAULT]
 support-files =
-	common.js
-	resource.sjs
-	manifestLoader.html
+  common.js
+  resource.sjs
+  manifestLoader.html
   file_reg_appinstalled_event.html
   file_testserver.sjs
 [test_ImageObjectProcessor_sizes.html]
 [test_ImageObjectProcessor_src.html]
 [test_ImageObjectProcessor_type.html]
 [test_ManifestProcessor_background_color.html]
 [test_ManifestProcessor_dir.html]
 [test_ManifestProcessor_display.html]
@@ -15,9 +15,9 @@ support-files =
 [test_ManifestProcessor_JSON.html]
 [test_ManifestProcessor_lang.html]
 [test_ManifestProcessor_name_and_short_name.html]
 [test_ManifestProcessor_orientation.html]
 [test_ManifestProcessor_scope.html]
 [test_ManifestProcessor_start_url.html]
 [test_ManifestProcessor_theme_color.html]
 [test_ManifestProcessor_warnings.html]
-[test_window_onappinstalled_event.html]
\ No newline at end of file
+[test_window_onappinstalled_event.html]
--- a/testing/mozbase/manifestparser/manifestparser/ini.py
+++ b/testing/mozbase/manifestparser/manifestparser/ini.py
@@ -74,17 +74,17 @@ def read_ini(fp, variables=None, default
             inline_prefixes = next_prefixes
 
         if comment_start != sys.maxsize:
             stripped = stripped[:comment_start].rstrip()
 
         # check for a new section
         if len(stripped) > 2 and stripped[0] == '[' and stripped[-1] == ']':
             section = stripped[1:-1].strip()
-            key = value = None
+            key = value = key_indent = None
 
             # deal with DEFAULT section
             if section.lower() == default.lower():
                 if strict:
                     assert default not in section_names
                 section_names.add(default)
                 current_section = variables
                 continue
@@ -99,39 +99,42 @@ def read_ini(fp, variables=None, default
             sections.append((section, current_section))
             continue
 
         # if there aren't any sections yet, something bad happen
         if not section_names:
             raise IniParseError(fp, linenum, "Expected a comment or section, "
                                              "instead found '{}'".format(stripped))
 
+        # continuation line ?
+        line_indent = len(line) - len(line.lstrip(' '))
+        if key and line_indent > key_indent:
+            value = '%s%s%s' % (value, os.linesep, stripped)
+            current_section[key] = value
+            continue
+
         # (key, value) pair
         for separator in separators:
             if separator in stripped:
                 key, value = stripped.split(separator, 1)
                 key = key.strip()
                 value = value.strip()
+                key_indent = line_indent
 
                 if strict:
                     # make sure this key isn't already in the section or empty
                     assert key
                     if current_section is not variables:
                         assert key not in current_section
 
                 current_section[key] = value
                 break
         else:
-            # continuation line ?
-            if line[0].isspace() and key:
-                value = '%s%s%s' % (value, os.linesep, stripped)
-                current_section[key] = value
-            else:
-                # something bad happened!
-                raise IniParseError(fp, linenum, "Unexpected line '{}'".format(stripped))
+            # something bad happened!
+            raise IniParseError(fp, linenum, "Unexpected line '{}'".format(stripped))
 
     # server-root is a special os path declared relative to the manifest file.
     # inheritance demands we expand it as absolute
     if 'server-root' in variables:
         root = os.path.join(os.path.dirname(fp.name),
                             variables['server-root'])
         variables['server-root'] = os.path.abspath(root)
 
--- a/testing/mozbase/manifestparser/tests/test_read_ini.py
+++ b/testing/mozbase/manifestparser/tests/test_read_ini.py
@@ -14,25 +14,49 @@ import unittest
 from manifestparser import read_ini
 from StringIO import StringIO
 
 import mozunit
 
 
 class IniParserTest(unittest.TestCase):
 
+    def parse_manifest(self, string):
+        buf = StringIO()
+        buf.write(string)
+        buf.seek(0)
+        return read_ini(buf)
+
     def test_inline_comments(self):
-        manifest = """
+        result = self.parse_manifest("""
 [test_felinicity.py]
 kittens = true # This test requires kittens
 cats = false#but not cats
-"""
+""")[0][1]
+
         # make sure inline comments get stripped out, but comments without a space in front don't
-        buf = StringIO()
-        buf.write(manifest)
-        buf.seek(0)
-        result = read_ini(buf)[0][1]
         self.assertEqual(result['kittens'], 'true')
         self.assertEqual(result['cats'], "false#but not cats")
 
+    def test_line_continuation(self):
+        result = self.parse_manifest("""
+[test_caninicity.py]
+breeds =
+  sheppard
+  retriever
+  terrier
+
+[test_cats_and_dogs.py]
+  cats=yep
+  dogs=
+    yep
+      yep
+birds=nope
+  fish=nope
+""")
+        self.assertEqual(result[0][1]['breeds'].split(), ['sheppard', 'retriever', 'terrier'])
+        self.assertEqual(result[1][1]['cats'], 'yep')
+        self.assertEqual(result[1][1]['dogs'].split(), ['yep', 'yep'])
+        self.assertEqual(result[1][1]['birds'].split(), ['nope', 'fish=nope'])
+
 
 if __name__ == '__main__':
     mozunit.main()
--- a/testing/mozharness/manifestparser/ini.py
+++ b/testing/mozharness/manifestparser/ini.py
@@ -74,17 +74,17 @@ def read_ini(fp, variables=None, default
             inline_prefixes = next_prefixes
 
         if comment_start != sys.maxsize:
             stripped = stripped[:comment_start].rstrip()
 
         # check for a new section
         if len(stripped) > 2 and stripped[0] == '[' and stripped[-1] == ']':
             section = stripped[1:-1].strip()
-            key = value = None
+            key = value = key_indent = None
 
             # deal with DEFAULT section
             if section.lower() == default.lower():
                 if strict:
                     assert default not in section_names
                 section_names.add(default)
                 current_section = variables
                 continue
@@ -99,39 +99,42 @@ def read_ini(fp, variables=None, default
             sections.append((section, current_section))
             continue
 
         # if there aren't any sections yet, something bad happen
         if not section_names:
             raise IniParseError(fp, linenum, "Expected a comment or section, "
                                              "instead found '{}'".format(stripped))
 
+        # continuation line ?
+        line_indent = len(line) - len(line.lstrip(' '))
+        if key and line_indent > key_indent:
+            value = '%s%s%s' % (value, os.linesep, stripped)
+            current_section[key] = value
+            continue
+
         # (key, value) pair
         for separator in separators:
             if separator in stripped:
                 key, value = stripped.split(separator, 1)
                 key = key.strip()
                 value = value.strip()
+                key_indent = line_indent
 
                 if strict:
                     # make sure this key isn't already in the section or empty
                     assert key
                     if current_section is not variables:
                         assert key not in current_section
 
                 current_section[key] = value
                 break
         else:
-            # continuation line ?
-            if line[0].isspace() and key:
-                value = '%s%s%s' % (value, os.linesep, stripped)
-                current_section[key] = value
-            else:
-                # something bad happened!
-                raise IniParseError(fp, linenum, "Unexpected line '{}'".format(stripped))
+            # something bad happened!
+            raise IniParseError(fp, linenum, "Unexpected line '{}'".format(stripped))
 
     # server-root is a special os path declared relative to the manifest file.
     # inheritance demands we expand it as absolute
     if 'server-root' in variables:
         root = os.path.join(os.path.dirname(fp.name),
                             variables['server-root'])
         variables['server-root'] = os.path.abspath(root)