From 2e6436b94466c558c45393df20ad2cedd29893d8 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 26 Sep 2026 18:58:22 +0700 Subject: [PATCH] fix(plugin-mongodb): store fields named "" and top-level $ names on insert --- CHANGELOG.md | 1 + .../MongoDBConnection+Documents.swift | 4 +- .../MongoDBConnection+ScriptHelpers.swift | 7 ++- .../MongoDocumentText.swift | 9 ---- .../MongoInsertOptions.swift | 21 ++++++++ .../MongoScriptPrelude.swift | 13 +++-- TablePro/Resources/Localizable.xcstrings | 3 -- .../MongoDB/MongoScriptPreludeTests.swift | 40 ++++++++++++++ .../Plugins/MongoDocumentTextTests.swift | 21 +++++--- .../Plugins/MongoDocumentWritePlanTests.swift | 16 ++++++ .../Plugins/MongoInsertOptionsTests.swift | 52 +++++++++++++++++++ docs/databases/mongodb.mdx | 3 ++ project.yml | 1 + 13 files changed, 168 insertions(+), 23 deletions(-) create mode 100644 Plugins/MongoDBDriverPlugin/MongoInsertOptions.swift create mode 100644 TableProTests/Plugins/MongoInsertOptionsTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index fd43cf0c69..0d57406368 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -529,6 +529,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - MongoDB collections could not be created from **New Table…**. (#3131) - A new or empty MongoDB collection showing only `_id` instead of the fields its validator declares. - MongoDB edits that stored dates and ObjectIds as text, rounded integers past 2^53, or missed a string `_id` that looks numeric. +- MongoDB inserts from the query editor and the data grid failing on a field named `""`. (#3132) - **New Table…** offered on databases that cannot create a table, such as Redis and Kafka. - Executing indicator and Stop button carried over for a moment onto the query tab switched to. diff --git a/Plugins/MongoDBDriverPlugin/MongoDBConnection+Documents.swift b/Plugins/MongoDBDriverPlugin/MongoDBConnection+Documents.swift index ad6251866f..ff5f43ece2 100644 --- a/Plugins/MongoDBDriverPlugin/MongoDBConnection+Documents.swift +++ b/Plugins/MongoDBDriverPlugin/MongoDBConnection+Documents.swift @@ -38,12 +38,14 @@ extension MongoDBConnection { try await onClient { [self] client in let documentBson = try parsedBson(document) defer { bson_destroy(documentBson) } + let optionsBson = try parsedBson(MongoInsertOptions.json) + defer { bson_destroy(optionsBson) } let handle = try getCollection(client, database: database, collection: collection) defer { mongoc_collection_destroy(handle) } guard let reply = bson_new() else { throw MongoDBError.connectionFailed } defer { bson_destroy(reply) } var error = bson_error_t() - guard mongoc_collection_insert_one(handle, documentBson, nil, reply, &error) else { + guard mongoc_collection_insert_one(handle, documentBson, optionsBson, reply, &error) else { if let failure = (try? canonicalText(of: reply)).flatMap(MongoWriteFailure.read(fromReply:)) { throw MongoDBError(code: failure.code, message: failure.message) } diff --git a/Plugins/MongoDBDriverPlugin/MongoDBConnection+ScriptHelpers.swift b/Plugins/MongoDBDriverPlugin/MongoDBConnection+ScriptHelpers.swift index a8ab73b585..704616ae5f 100644 --- a/Plugins/MongoDBDriverPlugin/MongoDBConnection+ScriptHelpers.swift +++ b/Plugins/MongoDBDriverPlugin/MongoDBConnection+ScriptHelpers.swift @@ -174,6 +174,11 @@ extension MongoDBConnection { identifiers.append("{\"$oid\": \"\(hex)\"}") } + guard let optsBson = jsonToBson(MongoInsertOptions.json) else { + throw MongoDBError(code: 0, message: MongoScriptText.invalidDocument(MongoInsertOptions.json)) + } + defer { bson_destroy(optsBson) } + try checkCancelled() var pointers: [OpaquePointer?] = prepared.map { Optional($0) } @@ -183,7 +188,7 @@ extension MongoDBConnection { let ok = pointers.withUnsafeMutableBufferPointer { buffer -> Bool in guard let base = buffer.baseAddress else { return false } - return mongoc_collection_insert_many(handle, base, buffer.count, nil, reply, &error) + return mongoc_collection_insert_many(handle, base, buffer.count, optsBson, reply, &error) } guard ok else { throw makeError(error) } return identifiers diff --git a/Plugins/MongoDBDriverPlugin/MongoDocumentText.swift b/Plugins/MongoDBDriverPlugin/MongoDocumentText.swift index eab4400cbb..2a00913848 100644 --- a/Plugins/MongoDBDriverPlugin/MongoDocumentText.swift +++ b/Plugins/MongoDBDriverPlugin/MongoDocumentText.swift @@ -28,7 +28,6 @@ struct MongoDocumentText: Equatable, Sendable { case trailingContent case malformed(line: Int, column: Int) case duplicateField(String) - case operatorField(String) case tooDeep var errorDescription: String? { @@ -47,11 +46,6 @@ struct MongoDocumentText: Equatable, Sendable { ) case .duplicateField(let name): return String(format: String(localized: "The field \u{201C}%@\u{201D} appears more than once."), name) - case .operatorField(let name): - return String( - format: String(localized: "The top-level field \u{201C}%@\u{201D} starts with $, which MongoDB reads as an operator."), - name - ) case .tooDeep: return String( format: String(localized: "The document is nested more than %d levels deep."), @@ -78,9 +72,6 @@ struct MongoDocumentText: Equatable, Sendable { guard case .object(let members) = try reader.readValue(depth: 1) else { throw Refusal.notAnObject } reader.skipWhitespace() guard reader.isAtEnd else { throw Refusal.trailingContent } - if let operatorField = members.first(where: { $0.key.hasPrefix("$") }) { - throw Refusal.operatorField(operatorField.key) - } self.members = members } diff --git a/Plugins/MongoDBDriverPlugin/MongoInsertOptions.swift b/Plugins/MongoDBDriverPlugin/MongoInsertOptions.swift new file mode 100644 index 0000000000..b77ea76da1 --- /dev/null +++ b/Plugins/MongoDBDriverPlugin/MongoInsertOptions.swift @@ -0,0 +1,21 @@ +// +// MongoInsertOptions.swift +// MongoDBDriverPlugin +// + +import Foundation + +/// The options every insert hands libmongoc: Insert Document's, and those behind a script's +/// `insertOne`, `insertMany`, `insert` and `save`, which is also how the grid writes a new row. +/// +/// libmongoc checks a document's keys before it sends an insert, and its default check refuses a +/// field named "" at any depth with "invalid document for insert: empty key". The server stores +/// such a field, and mongosh writes one. `validate` is that default without the empty-name flag, +/// libbson's `BSON_VALIDATE_UTF8` and `BSON_VALIDATE_UTF8_ALLOW_NULL`, so a key that is not UTF-8 +/// is still refused. It is never `false` or 0, which turns every check off, and for a replace that +/// includes the one keeping an update operator out of the replacement. +enum MongoInsertOptions { + static let validation = 1 | 8 + + static let json = #"{"validate":\#(validation)}"# +} diff --git a/Plugins/MongoDBDriverPlugin/MongoScriptPrelude.swift b/Plugins/MongoDBDriverPlugin/MongoScriptPrelude.swift index 3fe02d173c..ef3fa6683d 100644 --- a/Plugins/MongoDBDriverPlugin/MongoScriptPrelude.swift +++ b/Plugins/MongoDBDriverPlugin/MongoScriptPrelude.swift @@ -32,6 +32,13 @@ enum MongoScriptPrelude { })(); function __ejson(value) { return JSON.stringify(EJSON.serialize(value)); } + function __document(value) { + var document = {}; + for (var key in value) { + if (Object.prototype.hasOwnProperty.call(value, key)) { document[key] = EJSON.deserialize(value[key]); } + } + return document; + } """ private static let values = """ @@ -236,7 +243,7 @@ enum MongoScriptPrelude { if (this.__exhausted) { return false; } this.__started = true; var page = __tp.call({ op: "cursorFetch", handle: this.__handle }); - this.__batch = EJSON.deserialize(page.docs); + this.__batch = page.docs.map(__document); this.__index = 0; this.__exhausted = page.done; return this.__batch.length > 0; @@ -427,13 +434,13 @@ enum MongoScriptPrelude { if (!remove && (change === undefined || change === null)) { throw new Error("findOneAndUpdate needs an update document or pipeline"); } - var reply = this.__reply("findAndModify", { + var reply = this.__call("findAndModify", { filter: __ejson(filter === undefined ? {} : filter), update: change === undefined ? null : __ejson(change), options: options === undefined ? null : __ejson(options), remove: remove }); - return reply.value === undefined ? null : reply.value; + return reply.value === undefined || reply.value === null ? null : __document(reply.value); }; DBCollection.prototype.findOneAndUpdate = function (filter, update, options) { return this.__findAndModify(filter, update, options, false); diff --git a/TablePro/Resources/Localizable.xcstrings b/TablePro/Resources/Localizable.xcstrings index 55d3752f85..a13c221bf7 100644 --- a/TablePro/Resources/Localizable.xcstrings +++ b/TablePro/Resources/Localizable.xcstrings @@ -184345,9 +184345,6 @@ }, "The field “%@” appears more than once." : { - }, - "The top-level field “%@” starts with $, which MongoDB reads as an operator." : { - }, "The document is nested more than %d levels deep." : { diff --git a/TableProTests/Core/MongoDB/MongoScriptPreludeTests.swift b/TableProTests/Core/MongoDB/MongoScriptPreludeTests.swift index 7026a57126..2878c4de31 100644 --- a/TableProTests/Core/MongoDB/MongoScriptPreludeTests.swift +++ b/TableProTests/Core/MongoDB/MongoScriptPreludeTests.swift @@ -318,6 +318,46 @@ struct MongoScriptPreludeTests { #expect(value?.toString() == "507f1f77bcf86cd799439011") } + @Test("A document whose only field is named like a wrapper reads back as that document") + func wrapperNamedDocumentStaysADocument() throws { + let host = RecordingHost() + host.replies = [ + "1", + "{\"docs\": [{\"$oid\": \"not an id\"}, {\"$date\": \"2024\"}], \"done\": true}" + ] + let context = try makeContext(host) + + let value = context.evaluateScript(""" + db.orders.find({}, {_id: 0}).toArray().map(function (d) { return Object.keys(d)[0] + "=" + d[Object.keys(d)[0]]; }).join(",") + """) + #expect(context.exception == nil) + #expect(value?.toString() == "$oid=not an id,$date=2024") + } + + @Test("A findOneAndUpdate document named like a wrapper comes back as that document") + func findAndModifyDocumentStaysADocument() throws { + let host = RecordingHost() + host.replies = ["{\"value\": {\"$oid\": \"507f1f77bcf86cd799439011\"}}"] + let context = try makeContext(host) + + let value = context.evaluateScript(""" + var found = db.orders.findOneAndUpdate({}, {$set: {a: 1}}, {projection: {_id: 0}}); typeof found.$oid + """) + #expect(context.exception == nil) + #expect(value?.toString() == "string") + } + + @Test("A findOneAndUpdate that matches nothing returns null") + func findAndModifyWithNoMatchIsNull() throws { + let host = RecordingHost() + host.replies = ["{\"value\": null}"] + let context = try makeContext(host) + + let value = context.evaluateScript("db.orders.findOneAndUpdate({}, {$set: {a: 1}}) === null") + #expect(context.exception == nil) + #expect(value?.toBool() == true) + } + @Test("Variables and functions survive from one evaluated statement to the next") func shellStateSurvives() throws { let host = RecordingHost() diff --git a/TableProTests/Plugins/MongoDocumentTextTests.swift b/TableProTests/Plugins/MongoDocumentTextTests.swift index 6f2789125a..b2c6505be6 100644 --- a/TableProTests/Plugins/MongoDocumentTextTests.swift +++ b/TableProTests/Plugins/MongoDocumentTextTests.swift @@ -57,13 +57,22 @@ struct MongoDocumentTextTests { } } - @Test("A top-level field starting with $ is refused, a nested one is a wrapper") - func operatorFields() throws { - #expect(throws: MongoDocumentText.Refusal.operatorField("$set")) { - try MongoDocumentText(parsing: #"{"$set": {"a": 1}}"#) + @Test("An empty, blank, dotted or $-prefixed name reads as written, since an insert stores each one") + func namesAreReadAsWritten() throws { + let text = #"{"":1," ":2,"a.b":3,"$set":{"x":4},"$oid":"507f1f77bcf86cd799439011","n":{"":5}}"# + let document = try MongoDocumentText(parsing: text) + #expect(document.members.map(\.key) == ["", " ", "a.b", "$set", "$oid", "n"]) + #expect(document.compactText == text) + + let wrapped = try MongoDocumentText(parsing: #"{"_id": {"$oid": "507f1f77bcf86cd799439011"}}"#) + #expect(wrapped.members.count == 1) + } + + @Test("A field named \"\" is held to the rule against repeated fields") + func duplicateEmptyName() { + #expect(throws: MongoDocumentText.Refusal.duplicateField("")) { + try MongoDocumentText(parsing: #"{"": 1, "": 2}"#) } - let document = try MongoDocumentText(parsing: #"{"_id": {"$oid": "507f1f77bcf86cd799439011"}}"#) - #expect(document.members.count == 1) } @Test("Nesting deeper than MongoDB allows is refused without exhausting the stack") diff --git a/TableProTests/Plugins/MongoDocumentWritePlanTests.swift b/TableProTests/Plugins/MongoDocumentWritePlanTests.swift index ce20551717..4fb948d591 100644 --- a/TableProTests/Plugins/MongoDocumentWritePlanTests.swift +++ b/TableProTests/Plugins/MongoDocumentWritePlanTests.swift @@ -54,6 +54,22 @@ struct MongoDocumentWritePlanTests { #expect(!asked) } + @Test("Every name the server stores on insert reaches libbson and is sent as written") + func namesTheServerStores() throws { + let text = #"{"":1,"a":{"":2},"$set":{"x":1},"a.b":3,"$ref":"c","$id":1}"# + var asked = false + let spy: (String) throws -> String = { text in + asked = true + return text + } + let plan = try MongoDocumentWritePlan.make( + collection: "events", operation: .insert(document: text), canonicalize: spy + ) + #expect(asked) + #expect(plan.document == text) + #expect(plan.statement == "db.events.insertOne(\(text))") + } + @Test("A wrapper libbson cannot read is refused with libbson's reason") func libbsonRefusal() { struct Unreadable: Error {} diff --git a/TableProTests/Plugins/MongoInsertOptionsTests.swift b/TableProTests/Plugins/MongoInsertOptionsTests.swift new file mode 100644 index 0000000000..86908becf8 --- /dev/null +++ b/TableProTests/Plugins/MongoInsertOptionsTests.swift @@ -0,0 +1,52 @@ +// +// MongoInsertOptionsTests.swift +// TableProTests +// + +import Foundation +import Testing + +struct MongoInsertOptionsTests { + /// libbson 1.28.1's `bson_validate_flags_t`, which the test target cannot import. + private enum LibbsonValidation { + static let utf8 = 1 << 0 + static let dollarKeys = 1 << 1 + static let dotKeys = 1 << 2 + static let utf8AllowNull = 1 << 3 + static let emptyKeys = 1 << 4 + } + + /// `_mongoc_default_insert_vflags` in libmongoc 1.28.1, used when an insert passes no options. + private let libmongocInsertDefault = LibbsonValidation.utf8 | LibbsonValidation.utf8AllowNull + | LibbsonValidation.emptyKeys + + private func validate(in json: String) throws -> Int { + let data = try #require(json.data(using: .utf8)) + let options = try #require(try JSONSerialization.jsonObject(with: data) as? [String: Any]) + #expect(options.keys.sorted() == ["validate"]) + let number = try #require(options["validate"] as? NSNumber) + #expect(CFGetTypeID(number) != CFBooleanGetTypeID()) + return number.intValue + } + + @Test("An insert asks for libmongoc's default check without the one that refuses a field named \"\"") + func dropsOnlyTheEmptyNameCheck() throws { + let validate = try validate(in: MongoInsertOptions.json) + #expect(validate & LibbsonValidation.emptyKeys == 0) + #expect(validate == libmongocInsertDefault & ~LibbsonValidation.emptyKeys) + } + + @Test("An insert keeps the UTF-8 check and adds no key check of its own") + func keepsTheRestOfTheDefault() throws { + let validate = try validate(in: MongoInsertOptions.json) + #expect(validate != 0) + #expect(validate & LibbsonValidation.utf8 != 0) + #expect(validate & LibbsonValidation.utf8AllowNull != 0) + #expect(validate & (LibbsonValidation.dollarKeys | LibbsonValidation.dotKeys) == 0) + } + + @Test("The options text carries the value the type names") + func textMatchesValue() throws { + #expect(try validate(in: MongoInsertOptions.json) == MongoInsertOptions.validation) + } +} diff --git a/docs/databases/mongodb.mdx b/docs/databases/mongodb.mdx index 8145037bd8..ddd1ab3add 100644 --- a/docs/databases/mongodb.mdx +++ b/docs/databases/mongodb.mdx @@ -118,6 +118,8 @@ A field exists only in the documents that hold it, so a collection with no docum The text is Extended JSON: quote every field name, and write an ObjectId as `{"$oid": "…"}`, a date as `{"$date": "2024-05-01T10:00:00Z"}` and a decimal as `{"$numberDecimal": "1.10"}`. A whole number is stored as a 32-bit integer, or as a 64-bit one when it does not fit; `{"$numberLong": "5"}` stores a small 64-bit integer. A number with a decimal point is a double. Fields are stored in the order written. Leave out `_id` and the server generates one. +Every field name is stored as typed, an empty name, a dotted name such as `a.b` and a top-level name starting with `$` included. MongoDB before 5.0 refuses the last two kinds. mongosh reads a document holding both `$ref` and `$id` at the top level as a DBRef, so keep that pair for references. + ## Writing queries Queries run through JavaScriptCore, so a statement is JavaScript and the whole language is @@ -215,6 +217,7 @@ New connections default to **Disabled**, and the driver has no TLS fallback: **P - A collection takes one text index. A second **FULLTEXT** row fails after the collection and the indexes before it are created: list every text field in one index instead. - **New Table…** writes the validator with the server's own level and action. To log bad documents instead of refusing them, run `db.runCommand({collMod: "articles", validationAction: "warn"})` after creating the collection. - Nested paths filter but do not sort. Sorting works on the grid's own columns. +- Filtering, sorting or editing a field named `""` or starting with `$` fails in the grid. A dotted name such as `a.b` reads as the path to `b` inside `a`. Use `$getField` and `$setField` in the query editor. - **same element** covers a field one array deep. A path through an array inside another array needs nested `$elemMatch`, so those filter with dot notation only. - GridFS buckets are not browsable, and change streams are unsupported. - A script that loops without touching the database cannot be stopped: JavaScriptCore has no public way to interrupt one. `Cmd+.` stops anything that reads, writes or prints, which covers every query. A script silent for 120 seconds is abandoned and the shell restarts. diff --git a/project.yml b/project.yml index ecd82d75d7..5754be7ad5 100644 --- a/project.yml +++ b/project.yml @@ -521,6 +521,7 @@ targets: - Plugins/MongoDBDriverPlugin/MongoDBStatementGenerator.swift - Plugins/MongoDBDriverPlugin/MongoDocumentText.swift - Plugins/MongoDBDriverPlugin/MongoDocumentWritePlan.swift + - Plugins/MongoDBDriverPlugin/MongoInsertOptions.swift - Plugins/MongoDBDriverPlugin/MongoDBTimeoutPolicy.swift - Plugins/MongoDBDriverPlugin/MongoScriptCommandBuilder.swift - Plugins/MongoDBDriverPlugin/MongoShellCommandLine.swift