SERVER-28323 Don't pass JavaScript scopes a function ID number

(cherry picked from commit d20c74fe700d15ea555f9c6cae916b522005eec7)

7 files changed, 48 insertions, 60 deletions

diff --git a/jstests/core/evalj.js b/jstests/core/evalj.js
new file mode 100644
index 00000000000..f2326fff365
--- /dev/null
+++ b/jstests/core/evalj.js
@@ -0,0 +1,13 @@
+(function() {
+    "use strict";
+
+    db.col.insert({data: 5});
+
+    db.eval("print(5)");
+
+    db.system.js.insert({_id: "foo", value: Code("db.col.drop()")});
+
+    db.eval("print(5)");
+
+    assert.eq(5, db.col.findOne()["data"]);
+})();
diff --git a/src/mongo/scripting/engine.cpp b/src/mongo/scripting/engine.cpp
index 1bee97838f8..7e6628344ab 100644
--- a/src/mongo/scripting/engine.cpp
+++ b/src/mongo/scripting/engine.cpp
@@ -267,13 +267,11 @@ ScriptingFunction Scope::createFunction(const char* code) {
     FunctionCacheMap::iterator i = _cachedFunctions.find(code);
     if (i != _cachedFunctions.end())
         return i->second;
-    // NB: we calculate the function number for v8 so the cache can be utilized to
-    //     lookup the source on an exception, but SpiderMonkey uses the value
-    //     returned by JS_CompileFunction.
-    ScriptingFunction defaultFunctionNumber = getFunctionCache().size() + 1;
-    ScriptingFunction actualFunctionNumber = _createFunction(code, defaultFunctionNumber);
-    _cachedFunctions[code] = actualFunctionNumber;
-    return actualFunctionNumber;
+
+    // Get a function number, so the cache can be utilized to lookup the source on an exception
+    ScriptingFunction functionNumber = _createFunction(code);
+    _cachedFunctions[code] = functionNumber;
+    return functionNumber;
 }
 
 namespace JSFiles {
@@ -507,12 +505,8 @@ public:
     }
 
 protected:
