[master/f21 v3] Fix # handling in SimpleConfigFile (#1045687)

Anne Mulhern amulhern at redhat.com
Wed Oct 15 14:19:26 UTC 2014





----- Original Message -----
> From: "Anne Mulhern" <amulhern at redhat.com>
> To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> Sent: Wednesday, October 15, 2014 8:39:05 AM
> Subject: Re: [master/f21 v3] Fix # handling in SimpleConfigFile (#1045687)
> 
> 
> 
> 
> 
> ----- Original Message -----
> > From: "Brian C. Lane" <bcl at redhat.com>
> > To: anaconda-patches at lists.fedorahosted.org
> > Sent: Tuesday, October 14, 2014 5:48:07 PM
> > Subject: [master/f21 v3] Fix # handling in SimpleConfigFile (#1045687)
> > 
> > It was treating anything after a # as a comment, even if it appeared
> > inside a quoted string. This fixes it to find the last # outside a
> > (optional) quoted string and adds several tests.
> > 
> > This also adds back the non-IfcfgFile tests for SimpleConfigFile from
> > old_tests
> > ---
> >  pyanaconda/simpleconfig.py                  |  63 ++++++++++----
> >  tests/pyanaconda_tests/simpleconfig_test.py | 130
> >  ++++++++++++++++++++++++++++
> >  2 files changed, 177 insertions(+), 16 deletions(-)
> >  create mode 100644 tests/pyanaconda_tests/simpleconfig_test.py
> > 
> > diff --git a/pyanaconda/simpleconfig.py b/pyanaconda/simpleconfig.py
> > index a148384..1a9751c 100644
> > --- a/pyanaconda/simpleconfig.py
> > +++ b/pyanaconda/simpleconfig.py
> > @@ -43,6 +43,30 @@ def quote(s, always=False):
> >              return s
> >      return '"'+s.replace('"', '\\"')+'"'
> >  
> > +def find_comment(s):
> > +    """ Look for a # comment outside of a quoted string.
> > +        If there are no quotes, find the last # in the string.
> > +
> > +        :param str s: string to check for comment and quotes
> > +        :returns: index of comment or None
> > +        :rtype: int or None
> > +
> > +        Handles comments inside quotes and quotes inside quotes.
> > +    """
> > +    q = None
> > +    for i in range(len(s)):
> > +        if not q and s[i] == '#':
> > +            return i
> > +
> > +        # Ignore quotes inside other quotes
> > +        if s[i] in "'\"":
> > +            if s[i] == q:
> > +                q = None
> > +            elif q is None:
> > +                q = s[i]
> > +    return None
> > +
> > +
> >  class SimpleConfigFile(object):
> >      """ Edit values in a configuration file without changing comments.
> >          Supports KEY=VALUE lines and ignores everything else.
> > @@ -73,7 +97,7 @@ class SimpleConfigFile(object):
> >          with open(filename) as f:
> >              for line in f:
> >                  self._lines.append(line)
> > -                key, value = self._parseline(line)
> > +                key, value, _comment = self._parseline(line)
> >                  if key:
> >                      self.info[key] = value
> >  
> > @@ -116,27 +140,40 @@ class SimpleConfigFile(object):
> >          return self.info.get(upperASCII(key), "")
> >  
> >      def _parseline(self, line):
> > -        """ parse a line into a key, value pair
> > +        """ parse a line into a key, value and comment
> > +
> > +            :param str line: Line to be parsed
> > +            :returns: Tuple of key, value, comment
> > +            :rtype: tuple
> > +
> >              Handle comments and optionally unquote quoted strings
> > -            Returns (key, value) or (None, None)
> > -            key is always UPPERCASE
> > +            Returns (key, value, comment) or (None, None, comment)
> > +            key is always UPPERCASE and comment may by "" if none was
> > found.
> >          """
> >          s = line.strip()
> > -        if '#' in s:
> > -            s = s[:s.find('#')] # remove from comment to EOL
> > -            s = s.strip()       # and any unnecessary whitespace
> > +        # Look for a # outside any quotes
> > +        comment = ""
> > +        comment_index = find_comment(s)
> > +        if comment_index is not None:
> > +            comment = s[comment_index:]
> > +            s = s[:comment_index]   # remove from comment to EOL
> > +            s = s.strip()           # and any unnecessary whitespace
> 
> You should lose the conditional strip() above.
> 
> > +
> >          key, eq, val = s.partition('=')
> > +        key = key.strip()
> 
> But add:
>            val = val.strip()
> 
> here, unless leading whitespace is significant in val, which seems unlikely.
> 
> >          if self.read_unquote:
> >              val = unquote(val)
> >          if key != '' and eq == '=':
> > -            return (upperASCII(key), val)
> > +            return (upperASCII(key), val, comment)
> >          else:
> > -            return (None, None)
> > +            return (None, None, comment)
> >  
> >      def _kvpair(self, key, comment=""):
> >          value = self.info[key]
> >          if self.write_quote or self.always_quote:
> >              value = quote(value, self.always_quote)
> > +        if comment:
> > +            comment = " " + comment
> >          return key + '=' + value + comment + "\n"
> 
> Above three lines could be:
>            padding = " " if comment else ""
>            return key + "=" + value + padding + comment + "\n"
> 

