fix(configparser): restore space delimiter splitting option/value - #156375
fix(configparser): restore space delimiter splitting option/value#156375shoemoney wants to merge 2 commits into
Conversation
Sorry, something went wrong.
Fix verified RED->GREEN. configparser regression: space delimiter no longer splits option/value at Lib/configparser.py:618 - delimiters=( , =) parsing foo bar=baz yields option foo bar instead of foo, new regex greedily consumes \s+word as part of option
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead. |
Sorry, something went wrong.
|
The following commit authors need to sign the Contributor License Agreement: |
Sorry, something went wrong.
| if allow_no_value: | ||
| if any(dl.strip() == "" for dl in delimiters): | ||
| if allow_no_value: | ||
| self._optcre = re.compile( |
There was a problem hiding this comment.
Why not fix the pattern that was broken, instead of adding yet another one?
Sorry, something went wrong.
There was a problem hiding this comment.
Good question. The broken pattern is _OPT_TMPL/_OPT_NV_TMPL where (?:\s+(?:(?!{delim})\S)+)* greedily treats space-separated tokens as part of option. When space itself is a delimiter (delimiters=(' ', '=')) that continuation should not apply, the delimiter is the word boundary not part of option. A single regex that handles both would need a conditional inside the pattern on whether delimiter contains whitespace, which is the same branching but hidden inside the regex and harder to read plus risky for the ReDoS-safe backtracking the original fix (PR 146399) added. This keeps two small patterns: single-token option for whitespace delimiters, multi-word for =/:. Happy to unify into one template with a conditional if you prefer.
Sorry, something went wrong.
|
CLA: The commit author jeremy@shoemoney.com (@shoemoney) needs to sign the Python CLA at https://cla.python.org/ . This is not auto-signable, the author must authenticate via GitHub at that URL and sign. The CLA check is pre-existing and not introduced by this PR. Once signed the bot will re-check. |
Sorry, something went wrong.
|
Reply to @StanFromIreland on why a second pattern: the issue is the (\s+word)* continuation in _OPT_TMPL assumes space is not a delimiter. When space is a delimiter, that continuation consumes bar as option instead of stopping at the delimiter. A fix inside the same regex would need a whitespace-delimiter conditional anyway, same branch but obscured inside the regex and entangled with the ReDoS-safe backtracking from PR 146399. This keeps single-token for whitespace delimiters and multi-word for =/: separate, minimal, and reviewable. Can unify if you would prefer a single templated pattern with a comment. |
Sorry, something went wrong.
|
Note: bedevere/issue-number still fails because this fix has no linked issue number. A maintainer can apply the skip issue label if this is intentional as a regression fix for PR 146399, or retitle with gh-156375 if an issue is filed. |
Sorry, something went wrong.
Fixes regression where space delimiter no longer splits option/value.
Bug: with delimiters=(' ', '=') parsing "foo bar=baz" yields option "foo bar" instead of "foo". The new regex from PR 146399 greedily consumes \s+word as part of option.
Fix: detect whitespace delimiters and use single-token option pattern for that case, preserving ReDoS protection for normal delimiters while restoring correct split. Multi-word options still work when delimiter is "=" or ":".
Evidence: RED->GREEN verified: before fix "foo bar=baz" -> option "foo bar", after fix -> option "foo", value "bar=baz". Normal case "foo bar = baz" with (=,:) still yields "foo bar".
Written in conjunction with my pair programmer Claude.