-
Type:
Bug
-
Resolution: Fixed
-
Priority:
Major - P3
-
Affects Version/s: None
-
Component/s: JavaScript
-
Labels:
-
Minor Change
-
ALL
-
v6.0, v5.3, v5.0, v4.4, v4.2
-
QE 2021-12-13, QE 2021-12-27, QE 2022-01-10, QE 2022-04-04, QE 2022-02-07, QE 2022-02-21, QE 2022-03-07, QE 2022-03-21, QE 2022-01-24
-
14
This mostly affects the $function or $accumulator aggregation operators. (I expect MapReduce and $where are similarly affected.) When these types are embedded within a document (object), they are fine. But when directly returned by a function, they are incorrectly serialized as an empty BSON object:
> db.test_function_return.drop()
false
> db.test_function_return.insert({})
WriteResult({ "nInserted" : 1 })
> db.test_function_return.aggregate( [ { $project: { _id: { $function: {
body: function() { return { foo: NumberLong("123") }; },
args: [],
lang: "js" } } } } ] ).toArray()
[ { "_id" : { "foo" : NumberLong(123) } } ]
> db.test_function_return.aggregate( [ { $project: { _id: { $function: {
body: function() { return NumberLong("123"); },
args: [],
lang: "js" } } } } ] ).toArray()
[ { "_id" : { } } ]
// But the expected/consistent result is [ { "_id" : NumberLong(123) } ]
>
The specific (current) types affected by this are:
- NumberLong
- NumberDecimal
- ObjectId
- BinData
- Timestamp
- MinKey
- MaxKey
- RegExp
I've attached a test script
which fully demonstrates this (and could easily be adapted into a jstest).
The main problems are two-fold:
- The JS object types are not handled and returned by ValueWriter::type(), which leads to it returning Object as the BSON type for them.
- Some of the BSON types are not handled by the switch in Scope::append().
For example, this is the patch required to fix ObjectId/jstOID (evergreen patch build):
Unable to find source-code formatter for language: diff. Available languages are: actionscript, ada, applescript, bash, c, c#, c++, cpp, css, erlang, go, groovy, haskell, html, java, javascript, js, json, lua, none, nyan, objc, perl, php, python, r, rainbow, ruby, scala, sh, sql, swift, visualbasic, xml, yaml
diff --git a/src/mongo/scripting/engine.cpp b/src/mongo/scripting/engine.cpp index 397adff2a76..3674236d0f2 100644 --- a/src/mongo/scripting/engine.cpp +++ b/src/mongo/scripting/engine.cpp @@ -121,6 +121,9 @@ void Scope::append(BSONObjBuilder& builder, const char* fieldName, const char* s case Code: builder.appendCode(fieldName, getString(scopeName)); break; + case jstOID: + builder.append(fieldName, getOID(scopeName)); + break; default: uassert(10206, str::stream() << "can't append type from: " << t, 0); } @@ -511,6 +514,9 @@ public: BSONObj getObject(const char* field) { return _real->getObject(field); } + OID getOID(const char* field) { + return _real->getOID(field); + } void setNumber(const char* field, double val) { _real->setNumber(field, val); } diff --git a/src/mongo/scripting/engine.h b/src/mongo/scripting/engine.h index e48596c9f09..0ba475e1de4 100644 --- a/src/mongo/scripting/engine.h +++ b/src/mongo/scripting/engine.h @@ -75,6 +75,7 @@ public: virtual bool getBoolean(const char* field) = 0; virtual double getNumber(const char* field) = 0; virtual int getNumberInt(const char* field) = 0; + virtual OID getOID(const char* field) = 0; virtual long long getNumberLongLong(const char* field) = 0; diff --git a/src/mongo/scripting/mozjs/implscope.cpp b/src/mongo/scripting/mozjs/implscope.cpp index 7cf6e8c28c8..b9f2e677059 100644 --- a/src/mongo/scripting/mozjs/implscope.cpp +++ b/src/mongo/scripting/mozjs/implscope.cpp @@ -575,6 +575,10 @@ BSONObj MozJSImplScope::getObject(const char* field) { return _runSafely([&] { return ObjectWrapper(_context, _global).getObject(field); }); } +OID MozJSImplScope::getOID(const char* field) { + return _runSafely([&] { return ObjectWrapper(_context, _global).getOID(field); }); +} + void MozJSImplScope::newFunction(StringData raw, JS::MutableHandleValue out) { _runSafely([&] { _MozJSCreateFunction(raw, std::move(out)); }); } diff --git a/src/mongo/scripting/mozjs/implscope.h b/src/mongo/scripting/mozjs/implscope.h index 86aa6c5dd05..81810f7e844 100644 --- a/src/mongo/scripting/mozjs/implscope.h +++ b/src/mongo/scripting/mozjs/implscope.h @@ -127,6 +127,7 @@ public: std::string getString(const char* field) override; bool getBoolean(const char* field) override; BSONObj getObject(const char* field) override; + OID getOID(const char* field) override; void setNumber(const char* field, double val) override; void setString(const char* field, StringData val) override; diff --git a/src/mongo/scripting/mozjs/objectwrapper.cpp b/src/mongo/scripting/mozjs/objectwrapper.cpp index d934c28ed37..8039c7f15a1 100644 --- a/src/mongo/scripting/mozjs/objectwrapper.cpp +++ b/src/mongo/scripting/mozjs/objectwrapper.cpp @@ -386,6 +386,13 @@ BSONObj ObjectWrapper::getObject(Key key) { return ValueWriter(_context, x).toBSON(); } +OID ObjectWrapper::getOID(Key key) { + JS::RootedValue x(_context); + getValue(key, &x); + + return ValueWriter(_context, x).toOID(); +} + void ObjectWrapper::getValue(Key key, JS::MutableHandleValue value) { key.get(_context, _object, value); } diff --git a/src/mongo/scripting/mozjs/objectwrapper.h b/src/mongo/scripting/mozjs/objectwrapper.h index 459f9746fe2..cff9f8d0fdc 100644 --- a/src/mongo/scripting/mozjs/objectwrapper.h +++ b/src/mongo/scripting/mozjs/objectwrapper.h @@ -112,6 +112,7 @@ public: std::string getString(Key key); bool getBoolean(Key key); BSONObj getObject(Key key); + OID getOID(Key key); void getValue(Key key, JS::MutableHandleValue value); void setNumber(Key key, double val); diff --git a/src/mongo/scripting/mozjs/proxyscope.cpp b/src/mongo/scripting/mozjs/proxyscope.cpp index 4002704e2d6..aa34938c111 100644 --- a/src/mongo/scripting/mozjs/proxyscope.cpp +++ b/src/mongo/scripting/mozjs/proxyscope.cpp @@ -158,6 +158,12 @@ BSONObj MozJSProxyScope::getObject(const char* field) { return out; } +OID MozJSProxyScope::getOID(const char* field) { + OID out; + run([&] { out = _implScope->getOID(field); }); + return out; +} + void MozJSProxyScope::setNumber(const char* field, double val) { run([&] { _implScope->setNumber(field, val); }); } diff --git a/src/mongo/scripting/mozjs/proxyscope.h b/src/mongo/scripting/mozjs/proxyscope.h index 85be843b22d..9aa7ff8b775 100644 --- a/src/mongo/scripting/mozjs/proxyscope.h +++ b/src/mongo/scripting/mozjs/proxyscope.h @@ -143,6 +143,7 @@ public: std::string getString(const char* field) override; bool getBoolean(const char* field) override; BSONObj getObject(const char* field) override; + OID getOID(const char* field) override; void setNumber(const char* field, double val) override; void setString(const char* field, StringData val) override; diff --git a/src/mongo/scripting/mozjs/valuewriter.cpp b/src/mongo/scripting/mozjs/valuewriter.cpp index 7af1d5325a6..8cd1b7b79b4 100644 --- a/src/mongo/scripting/mozjs/valuewriter.cpp +++ b/src/mongo/scripting/mozjs/valuewriter.cpp @@ -92,6 +92,13 @@ int ValueWriter::type() { if (JS_ObjectIsFunction(_context, obj)) return Code; + auto scope = getScope(_context); + auto jsclass = JS_GetClass(obj); + + if (jsclass && scope->getProto<OIDInfo>().getJSClass() == jsclass) { + return jstOID; + } + return Object; } @@ -230,6 +237,13 @@ Decimal128 ValueWriter::toDecimal128() { uasserted(ErrorCodes::BadValue, str::stream() << "Unable to write Decimal128 value."); } +OID ValueWriter::toOID() { + if (getScope(_context)->getProto<OIDInfo>().instanceOf(_value)) + return OIDInfo::getOID(_context, _value); + + throwCurrentJSException(_context, ErrorCodes::BadValue, "Unable to write ObjectId value."); +} + void ValueWriter::writeThis(BSONObjBuilder* b, StringData sd, ObjectWrapper::WriteFieldRecursionFrames* frames) { diff --git a/src/mongo/scripting/mozjs/valuewriter.h b/src/mongo/scripting/mozjs/valuewriter.h index c42e716e35b..62bab3b1bc1 100644 --- a/src/mongo/scripting/mozjs/valuewriter.h +++ b/src/mongo/scripting/mozjs/valuewriter.h @@ -62,6 +62,7 @@ public: int64_t toInt64(); Decimal128 toDecimal128(); bool toBoolean(); + OID toOID(); /** * Provides the type of the value. For objects, it fetches the class name if possible.
The other types almost certainly require correspondingly similar adjustments.
There is another consideration for any future types that may be represented as a JS object, but without their own BSON type, ie. serialized to a BSON Object representation, eg. { $foo: { ... } }. Any such types will still fail to serialize if they are the top-level object (but will work fine if they are a field inside another BSON object). One way to fix this (without needing invasive restructuring of ValueWriter/ObjectWrapper) might be to adjust ValueWriter::toBSON() to nest the object inside a parent object, for example, something like as shown below (evergreen patch build). However, this may have some other adverse effects (eg. reducing the effective maximum BSON depth by 1, possibly lowering performance, etc).
Unable to find source-code formatter for language: diff. Available languages are: actionscript, ada, applescript, bash, c, c#, c++, cpp, css, erlang, go, groovy, haskell, html, java, javascript, js, json, lua, none, nyan, objc, perl, php, python, r, rainbow, ruby, scala, sh, sql, swift, visualbasic, xml, yaml
diff --git a/src/mongo/scripting/mozjs/valuewriter.cpp b/src/mongo/scripting/mozjs/valuewriter.cpp
index 65c9eacc3cf..5674377989b 100644
--- a/src/mongo/scripting/mozjs/valuewriter.cpp
+++ b/src/mongo/scripting/mozjs/valuewriter.cpp
@@ -149,9 +149,12 @@ BSONObj ValueWriter::toBSON() {
if (!_value.isObject())
return BSONObj();
- JS::RootedObject obj(_context, _value.toObjectOrNull());
+ JS::RootedObject obj(_context);
+ getScope(_context)->getProto<ObjectInfo>().newObject(&obj);
- return ObjectWrapper(_context, obj).toBSON();
+ auto objWrap = ObjectWrapper(_context, obj);
+ objWrap.setValue("", _value);
+ return objWrap.toBSON().firstElement().embeddedObject().getOwned();
}
std::string ValueWriter::toString() {
- is related to
-
-
- Closed
-
-
-
- Closed
-