From b1bc37597f0d36084c4dcb15977fe6d4b9322cd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20R=C3=B6thlisberger?= Date: Tue, 19 Mar 2024 09:59:08 +0000 Subject: gh-116957: configparser: Do post-process values after DuplicateOptionError (GH-116958) If you catch DuplicateOptionError / DuplicateSectionError when reading a config file (the intention is to skip invalid config files) and then attempt to use the ConfigParser instance, any values it *had* read successfully so far, were stored as a list instead of string! Later `get` calls would raise "AttributeError: 'list' object has no attribute 'find'" from somewhere deep in the interpolation code. --- Lib/configparser.py | 184 +++++++++++---------- Lib/test/test_configparser.py | 15 ++ .../2024-03-18-14-36-50.gh-issue-116957.dTCs4f.rst | 3 + 3 files changed, 111 insertions(+), 91 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-03-18-14-36-50.gh-issue-116957.dTCs4f.rst diff --git a/Lib/configparser.py b/Lib/configparser.py index 241f10a..8f182ee 100644 --- a/Lib/configparser.py +++ b/Lib/configparser.py @@ -961,102 +961,104 @@ class RawConfigParser(MutableMapping): lineno = 0 indent_level = 0 e = None # None, or an exception - for lineno, line in enumerate(fp, start=1): - comment_start = sys.maxsize - # strip inline comments - inline_prefixes = {p: -1 for p in self._inline_comment_prefixes} - while comment_start == sys.maxsize and inline_prefixes: - next_prefixes = {} - for prefix, index in inline_prefixes.items(): - index = line.find(prefix, index+1) - if index == -1: - continue - next_prefixes[prefix] = index - if index == 0 or (index > 0 and line[index-1].isspace()): - comment_start = min(comment_start, index) - inline_prefixes = next_prefixes - # strip full line comments - for prefix in self._comment_prefixes: - if line.strip().startswith(prefix): - comment_start = 0 - break - if comment_start == sys.maxsize: - comment_start = None - value = line[:comment_start].strip() - if not value: - if self._empty_lines_in_values: - # add empty line to the value, but only if there was no - # comment on the line - if (comment_start is None and - cursect is not None and - optname and - cursect[optname] is not None): - cursect[optname].append('') # newlines added at join - else: - # empty line marks end of value - indent_level = sys.maxsize - continue - # continuation line? - first_nonspace = self.NONSPACECRE.search(line) - cur_indent_level = first_nonspace.start() if first_nonspace else 0 - if (cursect is not None and optname and - cur_indent_level > indent_level): - if cursect[optname] is None: - raise MultilineContinuationError(fpname, lineno, line) - cursect[optname].append(value) - # a section header or option header? - else: - indent_level = cur_indent_level - # is it a section header? - mo = self.SECTCRE.match(value) - if mo: - sectname = mo.group('header') - if sectname in self._sections: - if self._strict and sectname in elements_added: - raise DuplicateSectionError(sectname, fpname, - lineno) - cursect = self._sections[sectname] - elements_added.add(sectname) - elif sectname == self.default_section: - cursect = self._defaults + try: + for lineno, line in enumerate(fp, start=1): + comment_start = sys.maxsize + # strip inline comments + inline_prefixes = {p: -1 for p in self._inline_comment_prefixes} + while comment_start == sys.maxsize and inline_prefixes: + next_prefixes = {} + for prefix, index in inline_prefixes.items(): + index = line.find(prefix, index+1) + if index == -1: + continue + next_prefixes[prefix] = index + if index == 0 or (index > 0 and line[index-1].isspace()): + comment_start = min(comment_start, index) + inline_prefixes = next_prefixes + # strip full line comments + for prefix in self._comment_prefixes: + if line.strip().startswith(prefix): + comment_start = 0 + break + if comment_start == sys.maxsize: + comment_start = None + value = line[:comment_start].strip() + if not value: + if self._empty_lines_in_values: + # add empty line to the value, but only if there was no + # comment on the line + if (comment_start is None and + cursect is not None and + optname and + cursect[optname] is not None): + cursect[optname].append('') # newlines added at join else: - cursect = self._dict() - self._sections[sectname] = cursect - self._proxies[sectname] = SectionProxy(self, sectname) - elements_added.add(sectname) - # So sections can't start with a continuation line - optname = None - # no section header in the file? - elif cursect is None: - raise MissingSectionHeaderError(fpname, lineno, line) - # an option line? + # empty line marks end of value + indent_level = sys.maxsize + continue + # continuation line? + first_nonspace = self.NONSPACECRE.search(line) + cur_indent_level = first_nonspace.start() if first_nonspace else 0 + if (cursect is not None and optname and + cur_indent_level > indent_level): + if cursect[optname] is None: + raise MultilineContinuationError(fpname, lineno, line) + cursect[optname].append(value) + # a section header or option header? else: - mo = self._optcre.match(value) + indent_level = cur_indent_level + # is it a section header? + mo = self.SECTCRE.match(value) if mo: - optname, vi, optval = mo.group('option', 'vi', 'value') - if not optname: - e = self._handle_error(e, fpname, lineno, line) - optname = self.optionxform(optname.rstrip()) - if (self._strict and - (sectname, optname) in elements_added): - raise DuplicateOptionError(sectname, optname, - fpname, lineno) - elements_added.add((sectname, optname)) - # This check is fine because the OPTCRE cannot - # match if it would set optval to None - if optval is not None: - optval = optval.strip() - cursect[optname] = [optval] + sectname = mo.group('header') + if sectname in self._sections: + if self._strict and sectname in elements_added: + raise DuplicateSectionError(sectname, fpname, + lineno) + cursect = self._sections[sectname] + elements_added.add(sectname) + elif sectname == self.default_section: + cursect = self._defaults else: - # valueless option handling - cursect[optname] = None + cursect = self._dict() + self._sections[sectname] = cursect + self._proxies[sectname] = SectionProxy(self, sectname) + elements_added.add(sectname) + # So sections can't start with a continuation line + optname = None + # no section header in the file? + elif cursect is None: + raise MissingSectionHeaderError(fpname, lineno, line) + # an option line? else: - # a non-fatal parsing error occurred. set up the - # exception but keep going. the exception will be - # raised at the end of the file and will contain a - # list of all bogus lines - e = self._handle_error(e, fpname, lineno, line) - self._join_multiline_values() + mo = self._optcre.match(value) + if mo: + optname, vi, optval = mo.group('option', 'vi', 'value') + if not optname: + e = self._handle_error(e, fpname, lineno, line) + optname = self.optionxform(optname.rstrip()) + if (self._strict and + (sectname, optname) in elements_added): + raise DuplicateOptionError(sectname, optname, + fpname, lineno) + elements_added.add((sectname, optname)) + # This check is fine because the OPTCRE cannot + # match if it would set optval to None + if optval is not None: + optval = optval.strip() + cursect[optname] = [optval] + else: + # valueless option handling + cursect[optname] = None + else: + # a non-fatal parsing error occurred. set up the + # exception but keep going. the exception will be + # raised at the end of the file and will contain a + # list of all bogus lines + e = self._handle_error(e, fpname, lineno, line) + finally: + self._join_multiline_values() # if any parsing errors occurred, raise an exception if e: raise e diff --git a/Lib/test/test_configparser.py b/Lib/test/test_configparser.py index 5d58e34..6340e37 100644 --- a/Lib/test/test_configparser.py +++ b/Lib/test/test_configparser.py @@ -646,6 +646,21 @@ boolean {0[0]} NO "'opt' in section 'Bar' already exists") self.assertEqual(e.args, ("Bar", "opt", "", None)) + def test_get_after_duplicate_option_error(self): + cf = self.newconfig() + ini = textwrap.dedent("""\ + [Foo] + x{equals}1 + y{equals}2 + y{equals}3 + """.format(equals=self.delimiters[0])) + if self.strict: + with self.assertRaises(configparser.DuplicateOptionError): + cf.read_string(ini) + else: + cf.read_string(ini) + self.assertEqual(cf.get('Foo', 'x'), '1') + def test_write(self): config_string = ( "[Long Line]\n" diff --git a/Misc/NEWS.d/next/Library/2024-03-18-14-36-50.gh-issue-116957.dTCs4f.rst b/Misc/NEWS.d/next/Library/2024-03-18-14-36-50.gh-issue-116957.dTCs4f.rst new file mode 100644 index 0000000..51fe049 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-03-18-14-36-50.gh-issue-116957.dTCs4f.rst @@ -0,0 +1,3 @@ +configparser: Don't leave ConfigParser values in an invalid state (stored as +a list instead of a str) after an earlier read raised DuplicateSectionError +or DuplicateOptionError. -- cgit v0.12