Stop parse_key from splitting a compound key on an escaped quote - #356
Open
afonsojanu wants to merge 1 commit into
Open
Stop parse_key from splitting a compound key on an escaped quote#356afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
ConfigTree.parse_key splits a raw key string into path segments with a regex that looks for a quoted run or a plain run of non-special characters. The quoted alternative only matched non-quote characters between the delimiters, so a key like "\"b" (an escaped quote followed by a letter) got cut at the escaped quote instead of treating the whole thing as one segment: the regex saw a quote, one backslash, then another quote and called that a complete (empty) quoted token, leaving the rest of the string to be picked up separately. In practice this meant a key such as "\"b" = 1 silently turned into a two-level nested tree with a backslash key on top, instead of a single key holding the unescaped value "b. Made the quoted alternative escape-aware, matching the same shape the tokenizer's own quoted-string regex already uses elsewhere in this file, and added proper unescaping (quote and backslash) for whatever comes out of it, reusing the same replacement table the value parser uses. Kept the "one or more" requirement instead of "zero or more" so a run of adjacent quotes with nothing meaningful between them (how a triple-quoted key like """foo""" shows up here) still falls through to the plain-run branch exactly as before, rather than getting eaten as a sequence of empty quoted tokens. This is related to chimplerGH-325 but doesn't fully close it: the narrower case in that issue, a key that is nothing but a single escaped quote and no other characters, still fails earlier in the pyparsing grammar itself before parse_key ever runs, and I couldn't track that one down with confidence in the time I had. What this does fix is real and reproducible on its own, so filing it separately rather than holding it back.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While looking at #325 I found that ConfigTree.parse_key mis-splits a key whenever it contains an escaped quote followed by more content. The path-splitting regex's quoted alternative only matched non-quote characters between the delimiters, so something like
"\"b" = 1gets read as a quote, a backslash, and another quote forming a complete (empty) token, with the rest of the string picked up separately afterward. In practice that turns into a two-level tree with a literal backslash as the top key, instead of one key holding the unescaped"b.Made the quoted alternative escape-aware (same shape the tokenizer's own quoted-string regex already uses a few lines up in config_parser.py) and unescape whatever comes out of it using the same replacement table the value side already relies on. Kept it as "one or more" rather than "zero or more" so a run of bare adjacent quotes, which is how a triple-quoted key like
"""foo"""shows up here, still falls through to the plain-run branch unchanged rather than getting eaten as a string of empty quoted tokens - that combination briefly broketest_triple_quotes_keyswhile I was working through this, which is what led me to that requirement in the first place.I want to flag something honestly rather than oversell the scope: this doesn't fully close #325. The narrower case in that issue - a key that's nothing but a single escaped quote and no other characters - still fails, but earlier than parse_key: at the pyparsing grammar level, before parse_key ever gets called. I spent a while on it and couldn't pin down the mechanism with real confidence, so I'm not touching that part here. What's in this PR is a distinct, reproducible bug I could verify cleanly - the compound-key case - not a full fix for the issue's original repro.
Added a regression test (
test_quoted_key_with_escaped_quote) confirming the fix; full suite passes (287 tests, one pre-existing xfail unrelated to this).