Skip to content

fix(editor): treat invisible characters around a statement as blank on the run path - #2729

Merged
datlechin merged 1 commit into
mainfrom
fix/run-path-invisible-whitespace
Sep 11, 2026
Merged

fix(editor): treat invisible characters around a statement as blank on the run path#2729
datlechin merged 1 commit into
mainfrom
fix/run-path-invisible-whitespace

Conversation

@datlechin

Copy link
Copy Markdown
Member

Found while investigating #2717 (PR #2724).

Root cause

The run path used three different definitions of whitespace, and they disagreed:

  • The scanner's content check and contentRange only knew space, tab, LF and CR.
  • Execution trimmed CharacterSet.whitespacesAndNewlines, which includes U+00A0, U+3000 and U+200B but not U+FEFF or U+0008.
  • QueryClassifier skipped Character.isWhitespace.

So a line holding only U+00A0 counted as a statement for the gutter but trimmed to nothing at run time: the run button showed and did nothing. And "\u{0008}SELECT 1" kept its U+0008, the leading keyword came out empty, the unknown-keyword arm answered "write", and Read-Only Safe Mode refused a plain SELECT. A leading BOM hid a DROP the same way, just in the other direction.

The review turned up two more ways the classifier could miss a write, in the same comment-skipping code this change edits:

  • It compared Characters to "\n". In Swift, CRLF is one "\r\n" Character, so -- note\r\nDROP TABLE users stripped to "" and classified as safe. A bare CR did the same. That text reaches the classifier through MCP execute_query on a read-only external connection, or through a CRLF .sql file under Read-Only Safe Mode.
  • It stripped MySQL /*! ... */ and MariaDB /*M! ... */ comments, which the server executes, so /*!50000 DROP TABLE users */ also classified as safe.

What changed

  • New StatementBlank (TablePro/Core/Utilities/SQL/StatementBlank.swift) is the one definition of blank for the run path. A character is blank when every scalar in it is Unicode whitespace, a control character, a default-ignorable code point or U+FFF9 to U+FFFB, and is not a letter. ASCII skips the property lookups.
  • Trimming works on Swift Characters from both ends, so an invisible scalar attached to a visible one stays with it: the VS16 in ❤️, flag tag characters, a ZWJ after a virama, a ZWNJ ending a Persian word. An invisible character inside a word is never removed, so SEL\u{200B}ECT 1 still reaches the server as written and stays a write.
  • Callers now go through it: the scanner's content check and content range, executable-statement trimming, statementAtCursor, allStatementsPreservingSemicolons, the JavaScript scanner's hasContent, the classifier's comment and keyword skipping, and DatabaseAccessBridge (MCP and AppleScript). The text an external client has classified is the text that is sent.
  • The two duplicate .whitespacesAndNewlines guards in QueryExecutionCoordinator and MainContentCoordinator are gone. The executable-statement list is the only gate, so the gutter's run controls and execution agree by construction.
  • QueryClassifier: a line comment ends at LF, CR or CRLF in all three places that skip comments (strippingLeadingComments, strippingStringLiterals, the EXPLAIN inner-statement walk). A statement holding a conditional comment is never safe: at least a write, destructive when the revealed text has DROP or TRUNCATE, and flagged when it touches the filesystem or runs code. An opener inside a string literal or an ordinary comment stays inert.
  • DatabaseAccessBridge.runStatement refuses a statement that trims to nothing ("The query is empty.", already in the catalog) before it opens a connection.
  • Docs: docs/features/sql-editor.mdx says invisible characters around a statement are not sent and a line of them gets no run button. docs/features/safe-mode.mdx says a conditional comment makes a statement count as a write.

