Skip to content

fix: reject dialect quote types that would match zero-length tokens - #976

Open
Jeremy-xuan wants to merge 1 commit into
sql-formatter-org:masterfrom
Jeremy-xuan:fix/zero-length-token-rule
Open

Jeremy-xuan wants to merge 1 commit into
sql-formatter-org:masterfrom
Jeremy-xuan:fix/zero-length-token-rule

Conversation

@Jeremy-xuan

Copy link
Copy Markdown

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 core
at 100% — when the dialect's quote type list resolves to no pattern at all:

import { formatDialect, sql } from 'sql-formatter';

const dialect = { ...sql, tokenizerOptions: { ...sql.tokenizerOptions, stringTypes: [] } };
formatDialect('SELECT 1;', { dialect });   // never returns

Three fields are affected — stringTypes, identTypes, variableTypes — and for each
of them two different inputs do it:

  • an empty list ([]), which reads as "this dialect has no such quotes";
  • a name that has no pattern, e.g. "''" or '""'quotePatterns keys are
    "''-qq", "''-bs", "''-raw", '""-qq' and so on, so a plain "''" looks up
    undefined;
  • an empty regex ({ regex: '' }), the case Supplying empty regex crashes browser #754 already fixed for paramTypes.

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:

quoteTypes.map(singleQuotePattern).join('|');     // [undefined].join('|') === ''

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 [''] with lastIndex still
0 — and TokenizerEngine advances by matchedText.length, which is 0, so the
while (this.index < this.input.length) loop never moves.

variableTypes already has a guard, but it only covers undefined, and [] is truthy,
so an empty array slips through it. There was no equivalent guard at all for
stringTypes and identTypes.

The fix

Two layers, because the two failure modes need different treatment.

1. Reject the configuration in createDialect() (new validateQuoteTypes()):

  • empty list → Empty stringTypes given for dialect "x". That would result in matching zero-length tokens.
  • unknown name → 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 shape
of 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 of
defence:

if (matchedText.length === 0) {
  throw new ConfigError(
    `A token rule matches the empty string at ${this.position()}, so the tokenizer would loop forever.\n${this.dialectInfo()}`
  );
}

#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. The
third one — the empty list — makes the test process hang on master rather than
fail, which is the bug itself.

pnpm test   →  Test Suites: 27 passed, 27 total
               Tests:       5850 passed, 1 skipped, 5851 total
pnpm run lint / pretty:check / ts:check / build   →  all clean

All 20 built-in dialects pass the new validation, which is what the unchanged 5846
baseline tests confirm.

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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant