Conversation
…iterals in MQL exports
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
An MQL export is a
.jsfile that a user replays inmongoshor 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:// Database:and// Collection:headers went throughsanitizeForSQLComment, 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 everyCharacter.isNewline, which adds VT, FF and U+0085, so the scanner and JavaScriptCore also disagreed about where a comment ended.fetchTableDDLtext into the file. On main that text writes the header, thecollModname and each index name exactly as the server returned them, so the export was only as safe as the installed driver's DDL.getCollectionaccessor) went throughescapeJSONString, 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 wrotedb["…"], escaping only\and".Fix
TableProJavaScriptText, provideslineBreakingEscape,stringLiteral,lineCommentandisPlainIdentifier. It uses the same spelling asMongoScriptJson.lineBreakingEscapein fix(plugin-mongodb): list views as views, keep index key order and options, and add db.createView to the shell #3150: lowercase\uhex with\n,\rand\tshort forms, covering C0, DEL, C1, U+2028 and U+2029.stringLiteralworks on UTF-8 bytes, and a test checks it against the scalar rule for every BMP character.MQLCollectionDefinition, with a small lexer and literal reader). It writes back only two shapes: this collection'screateIndex(keys[, options]), and acollModthat 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 validatorstands 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.db.runCommand(...)an index name had spelled inside it and wrote it out as a real validator.plugin-mql-v1.0.7in the registry.selectWinnerskeeps 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.MQLExportHelpers, all built on the package target. The export decides between baredb.<name>anddb.getCollection(...)itself: an ASCII identifier that is not adbmember goes bare. That way a plugin release for an app that already shipped does not inherit that app's olderMongoCollectionAccessor. A nested document thatJSONSerializationaccepted keeps its text, with separators inside its strings escaped.PluginExportUtilities.escapeJSONStringgot 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 andMongoCollectionAccessorwith the next app release.QueryTab.buildBaseTableQuerynow usesMongoCollectionAccessor.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.shcan publish the export fix for shipped apps.Verified
swift test --package-path Packages/TableProCore: exit 0.JavaScriptTextTestspassed 8/8 andPluginExportUtilitiesTestspassed 6/6. The agreement test covers all 63,488 BMP scalars. Main'sescapeJSONString, compiled standalone, leaves U+0085, U+2028 and U+2029 raw, so the new separator test fails there.verify.sh testover MQLExportHelpersTests, MQLCollectionDefinitionTests, QueryTabBaseQueryTests, MongoDBQueryBuilderTests, PluginRowWritersTests, ExtractTableNameTests, QueryExecutorTests and PluginExportUtilitiesTests, rebased onmainatc21dc512e: 206 executed, 206 passed.sanitizeForSQLCommentand restored the olddb["…"]fallback. 2 of 27 cases failed (headerCommentStaysOnOneLine,javascriptQueryUsesSharedAccessor).verify.sh buildpassed, andverify.sh pluginspassed for all 40 bundles.swiftlint --stricton the 14 touched files found 0 violations.verify.sh docspassed.verify.sh abi 4440f0b07reported "PluginKit ABI unchanged". The Repo Hygiene scripts all passed locally.swiftc -O, 200k cells of about 60 bytes: the oldescapeJSONStringtook 252-308 ms and the new byte loop 211-300 ms.db.runCommandinto the export. And the bundle version stayed at 1.0, below the registry's 1.0.7. New testsstatementInsideAFailedStatementIsLeftOut,validatorAfterAnIndexIsLeftOutandstatementSharingALineIsLeftOutfail 3 of 13 against the previous parser and pass against this one.MQLCollectionDefinitionTests,MQLExportHelpersTestsandPluginExportUtilitiesTests: 36/36. The builtMQLExport.tablepluginreportsCFBundleShortVersionString1.0.8.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 passedJSCheckScriptSyntax. Every emitted string literal round-tripped to the original name throughJSONDecoderand JavaScriptCore'sJSON.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
MongoDBQueryBuilder.escapeJsonString, the one inMongoDBStatementGenerator,MongoDBPluginDriver.escapeJsonStringand NumberText's JSON writer. fix(plugin-mongodb): list views as views, keep index key order and options, and add db.createView to the shell #3150 covers the driver's text withMongoScriptJson.lineBreakingEscape, and Can't create a database collection from the visual editor #3131 ownsMongoDBStatementGenerator. All of them can move ontoTableProJavaScriptTextlater.fetchTableDDLstill writes names raw in Show DDL and Copy DDL, and the live check showed it dropsexpireAfterSecondsfrom a TTL index. fix(plugin-mongodb): list views as views, keep index key order and options, and add db.createView to the shell #3150 replaces that builder and quotes every name it writes. The export no longer depends on either, and fix(plugin-mongodb): list views as views, keep index key order and options, and add db.createView to the shell #3150's layout (validator first, one statement per line) still reads.createIndexfor this collection on a line of its own, and nothing in the text tells it apart from a real one. It can no longer spell a validator or any other command.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.