From 8ec0941a0d72286dd53b10990b15b51402570b24 Mon Sep 17 00:00:00 2001 From: James Wahlin Date: Fri, 28 Feb 2020 12:51:59 -0500 Subject: SERVER-46492 Restore WhereMatchExpression for $where as default --- jstests/core/system_js_access.js | 2 +- src/mongo/db/matcher/SConscript | 1 + .../db/matcher/expression_serialization_test.cpp | 18 +-- src/mongo/db/matcher/expression_where.cpp | 122 +++++++++++++++++++++ src/mongo/db/matcher/expression_where.h | 56 ++++++++++ src/mongo/db/matcher/expression_where_base.cpp | 10 +- src/mongo/db/matcher/expression_where_base.h | 6 - src/mongo/db/matcher/expression_where_noop.cpp | 1 - src/mongo/db/matcher/extensions_callback.cpp | 4 +- src/mongo/db/matcher/extensions_callback_real.cpp | 36 +++--- src/mongo/db/pipeline/expression_context.h | 18 ++- src/mongo/db/query/query_knobs.idl | 7 ++ .../dbtests/extensions_callback_real_test.cpp | 3 + 13 files changed, 231 insertions(+), 53 deletions(-) create mode 100644 src/mongo/db/matcher/expression_where.cpp create mode 100644 src/mongo/db/matcher/expression_where.h diff --git a/jstests/core/system_js_access.js b/jstests/core/system_js_access.js index 131fe04d5dc..a0d205cee8b 100644 --- a/jstests/core/system_js_access.js +++ b/jstests/core/system_js_access.js @@ -75,7 +75,7 @@ assert.commandFailedWithCode(db.runCommand({ ] } }), - 31438); + 4649200); // Mixed queries with both $function and $accumulator should succeed. // TODO SERVER-45450: Change $_internalJsReduce to $accumulator. diff --git a/src/mongo/db/matcher/SConscript b/src/mongo/db/matcher/SConscript index 07ce6a58bcd..5e6d5c6a360 100644 --- a/src/mongo/db/matcher/SConscript +++ b/src/mongo/db/matcher/SConscript @@ -80,6 +80,7 @@ env.Library( source=[ 'extensions_callback_real.cpp', 'expression_text.cpp', + 'expression_where.cpp', ], LIBDEPS=[ '$BUILD_DIR/mongo/db/concurrency/lock_manager', diff --git a/src/mongo/db/matcher/expression_serialization_test.cpp b/src/mongo/db/matcher/expression_serialization_test.cpp index 5c353734e1d..78517a52634 100644 --- a/src/mongo/db/matcher/expression_serialization_test.cpp +++ b/src/mongo/db/matcher/expression_serialization_test.cpp @@ -1343,24 +1343,8 @@ TEST(SerializeBasic, ExpressionWhereSerializesCorrectly) { expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::kAllowAllSpecialFeatures); - ASSERT_BSONOBJ_EQ( - *reserialized.getQuery(), - BSONObjBuilder().appendCodeWScope("$where", "this.a == this.b", BSONObj()).obj()); - ASSERT_BSONOBJ_EQ(*reserialized.getQuery(), serialize(reserialized.getMatchExpression())); -} - -TEST(SerializeBasic, ExpressionWhereWithScopeSerializesCorrectly) { - boost::intrusive_ptr expCtx(new ExpressionContextForTest()); - Matcher original(BSON("$where" << BSONCodeWScope("this.a == this.b", BSON("x" << 3))), - expCtx, - ExtensionsCallbackNoop(), - MatchExpressionParser::kAllowAllSpecialFeatures); - Matcher reserialized(serialize(original.getMatchExpression()), - expCtx, - ExtensionsCallbackNoop(), - MatchExpressionParser::kAllowAllSpecialFeatures); ASSERT_BSONOBJ_EQ(*reserialized.getQuery(), - BSON("$where" << BSONCodeWScope("this.a == this.b", BSON("x" << 3)))); + BSONObjBuilder().appendCode("$where", "this.a == this.b").obj()); ASSERT_BSONOBJ_EQ(*reserialized.getQuery(), serialize(reserialized.getMatchExpression())); } diff --git a/src/mongo/db/matcher/expression_where.cpp b/src/mongo/db/matcher/expression_where.cpp new file mode 100644 index 00000000000..2518da25484 --- /dev/null +++ b/src/mongo/db/matcher/expression_where.cpp @@ -0,0 +1,122 @@ +/** + * Copyright (C) 2018-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + +#include "mongo/platform/basic.h" + +#include "mongo/db/matcher/expression_where.h" + +#include + +#include "mongo/base/init.h" +#include "mongo/db/auth/authorization_session.h" +#include "mongo/db/client.h" +#include "mongo/db/jsobj.h" +#include "mongo/db/matcher/expression.h" +#include "mongo/db/matcher/expression_parser.h" +#include "mongo/db/namespace_string.h" +#include "mongo/db/query/query_knobs_gen.h" +#include "mongo/scripting/engine.h" +#include "mongo/util/scopeguard.h" + + +namespace mongo { + +using std::string; +using std::stringstream; +using std::unique_ptr; + +namespace { +std::string getAuthenticatedUserNamesToken(Client* client) { + StringBuilder sb; + + auto as = AuthorizationSession::get(client); + for (auto nameIter = as->getAuthenticatedUserNames(); nameIter.more(); nameIter.next()) { + // Using a NUL byte which isn't valid in usernames to separate them. + sb << '\0' << nameIter->getUnambiguousName(); + } + + return sb.str(); +} +} // namespace + +WhereMatchExpression::WhereMatchExpression(OperationContext* opCtx, + WhereParams params, + StringData dbName) + : WhereMatchExpressionBase(std::move(params)), _dbName(dbName.toString()), _opCtx(opCtx) { + invariant(_opCtx != nullptr); + + uassert( + ErrorCodes::BadValue, "no globalScriptEngine in $where parsing", getGlobalScriptEngine()); + + uassert(ErrorCodes::BadValue, "ns for $where cannot be empty", dbName.size() != 0); + + const auto userToken = getAuthenticatedUserNamesToken(opCtx->getClient()); + _scope = getGlobalScriptEngine()->getPooledScope(_opCtx, _dbName, "where" + userToken); + const auto guard = makeGuard([&] { _scope->unregisterOperation(); }); + + _func = _scope->createFunction(getCode().c_str()); + + uassert(ErrorCodes::BadValue, "$where compile error", _func); +} + +bool WhereMatchExpression::matches(const MatchableDocument* doc, MatchDetails* details) const { + uassert(28692, "$where compile error", _func); + BSONObj obj = doc->toBSON(); + + _scope->registerOperation(Client::getCurrent()->getOperationContext()); + const auto guard = makeGuard([&] { _scope->unregisterOperation(); }); + + _scope->advanceGeneration(); + _scope->setObject("obj", const_cast(obj)); + _scope->setBoolean("fullObject", true); // this is a hack b/c fullObject used to be relevant + + int err = + _scope->invoke(_func, nullptr, &obj, internalQueryJavaScriptFnTimeoutMillis.load(), false); + if (err == -3) { // INVOKE_ERROR + stringstream ss; + ss << "error on invocation of $where function:\n" << _scope->getError(); + uassert(16812, ss.str(), false); + } else if (err != 0) { // ! INVOKE_SUCCESS + uassert(16813, "unknown error in invocation of $where function", false); + } + + return _scope->getBoolean("__returnValue") != 0; +} + +unique_ptr WhereMatchExpression::shallowClone() const { + WhereParams params; + params.code = getCode(); + unique_ptr e = + std::make_unique(_opCtx, std::move(params), _dbName); + if (getTag()) { + e->setTag(getTag()->clone()); + } + return std::move(e); +} +} // namespace mongo diff --git a/src/mongo/db/matcher/expression_where.h b/src/mongo/db/matcher/expression_where.h new file mode 100644 index 00000000000..dfe7ee809ea --- /dev/null +++ b/src/mongo/db/matcher/expression_where.h @@ -0,0 +1,56 @@ +/** + * Copyright (C) 2018-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + +#pragma once + +#include "mongo/db/matcher/expression_where_base.h" +#include "mongo/scripting/engine.h" + +namespace mongo { + +class OperationContext; + +class WhereMatchExpression final : public WhereMatchExpressionBase { +public: + WhereMatchExpression(OperationContext* opCtx, WhereParams params, StringData dbName); + + bool matches(const MatchableDocument* doc, MatchDetails* details = nullptr) const final; + + std::unique_ptr shallowClone() const final; + +private: + std::string _dbName; + + std::unique_ptr _scope; + ScriptingFunction _func; + + OperationContext* const _opCtx; +}; + +} // namespace mongo diff --git a/src/mongo/db/matcher/expression_where_base.cpp b/src/mongo/db/matcher/expression_where_base.cpp index 24168d26028..30148f9a42b 100644 --- a/src/mongo/db/matcher/expression_where_base.cpp +++ b/src/mongo/db/matcher/expression_where_base.cpp @@ -36,7 +36,7 @@ namespace mongo { WhereMatchExpressionBase::WhereMatchExpressionBase(WhereParams params) - : MatchExpression(WHERE), _code(std::move(params.code)), _scope(std::move(params.scope)) {} + : MatchExpression(WHERE), _code(std::move(params.code)) {} void WhereMatchExpressionBase::debugString(StringBuilder& debug, int indentationLevel) const { _debugAddSpace(debug, indentationLevel); @@ -44,13 +44,10 @@ void WhereMatchExpressionBase::debugString(StringBuilder& debug, int indentation _debugAddSpace(debug, indentationLevel + 1); debug << "code: " << getCode() << "\n"; - - _debugAddSpace(debug, indentationLevel + 1); - debug << "scope: " << getScope() << "\n"; } void WhereMatchExpressionBase::serialize(BSONObjBuilder* out, bool includePath) const { - out->appendCodeWScope("$where", getCode(), getScope()); + out->appendCode("$where", getCode()); } bool WhereMatchExpressionBase::equivalent(const MatchExpression* other) const { @@ -58,8 +55,7 @@ bool WhereMatchExpressionBase::equivalent(const MatchExpression* other) const { return false; } const WhereMatchExpressionBase* realOther = static_cast(other); - return getCode() == realOther->getCode() && - SimpleBSONObjComparator::kInstance.evaluate(getScope() == realOther->getScope()); + return getCode() == realOther->getCode(); } } // namespace mongo diff --git a/src/mongo/db/matcher/expression_where_base.h b/src/mongo/db/matcher/expression_where_base.h index dc839966fd0..f68ccb5efc6 100644 --- a/src/mongo/db/matcher/expression_where_base.h +++ b/src/mongo/db/matcher/expression_where_base.h @@ -42,7 +42,6 @@ class WhereMatchExpressionBase : public MatchExpression { public: struct WhereParams { std::string code; - BSONObj scope; // Owned. }; explicit WhereMatchExpressionBase(WhereParams params); @@ -78,17 +77,12 @@ protected: return _code; } - const BSONObj& getScope() const { - return _scope; - } - private: ExpressionOptimizerFunc getOptimizer() const final { return [](std::unique_ptr expression) { return expression; }; } const std::string _code; - const BSONObj _scope; // Owned. }; } // namespace mongo diff --git a/src/mongo/db/matcher/expression_where_noop.cpp b/src/mongo/db/matcher/expression_where_noop.cpp index 3328c18ea09..e300c1517c6 100644 --- a/src/mongo/db/matcher/expression_where_noop.cpp +++ b/src/mongo/db/matcher/expression_where_noop.cpp @@ -46,7 +46,6 @@ bool WhereNoOpMatchExpression::matches(const MatchableDocument* doc, MatchDetail std::unique_ptr WhereNoOpMatchExpression::shallowClone() const { WhereParams params; params.code = getCode(); - params.scope = getScope(); std::unique_ptr e = std::make_unique(std::move(params)); if (getTag()) { diff --git a/src/mongo/db/matcher/extensions_callback.cpp b/src/mongo/db/matcher/extensions_callback.cpp index f79ef3aac86..b74636463b1 100644 --- a/src/mongo/db/matcher/extensions_callback.cpp +++ b/src/mongo/db/matcher/extensions_callback.cpp @@ -105,11 +105,9 @@ ExtensionsCallback::extractWhereMatchExpressionParams(BSONElement where) { case mongo::String: case mongo::Code: params.code = where._asCode(); - params.scope = BSONObj(); break; case mongo::CodeWScope: - params.code = where._asCode(); - params.scope = where.codeWScopeObject().getOwned(); + uasserted(4649201, "$where no longer supports deprecated BSON type CodeWScope"); break; default: return {ErrorCodes::BadValue, "$where got bad type"}; diff --git a/src/mongo/db/matcher/extensions_callback_real.cpp b/src/mongo/db/matcher/extensions_callback_real.cpp index e2c3890dcfb..34d2dfc4a26 100644 --- a/src/mongo/db/matcher/extensions_callback_real.cpp +++ b/src/mongo/db/matcher/extensions_callback_real.cpp @@ -31,10 +31,13 @@ #include "mongo/db/matcher/extensions_callback_real.h" +#include "mongo/db/commands/test_commands_enabled.h" #include "mongo/db/matcher/expression_expr.h" #include "mongo/db/matcher/expression_text.h" +#include "mongo/db/matcher/expression_where.h" #include "mongo/db/namespace_string.h" #include "mongo/db/pipeline/expression_function.h" +#include "mongo/db/query/query_knobs_gen.h" #include "mongo/db/query/util/make_data_structure.h" #include "mongo/scripting/engine.h" @@ -57,27 +60,34 @@ StatusWithMatchExpression ExtensionsCallbackReal::parseText(BSONElement text) co StatusWithMatchExpression ExtensionsCallbackReal::parseWhere( const boost::intrusive_ptr& expCtx, BSONElement where) const { - auto whereParams = extractWhereMatchExpressionParams(where); if (!whereParams.isOK()) { return whereParams.getStatus(); } - uassert(ErrorCodes::BadValue, "ns for $where cannot be empty", expCtx->ns.db().size() != 0); + if (getTestCommandsEnabled() && internalQueryDesugarWhereToFunction.load()) { + uassert(ErrorCodes::BadValue, "ns for $where cannot be empty", expCtx->ns.db().size() != 0); - auto code = whereParams.getValue().code; + auto code = whereParams.getValue().code; - // Desugar $where to $expr. The $where function is invoked through a $function expression by - // passing the document as $$CURRENT. - auto fnExpression = ExpressionFunction::createForWhere( - expCtx, - ExpressionArray::create( + // Desugar $where to $expr. The $where function is invoked through a $function expression by + // passing the document as $$CURRENT. + auto fnExpression = ExpressionFunction::createForWhere( expCtx, - make_vector>( - ExpressionFieldPath::parse(expCtx, "$$CURRENT", expCtx->variablesParseState))), - code, - ExpressionFunction::kJavaScript); + ExpressionArray::create( + expCtx, + make_vector>( + ExpressionFieldPath::parse(expCtx, "$$CURRENT", expCtx->variablesParseState))), + code, + ExpressionFunction::kJavaScript); - return {std::make_unique(fnExpression, expCtx)}; + return {std::make_unique(fnExpression, expCtx)}; + } else { + expCtx->hasWhereClause = true; + auto exp = std::make_unique( + _opCtx, std::move(whereParams.getValue()), expCtx->ns.db()); + return {std::move(exp)}; + } } + } // namespace mongo diff --git a/src/mongo/db/pipeline/expression_context.h b/src/mongo/db/pipeline/expression_context.h index 0fba82ac610..35be79e3b1a 100644 --- a/src/mongo/db/pipeline/expression_context.h +++ b/src/mongo/db/pipeline/expression_context.h @@ -279,12 +279,19 @@ public: invariant(!forceLoadOfStoredProcedures); invariant(!isMapReduceCommand); } + + // Stored procedures are only loaded for the $where expression and MapReduce command. + const bool loadStoredProcedures = forceLoadOfStoredProcedures || isMapReduceCommand; + + if (hasWhereClause && !loadStoredProcedures) { + uasserted(4649200, + "A single operation cannot use both JavaScript aggregation expressions and " + "$where."); + } + const boost::optional& scope = runtimeConstants.getJsScope(); - return JsExecution::get(opCtx, - scope.get_value_or(BSONObj()), - ns.db(), - forceLoadOfStoredProcedures || isMapReduceCommand, - jsHeapLimitMB); + return JsExecution::get( + opCtx, scope.get_value_or(BSONObj()), ns.db(), loadStoredProcedures, jsHeapLimitMB); } // The explain verbosity requested by the user, or boost::none if no explain was requested. @@ -296,6 +303,7 @@ public: bool allowDiskUse = false; bool bypassDocumentValidation = false; bool inMultiDocumentTransaction = false; + bool hasWhereClause = false; NamespaceString ns; diff --git a/src/mongo/db/query/query_knobs.idl b/src/mongo/db/query/query_knobs.idl index 4b4fd4a852d..644c5c0fab9 100644 --- a/src/mongo/db/query/query_knobs.idl +++ b/src/mongo/db/query/query_knobs.idl @@ -348,3 +348,10 @@ server_parameters: expr: 60 * 1000 validator: gt: 0 + + internalQueryDesugarWhereToFunction: + description: "When true, desugars $where to $expr/$function." + set_at: [ startup, runtime ] + cpp_varname: "internalQueryDesugarWhereToFunction" + cpp_vartype: AtomicWord + default: false diff --git a/src/mongo/dbtests/extensions_callback_real_test.cpp b/src/mongo/dbtests/extensions_callback_real_test.cpp index ae2287ac77a..9783a15d693 100644 --- a/src/mongo/dbtests/extensions_callback_real_test.cpp +++ b/src/mongo/dbtests/extensions_callback_real_test.cpp @@ -242,6 +242,8 @@ TEST_F(ExtensionsCallbackRealTest, TextDiacriticSensitiveAndCaseSensitiveTrue) { // const NamespaceString kTestNss = NamespaceString("db.dummy"); +// TODO SERVER-46494: Re-enable this test. +/** TEST_F(ExtensionsCallbackRealTest, WhereExpressionDesugarsToExprAndInternalJs) { auto query1 = fromjson("{$where: 'function() { return this.x == 10; }'}"); boost::intrusive_ptr expCtx( @@ -258,6 +260,7 @@ TEST_F(ExtensionsCallbackRealTest, WhereExpressionDesugarsToExprAndInternalJs) { "['$$CURRENT'], 'lang': 'js', '_internalSetObjToThis': true}}}"); ASSERT_BSONOBJ_EQ(gotMatch.obj(), expectedMatch); } +*/ } // namespace } // namespace mongo -- cgit v1.2.1