Skip to content

fix(plugins): keep server-provided names inside comments and string literals in MQL exports - #3153

Open
datlechin wants to merge 1 commit into
mainfrom
fix/script-text-escaping
Open

datlechin wants to merge 1 commit into
mainfrom
fix/script-text-escaping

Conversation

@datlechin

Copy link
Copy Markdown
Member

Root cause

An MQL export is a .js file that a user replays in mongosh or opens in the editor. Three places in it wrote names chosen by the server without escaping them for where they stood, so a name could end the comment or the string literal around it early:

  • The // Database: and // Collection: headers went through sanitizeForSQLComment, which replaces LF and CR only. JavaScript also ends a // comment at U+2028 and U+2029. The editor's statement scanner ends one at every Character.isNewline, which adds VT, FF and U+0085, so the scanner and JavaScriptCore also disagreed about where a comment ended.
  • With Indexes on (the default), the export copied every line after the header of the MongoDB driver's fetchTableDDL text into the file. On main that text writes the header, the collMod name and each index name exactly as the server returned them, so the export was only as safe as the installed driver's DDL.
  • String literals (field names, values, the getCollection accessor) went through escapeJSONString, which escapes C0 only. U+0085, U+2028 and U+2029 stayed raw. JavaScript accepts them inside a string, but the editor's scanner ends a string at each one. The fallback query a JavaScript-editor tab opens with wrote db["…"], escaping only \ and ".

Fix

  • A new static package target, TableProJavaScriptText, provides lineBreakingEscape, stringLiteral, lineComment and isPlainIdentifier. It uses the same spelling as MongoScriptJson.lineBreakingEscape in fix(plugin-mongodb): list views as views, keep index key order and options, and add db.createView to the shell #3150: lowercase \u hex with \n, \r and \t short forms, covering C0, DEL, C1, U+2028 and U+2029. stringLiteral works on UTF-8 bytes, and a test checks it against the scalar rule for every BMP character.
  • MQL export now reads the driver's DDL instead of copying it (MQLCollectionDefinition, with a small lexer and literal reader). It writes back only two shapes: this collection's createIndex(keys[, options]), and a collMod that sets its validator. Both are rebuilt from the parsed values through the export's own escaping. Any other statement is left out, and // Skipped a statement that is not an index or a validator stands in its place. Comments in the DDL are written again through the comment rule. A view's DDL has no // Collection: header, so it still writes nothing.
  • The reader follows the driver's layout, because a driver that writes names raw can make a name look like a statement:
    • A statement is read only where a line starts. The driver writes one statement per line and never two on one line.
    • A statement that fails to read is left out up to the next line. Stepping through it one token at a time found a db.runCommand(...) an index name had spelled inside it and wrote it out as a real validator.
    • The validator is read only as the first statement, which is where the driver writes it. A validator after an index can only have come from a name.
  • MQL Export's bundle version is now 1.0.8, above plugin-mql-v1.0.7 in the registry. selectWinners keeps the higher version, so at the template's 1.0 a retained registry copy would have kept running the unfixed code. SQL Import does the same.
  • The headers, document field names, typed-value arguments and the collection accessor now come from pure functions in MQLExportHelpers, all built on the package target. The export decides between bare db.<name> and db.getCollection(...) itself: an ASCII identifier that is not a db member goes bare. That way a plugin release for an app that already shipped does not inherit that app's older MongoCollectionAccessor. A nested document that JSONSerialization accepted keeps its text, with separators inside its strings escaped.
  • PluginExportUtilities.escapeJSONString got a body-only change. It is still a byte loop and now also escapes U+0085, U+2028 and U+2029. There is no signature change and the ABI check reports none. It reaches JSON export, the row writers and MongoCollectionAccessor with the next app release.
  • The JavaScript fallback in QueryTab.buildBaseTableQuery now uses MongoCollectionAccessor.expression(for:).

MQL export uses only the package target plus PluginKit API that shipped in v0.75.0. Once this change is on a branch at that tag, release-plugin-for-shipped-app.sh can publish the export fix for shipped apps.

Verified

  • swift test --package-path Packages/TableProCore: exit 0. JavaScriptTextTests passed 8/8 and PluginExportUtilitiesTests passed 6/6. The agreement test covers all 63,488 BMP scalars. Main's escapeJSONString, compiled standalone, leaves U+0085, U+2028 and U+2029 raw, so the new separator test fails there.
  • verify.sh test over MQLExportHelpersTests, MQLCollectionDefinitionTests, QueryTabBaseQueryTests, MongoDBQueryBuilderTests, PluginRowWritersTests, ExtractTableNameTests, QueryExecutorTests and PluginExportUtilitiesTests, rebased on main at c21dc512e: 206 executed, 206 passed.
  • Red run: I put the header back on sanitizeForSQLComment and restored the old db["…"] fallback. 2 of 27 cases failed (headerCommentStaysOnOneLine, javascriptQueryUsesSharedAccessor).
  • verify.sh build passed, and verify.sh plugins passed for all 40 bundles. swiftlint --strict on the 14 touched files found 0 violations. verify.sh docs passed. verify.sh abi 4440f0b07 reported "PluginKit ABI unchanged". The Repo Hygiene scripts all passed locally.
  • Performance with swiftc -O, 200k cells of about 60 bytes: the old escapeJSONString took 252-308 ms and the new byte loop 211-300 ms.
  • Codex review, round 1, found two P1s, both fixed here. Stepping through a failed statement token by token let an index name promote an embedded db.runCommand into the export. And the bundle version stayed at 1.0, below the registry's 1.0.7. New tests statementInsideAFailedStatementIsLeftOut, validatorAfterAnIndexIsLeftOut and statementSharingALineIsLeftOut fail 3 of 13 against the previous parser and pass against this one. MQLCollectionDefinitionTests, MQLExportHelpersTests and PluginExportUtilitiesTests: 36/36. The built MQLExport.tableplugin reports CFBundleShortVersionString 1.0.8.
  • Live check on MongoDB 7.0.43, with a harness linking main's real fetchTableDDL. Five collections used plain names holding U+2028, U+2029, U+0085, LF or " in the collection, index and key names. Every exported script had 0 raw CR, VT, FF, NEL, LS or PS characters, re-read to itself unchanged, and passed JSCheckScriptSyntax. Every emitted string literal round-tripped to the original name through JSONDecoder and JavaScriptCore's JSON.parse. The validator and index that main wrote unescaped for the name with a quote were left out with the skip comment, and the rest were kept.

Deliberately not fixed here

No UI test: the export needs a MongoDB connection, and CI has no MongoDB server. Unit tests cover the text it writes through the pure functions it now calls.

@mintlify

mintlify Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

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

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 26, 2026, 5:19 PM

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

This branch was successfully deployed

1 active deployment
staging - docs — 06717cf9 Deployed Sep 26, 2026 by mintlify[bot]
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