-    FunctionCacheMap& getFunctionCache() {
-        return _real->getFunctionCache();
-    }
-
-    ScriptingFunction _createFunction(const char* code, ScriptingFunction functionNumber = 0) {
-        return _real->_createFunction(code, functionNumber);
+    ScriptingFunction _createFunction(const char* code) {
+        return _real->_createFunction(code);
     }
 
 private:
diff --git a/src/mongo/scripting/engine.h b/src/mongo/scripting/engine.h
index f06333e9a45..d31311f0114 100644
--- a/src/mongo/scripting/engine.h
+++ b/src/mongo/scripting/engine.h
@@ -211,11 +211,7 @@ protected:
      */
     class StoredFuncModLogOpHandler;
 
-    virtual FunctionCacheMap& getFunctionCache() {
-        return _cachedFunctions;
-    }
-    virtual ScriptingFunction _createFunction(const char* code,
-                                              ScriptingFunction functionNumber = 0) = 0;
+    virtual ScriptingFunction _createFunction(const char* code) = 0;
 
     std::string _localDBName;
     int64_t _loadedVersion;
diff --git a/src/mongo/scripting/mozjs/implscope.cpp b/src/mongo/scripting/mozjs/implscope.cpp
index 70f76355202..3b260c1558c 100644
--- a/src/mongo/scripting/mozjs/implscope.cpp
+++ b/src/mongo/scripting/mozjs/implscope.cpp
@@ -487,11 +487,20 @@ BSONObj MozJSImplScope::getObject(const char* field) {
 void MozJSImplScope::newFunction(StringData raw, JS::MutableHandleValue out) {
     MozJSEntry entry(this);
 
-    std::string code = str::stream() << "____MongoToSM_newFunction_temp = " << raw;
+    _MozJSCreateFunction(raw, std::move(out));
+}
+
+void MozJSImplScope::_MozJSCreateFunction(StringData raw, JS::MutableHandleValue fun) {
+    std::string code = str::stream()
+        << "(" << parseJSFunctionOrExpression(_context, StringData(raw)) << ")";
 
     JS::CompileOptions co(_context);
     setCompileOptions(&co);
-    _checkErrorState(JS::Evaluate(_context, _global, co, code.c_str(), code.length(), out));
+
+    _checkErrorState(JS::Evaluate(_context, _global, co, code.c_str(), code.length(), fun));
+    uassert(10232,
+            "not a function",
+            fun.isObject() && JS_ObjectIsFunction(_context, fun.toObjectOrNull()));
 }
 
 BSONObj MozJSImplScope::callThreadArgs(const BSONObj& args) {
@@ -532,39 +541,20 @@ bool hasFunctionIdentifier(StringData code) {
     return code[8] == ' ' || code[8] == '(';
 }
 
-void MozJSImplScope::_MozJSCreateFunction(const char* raw,
-                                          ScriptingFunction functionNumber,
-                                          JS::MutableHandleValue fun) {
-    std::string code = str::stream() << "_funcs" << functionNumber << " = "
-                                     << parseJSFunctionOrExpression(_context, StringData(raw));
-
-    JS::CompileOptions co(_context);
-    setCompileOptions(&co);
-
-    _checkErrorState(JS::Evaluate(_context, _global, co, code.c_str(), code.length(), fun));
-    uassert(10232,
-            "not a function",
-            fun.isObject() && JS_ObjectIsFunction(_context, fun.toObjectOrNull()));
-}
-
-ScriptingFunction MozJSImplScope::_createFunction(const char* raw,
-                                                  ScriptingFunction functionNumber) {
+ScriptingFunction MozJSImplScope::_createFunction(const char* raw) {
     MozJSEntry entry(this);
 
     JS::RootedValue fun(_context);
-    _MozJSCreateFunction(raw, functionNumber, &fun);
+    _MozJSCreateFunction(raw, &fun);
     _funcs.emplace_back(_context, fun.get());
-
-    return functionNumber;
+    return _funcs.size();
 }
 
 void MozJSImplScope::setFunction(const char* field, const char* code) {
     MozJSEntry entry(this);
 
     JS::RootedValue fun(_context);
-
-    _MozJSCreateFunction(code, getFunctionCache().size() + 1, &fun);
-
+    _MozJSCreateFunction(code, &fun);
     ObjectWrapper(_context, _global).setValue(field, fun);
 }
 
@@ -714,6 +704,11 @@ void MozJSImplScope::localConnectForDbEval(OperationContext* txn, const char* db
     // NOTE: order is important here.  the following methods must be called after
     //       the above conditional statements.
 
+    _connectState = ConnectState::Local;
+    _localDBName = dbName;
+
+    loadStored(txn);
+
     // install db access functions in the global object
     installDBAccess();
 
@@ -721,16 +716,11 @@ void MozJSImplScope::localConnectForDbEval(OperationContext* txn, const char* db
     _mongoLocalProto.install(_global);
     execCoreFiles();
 
-    const char* const makeMongo = "_mongo = new Mongo()";
+    const char* const makeMongo = "const _mongo = new Mongo()";
     exec(makeMongo, "local connect 2", false, true, true, 0);
 
-    std::string makeDB = str::stream() << "db = _mongo.getDB(\"" << dbName << "\");";
+    std::string makeDB = str::stream() << "const db = _mongo.getDB(\"" << dbName << "\");";
     exec(makeDB, "local connect 3", false, true, true, 0);
-
-    _connectState = ConnectState::Local;
-    _localDBName = dbName;
-
-    loadStored(txn);
 }
 
 void MozJSImplScope::externalSetup() {
diff --git a/src/mongo/scripting/mozjs/implscope.h b/src/mongo/scripting/mozjs/implscope.h
index 48544aaf5b2..bfff70a93b0 100644
--- a/src/mongo/scripting/mozjs/implscope.h
+++ b/src/mongo/scripting/mozjs/implscope.h
@@ -147,8 +147,7 @@ public:
 
     void injectNative(const char* field, NativeFunction func, void* data = 0) override;
 
-    ScriptingFunction _createFunction(const char* code,
-                                      ScriptingFunction functionNumber = 0) override;
+    ScriptingFunction _createFunction(const char* code) override;
 
     void newFunction(StringData code, JS::MutableHandleValue out);
 
@@ -311,9 +310,7 @@ public:
     }
 
 private:
-    void _MozJSCreateFunction(const char* raw,
-                              ScriptingFunction functionNumber,
-                              JS::MutableHandleValue fun);
+    void _MozJSCreateFunction(StringData raw, JS::MutableHandleValue fun);
 
     /**
      * This structure exists exclusively to construct the runtime and context
diff --git a/src/mongo/scripting/mozjs/proxyscope.cpp b/src/mongo/scripting/mozjs/proxyscope.cpp
index 2fd32ff6878..bb4cd5c06ff 100644
--- a/src/mongo/scripting/mozjs/proxyscope.cpp
+++ b/src/mongo/scripting/mozjs/proxyscope.cpp
@@ -229,10 +229,9 @@ void MozJSProxyScope::injectNative(const char* field, NativeFunction func, void*
     run([&] { _implScope->injectNative(field, func, data); });
 }
 
-ScriptingFunction MozJSProxyScope::_createFunction(const char* raw,
-                                                   ScriptingFunction functionNumber) {
+ScriptingFunction MozJSProxyScope::_createFunction(const char* raw) {
     ScriptingFunction out;
-    run([&] { out = _implScope->_createFunction(raw, functionNumber); });
+    run([&] { out = _implScope->_createFunction(raw); });
     return out;
 }
 
diff --git a/src/mongo/scripting/mozjs/proxyscope.h b/src/mongo/scripting/mozjs/proxyscope.h
index d19e24149bd..451981330a1 100644
--- a/src/mongo/scripting/mozjs/proxyscope.h
+++ b/src/mongo/scripting/mozjs/proxyscope.h
@@ -165,8 +165,7 @@ public:
 
     void injectNative(const char* field, NativeFunction func, void* data = 0) override;
 
-    ScriptingFunction _createFunction(const char* code,
-                                      ScriptingFunction functionNumber = 0) override;
+    ScriptingFunction _createFunction(const char* code) override;
 
     OperationContext* getOpContext() const;