Verification

  • Build: verify.sh build passed, .analysis/fix-run-path-invisible-whitespace/logs/build-TablePro-161542.log.
  • Tests with the fix: 481 cases executed, 481 passed, 0 failed across 32 suites: StatementBlankTests, ten QueryClassifier* suites (including the new QueryClassifierCommentBoundaryTests and QueryClassifierInvisibleCharacterTests), SQLExecutableStatementTests, SQLStatementNavigationTests, SQLStatementScannerTests, SQLStatementBlockSplittingTests, SQLStatementRangeTests, SQLStatementScannerLocatedTests, StatementAnchorTests, StatementNavigationCommandTests, StatementRunPerformanceGuardTests, StatementRunControllerTests, JavaScriptStatementScannerTests, QueryStatementModelTests, DatabaseAccessBridgeStatementTests, ExternalStatementGateTests, ExecutionGateTests, MCPStatementGateConsentPolicyTests, MCPStatementGateRefusalTests, ConfirmDestructiveOperationToolTests, ExportDataToolStatementTests, ExplainResultRouterTests, StringCatalogIntegrityTests. Log: .analysis/fix-run-path-invisible-whitespace/logs/test-StatementBlankTests-161621.log.
  • Tests without the fix, with the source changes reverted in the worktree:
    • First round (invisible characters): 47 of 138 cases failed across the scanner, navigation, statement model, classifier, gate and bridge suites. Log: test-SQLExecutableStatementTests-152352.log.
    • Second round (CRLF comments, conditional comments, empty external statement, ZWNJ): 20 cases failed across QueryClassifierCommentBoundaryTests, ExternalStatementGateTests, DatabaseAccessBridgeStatementTests and SQLExecutableStatementTests. Log: test-QueryClassifierCommentBoundaryTests-161227.log.
  • Performance, measured with a standalone copy of the trimming code: the content range of a statement wrapped in 4,000,000 ASCII spaces takes about 100 ms with the ASCII fast path, where the reviewer measured 199 ms for half that many before it. 2,000,000 non-ASCII blanks (U+00A0, U+3000) take 115 to 200 ms, and 20,000 ordinary statements take 7 ms in total. Only long blank runs cost anything, and they are rare in real scripts.
  • SwiftLint --strict on the changed files: no violations on added lines. The two hits are on existing lines 12 and 16 of SQLExecutableStatementTests.swift.
  • scripts/localization.py verify: ok. Docs check-writing-style.sh and check-docs-against-source.py: pass.
  • Package tests: not applicable, LocalPackages/CodeEditTextView is untouched.
  • UI tests: none. The behaviour is classification and text trimming, and the unit suites cover the gutter parity, the executed text and spans, the classifier tiers and the Safe Mode gate decisions. A UI test would have to put U+0008 or U+00A0 into the editor, but fix(editor): reveal invisible characters and drop typed control characters (#2717) #2724 drops typed control characters and XCUITest cannot paste one reliably, so it would not run deterministically.

Notes

Codex was unavailable, so an adversarial reviewer agent read the diff instead. It raised four findings:

  1. CRLF and conditional comments hiding a DROP from the classifier (material, existing before this change). Fixed as described above.
  2. Trimming split on NSString composed sequences, which do not attach a ZWNJ, so a ZWNJ ending a word was stripped. Fixed: trimming now works on Swift Characters, and a ZWNJ case is in the tests.
  3. Run and Execute stay enabled for a tab holding only invisible characters, and pressing them now does nothing. Partly taken: the external bridge now refuses an empty statement. The enablement change was left out. Execute has no text gate and the menu only checks isEmpty, so a tab of only spaces or only comments already ran nothing through those controls before this change. Giving them a runnable-statement signal is a separate change to how those controls validate.
  4. Four Unicode property lookups per blank character on the main thread. Fixed with the ASCII fast path.

Known trade-offs, all in the safe direction:

  • SELECT /*!40001 SQL_NO_CACHE */ * FROM t is now a write, so Read-Only Safe Mode and read-only MCP connections refuse it. The Safe Mode page says so and tells the user to remove the comment.
  • A bare CR now ends a line comment for the classifier on every engine. MySQL and SQLite end comments only at LF, so text after a bare CR there may be over-classified. It is never under-classified.
  • A NUL at the very start or end of a statement is now trimmed and the statement runs. A NUL inside a statement is still refused, and the docs sentence says that.
  • The Safe Mode section heading changed to "What always counts as a write". No page links to the old anchor.

https://claude.ai/code/session_011THKc9TRHE8xjcxXidDHXP

@mintlify

mintlify Bot commented Sep 11, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 11, 2026, 9:30 AM

💡 Tip: Enable Automations to automatically generate PRs for you.

@datlechin
datlechin merged commit 5356343 into main Sep 11, 2026
12 of 14 checks passed
@datlechin
datlechin deleted the fix/run-path-invisible-whitespace branch September 11, 2026 13:03
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