Guard case/parenthesis alignment against malformed input - #887
Open
afonsojanu wants to merge 1 commit into
Open
Conversation
reindent_aligned=True and strip_whitespace=True could both crash on certain malformed SQL where grouping doesn't produce the shape these filters expect. For CASE without a matching END as a direct child (it can end up nested inside a sibling group instead), _process_case would grab a None token and hand it to insert_before, which blew up with ValueError: None is not in list. For a parenthesis whose contents collapse into a single nested group during parsing (e.g. "( AS )"), _stripws_parenthesis assumed at least two direct children and raised IndexError on tokens[1]. Both filters now bail out gracefully instead of assuming a well-formed tree, with regression tests for each case using the exact reproducers from the two issues.
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.
Two separate crash reports (#885, #886) both trace back to the same kind of problem: the aligned-indent and strip-whitespace filters assume a well-formed token tree, and a couple of malformed inputs break that assumption in slightly different ways.
For
strip_whitespace=Trueon( AS )(#885): parsing this collapses the whole parenthesis body into a single nested group, so theParenthesistoken list ends up with only one direct child instead of the usual opening paren / content / closing paren shape._stripws_parenthesisindexed straight intotokens[1], which doesn't exist there, raisingIndexError.For
reindent_aligned=Trueon a malformedCASE ... END(#886): whenENDisn't a direct child of theCasetoken list (it can end up nested a couple of levels down inside a sibling group),token_next_bycan't find it and returnsNone. ThatNonethen gets passed straight intoinsert_before, which tries to look up its position in the token list and raisesValueError: None is not in list.Fixed both by having the filters check their assumptions instead of trusting them:
_stripws_parenthesisonly pops surrounding whitespace when there's actually room for it, and_process_casebails out early if it can't locate a realENDtoken to align against. Neither fix changes behavior for well-formed SQL, only what happens on inputs that don't have a proper structure to begin with.Added a regression test for each, using the exact reproducers from both issues, confirmed they fail on master and pass with the fix. Ran the full suite locally, 508 passed (plus the 2 pre-existing xfails and 1 xpass, untouched by this change).
Closes #885, closes #886