Don't re-derive token positions callers already know

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__':

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)
 

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):

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):