fix: reject dialect quote types that would match zero-length tokens - #976
Open
Jeremy-xuan wants to merge 1 commit into
Open
Jeremy-xuan wants to merge 1 commit into
Jeremy-xuan wants to merge 1 commit into
Conversation
A token rule that matches the empty string never advances the tokenizer, so formatDialect() looped forever without producing any output or error when a custom dialect had a quote type list that resolved to no pattern at all. Two layers: - validateQuoteTypes() in createDialect() rejects the configurations that produce this: an empty stringTypes/identTypes/variableTypes list, an empty regex, and a quote type name that has no entry in quotePatterns. This follows the check added for paramTypes in sql-formatter-org#754. - TokenizerEngine.match() refuses a zero-length match as a last resort, so a future rule that can match nothing fails loudly instead of hanging. sql-formatter-org#754 noted that not every regex that matches the empty string can be enumerated up front, which is what this covers. The 20 built-in dialects are unaffected: they all have non-empty lists with known names.
This branch has not been deployed
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
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.
fix: reject dialect quote types that would match zero-length tokens
What's wrong
formatDialect()with a custom dialect hangs forever — no error, no output, one coreat 100% — when the dialect's quote type list resolves to no pattern at all:
Three fields are affected —
stringTypes,identTypes,variableTypes— and for eachof them two different inputs do it:
[]), which reads as "this dialect has no such quotes";"''"or'""'—quotePatternskeys are"''-qq","''-bs","''-raw",'""-qq'and so on, so a plain"''"looks upundefined;{ regex: '' }), the case Supplying empty regex crashes browser #754 already fixed forparamTypes.Nothing in the 20 built-in dialects hits this — they all have non-empty lists with
known names — so it only affects custom dialects.
Root cause
regexFactory.stringPattern()maps the quote types to patterns and joins them:An unknown name yields
undefined, and[undefined].join('|')is the empty string(exactly as an empty array is), so the whole rule becomes
new RegExp('(?:)', 'uy').That matches the empty string —
exec('SELECT')returns['']withlastIndexstill0 — and
TokenizerEngineadvances bymatchedText.length, which is 0, so thewhile (this.index < this.input.length)loop never moves.variableTypesalready has a guard, but it only coversundefined, and[]is truthy,so an empty array slips through it. There was no equivalent guard at all for
stringTypesandidentTypes.The fix
Two layers, because the two failure modes need different treatment.
1. Reject the configuration in
createDialect()(newvalidateQuoteTypes()):Empty stringTypes given for dialect "x". That would result in matching zero-length tokens.Unknown quote type "''" given in stringTypes of dialect "x". Known ones are: `` , [], ""-qq, …(the message lists the valid names, so the caller can fix the typo){ regex: '' }→Empty regex given in stringTypes of dialect "x". …This follows
validateParamTypes()from #754 — same idea, same error type, same shapeof message. Where #754 checked one field, this checks the three fields that take quote
types.
2. Refuse a zero-length match in
TokenizerEngine.match(), as the last line ofdefence:
#754's closing comment notes that "there are infinitely many regexes that would end up
matching empty string (for example
(|)) and therefore triggering an infinite loop.It's not really feasible to properly validate them all." That is true, so this does not
try to enumerate them: it refuses the consequence. Any future rule that cannot match
anything now fails loudly with the dialect name and position instead of hanging.
Tests
Four tests in
test/sqlFormatter.test.ts, next to the existing custom-dialect tests:an empty list (all three fields), an unknown name (plain and with prefixes), an empty
regex, and a valid custom dialect that must keep working. Three fail on
master. Thethird one — the empty list — makes the test process hang on
masterrather thanfail, which is the bug itself.
All 20 built-in dialects pass the new validation, which is what the unchanged 5846
baseline tests confirm.