diff --git a/benchmarks/bench_reindent_offset.py b/benchmarks/bench_reindent_offset.py index 720a9592..fa30448b 100644 --- a/benchmarks/bench_reindent_offset.py +++ b/benchmarks/bench_reindent_offset.py @@ -12,8 +12,10 @@ Both shapes reach the same offset calculation through different filters: ``IN (...)`` tuple lists via ``_process_parenthesis()`` and ``_process_identifierlist()``, ``VALUES`` lists via ``_process_values()``. -Sizes stay below the grouping-token cap; larger input is rejected by the -cap instead of reaching the measured path. + +The first two vectors stay below the grouping-token cap, which bounds both +what an attacker can reach and the exponent they can show; a third runs the +same shape with the guards lifted, where the growth is unobscured. Run with: python benchmarks/bench_reindent_offset.py """ @@ -23,6 +25,12 @@ from _harness import Vector, main import sqlparse +from sqlparse.engine import grouping + +# Only the third vector needs these lifted; the other two are sized to stay +# below the cap (as an attacker facing the defaults would also have to). +grouping.MAX_GROUPING_DEPTH = None +grouping.MAX_GROUPING_TOKENS = None def in_tuple_list(n): @@ -44,6 +52,7 @@ def reindent(sql): VECTORS = [ Vector('IN-tuple list', in_tuple_list, (150, 300, 600, 1200), reindent), Vector('VALUES list', values_list, (250, 500, 1000, 1950), reindent), + Vector('VALUES list, no guards', values_list, (1000, 2000, 4000, 8000), reindent), ] if __name__ == '__main__': diff --git a/sqlparse/engine/grouping.py b/sqlparse/engine/grouping.py index d8cfa9e1..868c320a 100644 --- a/sqlparse/engine/grouping.py +++ b/sqlparse/engine/grouping.py @@ -335,10 +335,8 @@ def group_comments(tlist): eidx, end = tlist.token_not_matching( lambda tk: imt(tk, t=T.Comment) or tk.is_newline, idx=tidx) if end is None: - # From tidx onward everything is comment/newline: there is no - # terminator to group against, and every later start would hit - # the same dead end. Stop instead of re-scanning the tail once - # per remaining comment token (which is O(n**2)). + # Everything from tidx on is comment/newline, so every later + # start hits the same dead end; continuing rescans it each time. break eidx, end = tlist.token_prev(eidx, skip_ws=False) tlist.group_tokens(sql.Comment, tidx, eidx) @@ -353,12 +351,13 @@ def group_where(tlist): eidx, end = tlist.token_next_by(m=sql.Where.M_CLOSE, idx=tidx) if end is None: - end = tlist._groupable_tokens[-1] + # _groupable_tokens drops the enclosing pair for Parenthesis and + # SquareBrackets, so its last token needn't be the last one here. + eidx = tlist.token_index(tlist._groupable_tokens[-1], tidx) else: - end = tlist.tokens[eidx - 1] - # TODO: convert this to eidx instead of end token. - # i think above values are len(tlist) and eidx-1 - eidx = tlist.token_index(end) + # Re-deriving this by scanning for the token was quadratic in the + # number of WHERE clauses sharing a statement. + eidx -= 1 tlist.group_tokens(sql.Where, tidx, eidx) tidx, token = tlist.token_next_by(m=sql.Where.M_OPEN, idx=tidx) diff --git a/sqlparse/filters/reindent.py b/sqlparse/filters/reindent.py index f8f64532..5e95263d 100644 --- a/sqlparse/filters/reindent.py +++ b/sqlparse/filters/reindent.py @@ -31,27 +31,36 @@ def __init__(self, width=2, char=' ', wrap_after=0, n='\n', def leading_ws(self): return self.offset + self.indent * self.width - def _current_line_len(self, token): + def _current_line_len(self, token, idx=None): """Returns the width of what's already emitted on *token*'s line. - The tokens preceding *token* are visited last one first, so the walk - stops at the line break that starts the current line. Rebuilding the - statement prefix from its start instead made every caller - O(statement), and the callers running once per group (tuple lists, - identifier lists) quadratic in the number of groups -- a CPU - exhaustion vector (GHSA-cfqr-cjx5-5jcm). The walk is inlined and - counts characters rather than collecting them: both matter, since a - line without any break still has to be measured token by token. + Walks back to the nearest line break rather than rebuilding the + statement prefix from its start, which was O(statement) per call and + so quadratic over the callers that run once per group -- a CPU + exhaustion vector (GHSA-cfqr-cjx5-5jcm). + + *idx* is ``token``'s position in its parent, for callers that already + know it; without it an O(position) identity scan recovers it. """ length = 0 node = token while node is not self._curr_stmt and node.parent is not None: parent = node.parent - # ``Token`` doesn't implement ``__eq__``, so ``index()`` is an - # identity lookup and safe against tokens sharing a value. - stack = parent.tokens[:parent.tokens.index(node)] - while stack: - prev_ = stack.pop() + siblings = parent.tokens + if idx is None: + # ``Token`` doesn't implement ``__eq__``, so ``index()`` is an + # identity lookup and safe against tokens sharing a value. + idx = siblings.index(node) + + # Right to left, groups expanded as reached, so a break in the + # first token examined costs one token rather than O(idx). + stack = [] + while stack or idx: + if stack: + prev_ = stack.pop() + else: + idx -= 1 + prev_ = siblings[idx] if prev_.is_group: stack.extend(prev_.tokens) continue @@ -62,9 +71,6 @@ def _current_line_len(self, token): continue lines = value.splitlines() if len(lines) == 1 and len(lines[0]) == size: - # No break in here. ``splitlines()`` hands back the value - # itself in that case, so this costs a scan but no copy -- - # which is what keeps a long break-free line affordable. length += size continue @@ -75,17 +81,19 @@ def _current_line_len(self, token): tail = len(lines[-1]) - 1 + length if tail: return tail - # Nothing but a break to our right, and ``splitlines()`` drops - # that empty line -- so the line to measure is the one before. + # Nothing but a break to our right, whose empty line + # ``splitlines()`` drops: measure the one before it. if len(lines) > 2: return len(lines[-2]) length = len(lines[0]) node = parent + idx = None # only ever applied to token's own parent return length - def _get_offset(self, token): + def _get_offset(self, token, idx=None): # Now take current offset into account and return relative offset. - return self._current_line_len(token) - len(self.char * self.leading_ws) + return (self._current_line_len(token, idx) + - len(self.char * self.leading_ws)) def nl(self, offset=0): return sql.Token( @@ -261,13 +269,15 @@ def _process_values(self, tlist): ptidx, ptoken = tlist.token_next_by(m=(T.Punctuation, ','), idx=tidx) if ptoken: + # Both indexes were resolved after the previous insert, so + # passing them avoids the scans that made this quadratic. if self.comma_first: adjust = -2 offset = self._get_offset(first_token) + adjust - tlist.insert_before(ptoken, self.nl(offset)) + tlist.insert_before(ptidx, self.nl(offset)) else: - tlist.insert_after(ptoken, - self.nl(self._get_offset(token))) + tlist.insert_after(ptidx, + self.nl(self._get_offset(token, tidx))) tidx, token = tlist.token_next_by(i=sql.Parenthesis, idx=tidx) def _process_default(self, tlist, stmts=True): diff --git a/sqlparse/sql.py b/sqlparse/sql.py index ec44a6da..a5ed0f27 100644 --- a/sqlparse/sql.py +++ b/sqlparse/sql.py @@ -310,7 +310,8 @@ def matcher(tk): def token_index(self, token, start=0): """Return list index of token.""" start = start if isinstance(start, int) else self.token_index(start) - return start + self.tokens[start:].index(token) + # index() takes a start offset; slicing copied every token past it. + return self.tokens.index(token, start) def group_tokens(self, grp_cls, start, end, include_end=True, extend=False):