Or try,

if comment:
    padding = " "
else:
    padding = ""
return key + "=" + value + padding + comment + "\n"

so that name is more meaningful.

> >  
> >      def __str__(self):
> > @@ -146,17 +183,13 @@ class SimpleConfigFile(object):
> >          oldkeys = []
> >          s = ""
> >          for line in self._lines:
> > -            key = self._parseline(line)[0]
> > +            key, _value, comment = self._parseline(line)
> >              if key is None:
> >                  s += line
> >              else:
> >                  if key not in self.info:
> >                      continue
> >                  oldkeys.append(key)
> > -                if "#" in line:
> > -                    comment = " " + line[line.find("#"):]
> > -                else:
> > -                    comment = ""
> >                  s += self._kvpair(key, comment)
> >  
> >          # Add new keys
> > @@ -165,5 +198,3 @@ class SimpleConfigFile(object):
> >                  s += self._kvpair(key)
> >  
> >          return s
> > -
> > -
> > diff --git a/tests/pyanaconda_tests/simpleconfig_test.py
> > b/tests/pyanaconda_tests/simpleconfig_test.py
> > new file mode 100644
> > index 0000000..3719ec1
> > --- /dev/null
> > +++ b/tests/pyanaconda_tests/simpleconfig_test.py
> > @@ -0,0 +1,130 @@
> > +# -*- coding: utf-8 -*-
> > +#
> > +# Copyright (C) 2014  Red Hat, Inc.
> > +#
> > +# This copyrighted material is made available to anyone wishing to use,
> > +# modify, copy, or redistribute it subject to the terms and conditions of
> > +# the GNU General Public License v.2, or (at your option) any later
> > version.
> > +# This program is distributed in the hope that it will be useful, but
> > WITHOUT
> > +# ANY WARRANTY expressed or implied, including the implied warranties of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > General
> > +# Public License for more details.  You should have received a copy of the
> > +# GNU General Public License along with this program; if not, write to the
> > +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> > MA
> > +# 02110-1301, USA.  Any Red Hat trademarks that are incorporated in the
> > +# source code or documentation are not subject to the GNU General Public
> > +# License and may only be used or replicated with the express permission
> > of
> > +# Red Hat, Inc.
> > +#
> > +# Red Hat Author(s): Brian C. Lane <bcl at redhat.com>
> > +
> > +from pyanaconda.simpleconfig import SimpleConfigFile
> > +from pyanaconda import simpleconfig
> > +import unittest
> > +import tempfile
> > +
> > +class SimpleConfigTests(unittest.TestCase):
> > +    TEST_CONFIG = """ESSID="Example Network #1"
> > +ESSID2="Network #2" # With a comment
> > +COMMENT="Save this string" # Strip this comment
> > +#SKIP=Skip this commented line
> > +BOOT=always
> > +KEY=VALUE # Comment "with quotes"
> > +KEY2="A single ' inside" # And comment "with quotes"
> > +"""
> > +
> > +    def comment_test(self):
> > +        with tempfile.NamedTemporaryFile() as testconfig:
> > +            testconfig.write(self.TEST_CONFIG)
> > +            testconfig.flush()
> > +
> > +            config = SimpleConfigFile(testconfig.name)
> > +            config.read()
> > +            self.assertEqual(config.get("ESSID"), "Example Network #1")
> > +            self.assertEqual(config.get("ESSID2"), "Network #2")
> > +            self.assertEqual(config.get("COMMENT"), "Save this string")
> > +            self.assertEqual(str(config), self.TEST_CONFIG)
> > +
> > +    def unquote_test(self):
> > +        self.assertEqual(simpleconfig.unquote("plain string"), "plain
> > string")
> > +        self.assertEqual(simpleconfig.unquote('"double quote"'), "double
> > quote")
> > +        self.assertEqual(simpleconfig.unquote("'single quote'"), "single
> > quote")
> > +
> > +    def quote_test(self):
> > +        self.assertEqual(simpleconfig.quote("nospaces"), "nospaces")
> > +        self.assertEqual(simpleconfig.quote("plain string"), '"plain
> > string"')
> > +        self.assertEqual(simpleconfig.quote("alwaysquote", always=True),
> > '"alwaysquote"')
> > +
> > +    def set_and_get_test(self):
> > +        """Setting and getting values"""
> > +        scf = SimpleConfigFile()
> > +        scf.set(('key1', 'value1'))
> > +        self.assertEqual(scf.get('key1'), 'value1')
> > +        scf.set(('KEY2', 'value2'))
> > +        self.assertEqual(scf.get('key2'), 'value2')
> > +        scf.set(('KEY3', 'value3'))
> > +        self.assertEqual(scf.get('KEY3'), 'value3')
> > +        scf.set(('key4', 'value4'))
> > +        self.assertEqual(scf.get('KEY4'), 'value4')
> > +
> > +    def unset_test(self):
> > +        scf = SimpleConfigFile()
> > +        scf.set(('key1', 'value1'))
> > +        scf.unset(('key1'))
> > +        self.assertEqual(scf.get('key1'), '')
> > +
> > +    def write_test(self):
> > +        with tempfile.NamedTemporaryFile() as testconfig:
> > +            scf = SimpleConfigFile()
> > +            scf.set(('key1', 'value1'))
> > +            scf.write(testconfig.name)
> > +            testconfig.flush()
> > +            self.assertEqual(open(testconfig.name).read(),
> > 'KEY1=value1\n')
> > +
> > +    def read_test(self):
> > +        with tempfile.NamedTemporaryFile() as testconfig:
> > +            scf = SimpleConfigFile()
> > +            open(testconfig.name, 'w').write('KEY1="value1"\n')
> > +            testconfig.flush()
> > +            scf.read(testconfig.name)
> > +            self.assertEqual(scf.get('key1'), 'value1')
> > +
> > +    def read_write_test(self):
> > +        with tempfile.NamedTemporaryFile() as testconfig:
> > +            testconfig.write(self.TEST_CONFIG)
> > +            testconfig.flush()
> > +
> > +            scf = SimpleConfigFile()
> > +            scf.read(testconfig.name)
> > +            scf.write(testconfig.name)
> > +            testconfig.flush()
> > +            self.assertEqual(open(testconfig.name).read(),
> > self.TEST_CONFIG)
> > +
> > +    def write_new_keys_test(self):
> > +        with tempfile.NamedTemporaryFile() as testconfig:
> > +            testconfig.write(self.TEST_CONFIG)
> > +            testconfig.flush()
> > +
> > +            scf = SimpleConfigFile()
> > +            scf.read(testconfig.name)
> > +            scf.set(("key1", "value1"))
> > +            scf.write(testconfig.name)
> > +            testconfig.flush()
> > +
> > +            self.assertEqual(open(testconfig.name).read(),
> > +                             self.TEST_CONFIG+"KEY1=value1\n")
> > +
> > +    def remove_key_test(self):
> > +        with tempfile.NamedTemporaryFile() as testconfig:
> > +            testconfig.write(self.TEST_CONFIG)
> > +            testconfig.flush()
> > +
> > +            scf = SimpleConfigFile()
> > +            scf.read(testconfig.name)
> > +            self.assertEqual(scf.get("BOOT"), "always")
> > +            scf.unset("BOOT")
> > +            scf.write(testconfig.name)
> > +            testconfig.flush()
> > +            scf.reset()
> > +            scf.read(testconfig.name)
> > +            self.assertEqual(scf.get("BOOT"), "")
> > --
> > 1.9.3
> > 
> > _______________________________________________
> > anaconda-patches mailing list
> > anaconda-patches at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> > 
> 
> Other than two inline comments, looks good.
> 
> - mulhern
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> 


More information about the anaconda-patches mailing list