From a5a754f75e887da38b67cc579f5917d0d92e3a37 Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Thu, 15 Oct 2026 07:19:04 +0300 Subject: [PATCH] perf(parser): scan SQL for comments and strings with code units stripCommentsAndStrings compiled a RegExp and copied the rest of the script (sql.substring(i)) at every $, including $1 / $2 parameters, and indexed the text one-character string at a time. The loop now reads code units, with _skipLineComment, _skipBlockComment, _skipDollarQuote and _skipStringLiteral returning the next offset, and appends the untouched text in runs. The WHERE and SET regexes are static. The output is the same; a test compares it with the previous implementation on hand-written and random scripts. Closes #1511 --- .../database/destructive_sql_detector.dart | 174 ++++++++++++------ .../database/destructive_sql_strip_test.dart | 143 ++++++++++++++ 2 files changed, 256 insertions(+), 61 deletions(-) create mode 100644 test/core/database/destructive_sql_strip_test.dart diff --git a/lib/core/database/destructive_sql_detector.dart b/lib/core/database/destructive_sql_detector.dart index 8684dcdc..cbb94996 100644 --- a/lib/core/database/destructive_sql_detector.dart +++ b/lib/core/database/destructive_sql_detector.dart @@ -190,6 +190,9 @@ abstract final class DestructiveSqlDetector { caseSensitive: false, ); + static final _whereRegex = RegExp(r'\bWHERE\b', caseSensitive: false); + static final _setRegex = RegExp(r'\bSET\b', caseSensitive: false); + /// True when [sanitized] has a WHERE outside any parentheses, so a WHERE /// inside a subquery (`SET x = (SELECT ... WHERE ...)`) does not count. static bool _hasTopLevelWhere(String sanitized) { @@ -205,84 +208,133 @@ abstract final class DestructiveSqlDetector { top.write(ch); } } - return RegExp(r'\bWHERE\b', caseSensitive: false).hasMatch(top.toString()); + return _whereRegex.hasMatch(top.toString()); + } + + static const int _chLf = 0x0A; + static const int _chCr = 0x0D; + static const int _chQuote = 0x27; // ' + static const int _chDollar = 0x24; // $ + static const int _chStar = 0x2A; // * + static const int _chSlash = 0x2F; // / + static const int _chDash = 0x2D; // - + static const int _chBackslash = 0x5C; // \ + + static bool _isTagChar(int c) => + (c >= 0x30 && c <= 0x39) || // 0-9 + (c >= 0x41 && c <= 0x5A) || // A-Z + (c >= 0x61 && c <= 0x7A) || // a-z + c == 0x5F; // _ + + /// Offset of the end of the line comment whose body starts at [start]: the + /// line break itself is not consumed. + static int _skipLineComment(String sql, int start, int len) { + var i = start; + while (i < len) { + final c = sql.codeUnitAt(i); + if (c == _chLf || c == _chCr) break; + i++; + } + return i; + } + + /// Offset after the `*/` that closes the block comment whose body starts at + /// [start]; [len] when it is never closed. + static int _skipBlockComment(String sql, int start, int len) { + var i = start; + while (i + 1 < len) { + if (sql.codeUnitAt(i) == _chStar && sql.codeUnitAt(i + 1) == _chSlash) { + return i + 2; + } + i++; + } + return len; + } + + /// Offset after the dollar-quoted string (`$$...$$`, `$tag$...$tag$`) that + /// opens at [start], [len] when it is never closed, or [start] when there is + /// no opening tag at [start] (`$1`, a lone `$`). + static int _skipDollarQuote(String sql, int start, int len) { + var j = start + 1; + while (j < len && _isTagChar(sql.codeUnitAt(j))) { + j++; + } + if (j >= len || sql.codeUnitAt(j) != _chDollar) return start; + final bodyStart = j + 1; + final close = sql.indexOf(sql.substring(start, bodyStart), bodyStart); + return close == -1 ? len : close + (bodyStart - start); + } + + /// Offset after the single-quoted literal whose body starts at [start] + /// (`''` and backslash escapes); [len] when it is never closed. + static int _skipStringLiteral(String sql, int start, int len) { + var i = start; + while (i < len) { + final c = sql.codeUnitAt(i); + if (c == _chQuote) { + if (i + 1 < len && sql.codeUnitAt(i + 1) == _chQuote) { + i += 2; // escaped quote + } else { + return i + 1; // closing quote + } + } else if (c == _chBackslash && i + 1 < len) { + i += 2; // escaped char + } else { + i++; + } + } + return len; } /// Strips comments and string literals to prevent false positives when keywords /// appear inside strings or comments. + /// + /// Scans code units with the `_skip*` helpers (no per-character strings, no + /// regex, no copy of the rest of the script at every `$`), and appends the + /// text between tokens in runs (#1511). static String stripCommentsAndStrings(String sql) { final buffer = StringBuffer(); final len = sql.length; var i = 0; + var runStart = 0; - while (i < len) { - // 1. Line comment: -- - if (i + 1 < len && sql[i] == '-' && sql[i + 1] == '-') { - i += 2; - while (i < len && sql[i] != '\n' && sql[i] != '\r') { - i++; - } - buffer.write(' '); - continue; - } + void flushRun(int end) { + if (end > runStart) buffer.write(sql.substring(runStart, end)); + } - // 2. Block comment: /* ... */ - if (i + 1 < len && sql[i] == '/' && sql[i + 1] == '*') { - i += 2; - while (i + 1 < len && !(sql[i] == '*' && sql[i + 1] == '/')) { + while (i < len) { + final c = sql.codeUnitAt(i); + final next = i + 1 < len ? sql.codeUnitAt(i + 1) : -1; + int after; + String? replacement; + + if (c == _chDash && next == _chDash) { + after = _skipLineComment(sql, i + 2, len); + replacement = ' '; + } else if (c == _chSlash && next == _chStar) { + after = _skipBlockComment(sql, i + 2, len); + replacement = ' '; + } else if (c == _chDollar) { + after = _skipDollarQuote(sql, i, len); + if (after == i) { i++; - } - if (i + 1 < len) { - i += 2; // skip */ - } else { - i = len; - } - buffer.write(' '); - continue; - } - - // 3. Dollar quotes in PostgreSQL: $$ or $tag$ - if (sql[i] == '\$') { - final match = - RegExp(r'^\$([a-zA-Z0-9_]*)\$').matchAsPrefix(sql.substring(i)); - if (match != null) { - final tag = match.group(0)!; - i += tag.length; - final closeIdx = sql.indexOf(tag, i); - if (closeIdx != -1) { - i = closeIdx + tag.length; - } else { - i = len; - } - buffer.write("''"); continue; } - } - - // 4. Standard string literal: '...' (supporting '' escaping) - if (sql[i] == "'") { + replacement = "''"; + } else if (c == _chQuote) { + after = _skipStringLiteral(sql, i + 1, len); + replacement = "''"; + } else { i++; - while (i < len) { - if (sql[i] == "'") { - if (i + 1 < len && sql[i + 1] == "'") { - i += 2; // escaped quote - } else { - i++; // closing quote - break; - } - } else if (sql[i] == '\\' && i + 1 < len) { - i += 2; // escaped char - } else { - i++; - } - } - buffer.write("''"); continue; } - buffer.write(sql[i]); - i++; + flushRun(i); + buffer.write(replacement); + i = after; + runStart = i; } + flushRun(len); return buffer.toString(); } @@ -420,7 +472,7 @@ abstract final class DestructiveSqlDetector { // 8. UPDATE table SET ... without WHERE final updateMatch = _updateRegex.firstMatch(sanitized); if (updateMatch != null && - RegExp(r'\bSET\b', caseSensitive: false).hasMatch(sanitized) && + _setRegex.hasMatch(sanitized) && !_hasTopLevelWhere(sanitized)) { final schema = updateMatch.group(1); final table = updateMatch.group(2) ?? 'table'; diff --git a/test/core/database/destructive_sql_strip_test.dart b/test/core/database/destructive_sql_strip_test.dart new file mode 100644 index 00000000..16ba3df2 --- /dev/null +++ b/test/core/database/destructive_sql_strip_test.dart @@ -0,0 +1,143 @@ +import 'dart:math'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:querya_desktop/core/database/destructive_sql_detector.dart'; + +/// `stripCommentsAndStrings` as it was before #1511 (regex + substring at each +/// `$`, one-character strings), kept as the reference the scanner must match. +String _reference(String sql) { + final buffer = StringBuffer(); + final len = sql.length; + var i = 0; + + while (i < len) { + if (i + 1 < len && sql[i] == '-' && sql[i + 1] == '-') { + i += 2; + while (i < len && sql[i] != '\n' && sql[i] != '\r') { + i++; + } + buffer.write(' '); + continue; + } + if (i + 1 < len && sql[i] == '/' && sql[i + 1] == '*') { + i += 2; + while (i + 1 < len && !(sql[i] == '*' && sql[i + 1] == '/')) { + i++; + } + if (i + 1 < len) { + i += 2; + } else { + i = len; + } + buffer.write(' '); + continue; + } + if (sql[i] == '\$') { + final match = + RegExp(r'^\$([a-zA-Z0-9_]*)\$').matchAsPrefix(sql.substring(i)); + if (match != null) { + final tag = match.group(0)!; + i += tag.length; + final closeIdx = sql.indexOf(tag, i); + if (closeIdx != -1) { + i = closeIdx + tag.length; + } else { + i = len; + } + buffer.write("''"); + continue; + } + } + if (sql[i] == "'") { + i++; + while (i < len) { + if (sql[i] == "'") { + if (i + 1 < len && sql[i + 1] == "'") { + i += 2; + } else { + i++; + break; + } + } else if (sql[i] == '\\' && i + 1 < len) { + i += 2; + } else { + i++; + } + } + buffer.write("''"); + continue; + } + buffer.write(sql[i]); + i++; + } + return buffer.toString(); +} + +void main() { + group('stripCommentsAndStrings scanner (#1511)', () { + test('matches the previous implementation on hand-written scripts', () { + const cases = [ + '', + 'SELECT 1', + '-- only a comment', + 'SELECT 1 -- tail\nFROM t', + 'SELECT 1 -- tail\r\nFROM t', + '/* block */ SELECT 1', + 'SELECT /* unterminated', + 'SELECT /* a */ 1 /* b */', + '/**/SELECT/***/1', + "SELECT 'it''s' FROM t", + "SELECT 'a\\'b' FROM t", + "SELECT 'unterminated", + "SELECT 'x\\", + r'SELECT $$ DROP TABLE x; $$ FROM t', + r'SELECT $fn$ body $fn$ , $1, $2', + r'SELECT $a$ never closed', + r'SELECT $ alone, $1, $_x$ one $_x$', + r'SELECT $$$$', + r"DO $$ BEGIN RAISE NOTICE 'x'; END $$;", + "DELETE FROM t WHERE a = 'WHERE' -- WHERE\n", + '-', + '/', + '\$', + "'", + 'aé\u{1F600}b -- \u{1F600}\n\'\u{1F600}\'', + ]; + for (final sql in cases) { + expect( + DestructiveSqlDetector.stripCommentsAndStrings(sql), + _reference(sql), + reason: sql, + ); + } + }); + + test('matches the previous implementation on random SQL-like text', () { + const pieces = [ + "'", "''", '"', '--', '/*', '*/', '\n', '\r\n', ' ', ';', r'$', r'$$', + r'$a$', r'$tag$', r'$1', r'\', 'SELECT', 'DROP', 'WHERE', 'x', '1', + '(', ')', 'é', '\u{1F600}', '-', '/', '*', + ]; + final random = Random(1511); + for (var n = 0; n < 3000; n++) { + final sql = [ + for (var k = random.nextInt(30); k > 0; k--) + pieces[random.nextInt(pieces.length)], + ].join(); + expect( + DestructiveSqlDetector.stripCommentsAndStrings(sql), + _reference(sql), + reason: sql, + ); + } + }); + + test('a large script with many parameters is handled', () { + final sql = List.generate(20000, (i) => 'SELECT \$1, \$2 -- row $i') + .join('\n'); + final out = DestructiveSqlDetector.stripCommentsAndStrings(sql); + expect(out, _reference(sql)); + expect(out, isNot(contains('row'))); + }); + }); +}