8000 fixed issue #17291 (#17623) · arangodb/arangodb@38ca609 · GitHub
[go: up one dir, main page]
Skip to content

Commit 38ca609

Browse files
jsteemannKVS85
jsteemann
and
KVS85
authored
fixed issue #17291 (#17623)
* fixed issue #17291 PRUNE expressions containing JavaScript or user-defined functions are now properly rejected. * Update CHANGELOG Co-authored-by: Vadim <vadim@arangodb.com>
1 parent c1b0bbd commit 38ca609

File tree

5 files changed

+160
-0
lines changed

5 files changed

+160
-0
lines changed

‎CHANGELOG

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
11
v3.8.9 (XXXX-XX-XX)
22
-------------------
33

4+
* Fixed issue #17291: Server crash on error in the PRUNE expression.
5+
Traversal PRUNE expressions containing JavaScript user-defined functions
6+
(UDFs) are now properly rejected in single server and cluster mode.
7+
PRUNE expressions that use UDFs require a V8 context for execution, which is
8+
not available on DB-servers in a cluster, and also isn't necessarily available
9+
for regular queries on single servers (a V8 context is only available if a
10+
query was executed inside Foxx or from inside a JS transaction, but not
11+
otherwise).
12+
413
* Updated OpenSSL to 1.1.1s.
514

615
* Solve a case of excessive memory consumption in certain AQL queries with IN

‎arangod/Aql/ExecutionPlan.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1191,6 +1191,18 @@ ExecutionNode* ExecutionPlan::fromNodeTraversal(ExecutionNode* previous, AstNode
11911191
std::unique_ptr<Expression> pruneExpression =
11921192
createPruneExpression(this, _ast, node->getMember(3));
11931193

1194+
std::string errorReason;
1195+
if (pruneExpression != nullptr &&
1196+
!pruneExpression->canBeUsedInPrune(_ast->query().vocbase().isOneShard(),
1197+
errorReason)) {
1198+
// PRUNE is designed to be executed inside a DBServer. Therefore, we need a
1199+
// check here and abort in cases which are just not allowed, e.g. execution
1200+
// of user defined JavaScript method or V8 based methods.
1201+
THROW_ARANGO_EXCEPTION_MESSAGE(
1202+
TRI_ERROR_QUERY_PARSE,
1203+
std::string("Invalid PRUNE expression: ") + errorReason);
1204+
}
1205+
11941206
auto options =
11951207
createTraversalOptions(getAst(), direction, node->getMember(4));
11961208

‎arangod/Aql/Expression.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
#include "Basics/StringBuffer.h"
4444
#include "Basics/VPackStringBufferAdapter.h"
4545
#include "Basics/VelocyPackHelper.h"
46+
#include "Cluster/ServerState.h"
4647
#include "Transaction/Context.h"
4748
#include "Transaction/Helpers.h"
4849
#include "Transaction/Methods.h"
@@ -930,6 +931,15 @@ AqlValue Expression::invokeV8Function(ExpressionContext* expressionContext,
930931
/// @brief execute an expression of type SIMPLE, JavaScript variant
931932
AqlValue Expression::executeSimpleExpressionFCallJS(AstNode const* node,
932933
bool& mustDestroy) {
934+
if (ServerState::instance()->isDBServer()) {
935+
// we actually should not get here, but in case we do due to some changes,
936+
// it is better to abort the query with a proper error rather than crashing
937+
// with assertion failure or segfault.
938+
THROW_ARANGO_EXCEPTION_MESSAGE(
939+
TRI_ERROR_NOT_IMPLEMENTED,
940+
"user-defined functions cannot be executed on DB-Servers");
941+
}
942+
933943
auto member = node->getMemberUnchecked(0);
934944
TRI_ASSERT(member->type == NODE_TYPE_ARRAY);
935945

@@ -1704,6 +1714,24 @@ bool Expression::willUseV8() {
17041714
return (_type == SIMPLE && _node->willUseV8());
17051715
}
17061716

1717+
bool Expression::canBeUsedInPrune(bool isOneShard, std::string& errorReason) {
1718+
errorReason.clear();
1719+
1720+
if (willUseV8()) {
1721+
errorReason = "JavaScript expressions cannot be used inside PRUNE";
1722+
return false;
1723+
}
1724+
if (!canRunOnDBServer(isOneShard)) {
1725+
errorReason =
1726+
"PRUNE expression contains a function that cannot be used on "
1727+
"DB-Servers";
1728+
return false;
1729+
}
1730+
1731+
TRI_ASSERT(errorReason.empty());
1732+
return true;
1733+
}
1734+
17071735
std::unique_ptr<Expression> Expression::clone(Ast* ast, bool deepCopy) {
17081736
// We do not need to copy the _ast, since it is managed by the
17091737
// query object and the memory management of the ASTs

‎arangod/Aql/Expression.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDi B41A ff line change
@@ -100,6 +100,9 @@ class Expression {
100100
/// @brief whether or not the expression will use V8
101101
bool willUseV8();
102102

103+
/// @brief whether or not the expression can be used inside a PRUNE statement
104+
bool canBeUsedInPrune(bool isOneShard, std::string& errorReason);
105+
103106
/// @brief clone the expression, needed to clone execution plans
104107
std::unique_ptr<Expression> clone(Ast* ast, bool deepCopy = false);
105108

‎tests/js/common/aql/aql-prune-expressions.js

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/*jshint globalstrict:false, strict:false, maxlen: 500 */
2+
/*global AQL_EXPLAIN, assertEqual, fail */
3+
4+
////////////////////////////////////////////////////////////////////////////////
5+
/// @brief tests for traversal optimization
6+
///
7+
/// DISCLAIMER
8+
///
9+
/// Copyright 2010-2014 triagens GmbH, Cologne, Germany
10+
///
11+
/// Licensed under the Apache License, Version 2.0 (the "License");
12+
/// you may not use this file except in compliance with the License.
13+
/// You may obtain a copy of the License at
14+
///
15+
/// http://www.apache.org/licenses/LICENSE-2.0
16+
///
17+
/// Unless required by applicable law or agreed to in writing, software
18+
/// distributed under the License is distributed on an "AS IS" BASIS,
19+
/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
20+
/// See the License for the specific language governing permissions and
21+
/// limitations under the License.
22+
///
23+
/// Copyright holder is triAGENS GmbH, Cologne, Germany
24+
///
25+
/// @author Jan Steemann
26+
/// @author Copyright 2020, triAGENS GmbH, Cologne, Germany
27+
////////////////////////////////////////////////////////////////////////////////
28+
29+
const jsunity = require("jsunity");
30+
const db = require("@arangodb").db;
31+
const internal = require("internal");
32+
const errors = internal.errors;
33+
34+
function PruneExpressionsSuite() {
35+
const vn = "UnitTestsVertex";
36+
const en = "UnitTestsEdge";
37+
const udf = "test::fuchs";
38+
39+
let cleanup = function () {
40+
db._drop(en);
41+
db._drop(vn);
42+
43+
try {
44+
require("@arangodb/aql/functions").unregister(udf);
45+
} catch (err) {}
46+
};
47+
48+
return {
49+
setUpAll : function () {
50+
cleanup();
51+
52+
require("@arangodb/aql/functions").register(udf, function() { return '0'; });
53+
54+
db._create(vn, { numberOfShards: 1 });
55+
db._createEdgeCollection(en, { distributeShardsLike: vn, numberOfShards: 1 });
56+
57+
db[vn].insert({ _key: "test1", value: 1 });
58+
db[vn].insert({ _key: "test2", value: 2 });
59+
db[vn].insert({ _key: "test3", value: 3 });
60+
61+
db[en].insert({ _from: vn + "/test1", _to: vn + "/test2", value: 1 });
62+
db[en].insert({ _from: vn + "/test2", _to: vn + "/test3", value: 0 });
63+
},
64+
65+
tearDownAll : function () {
66+
cleanup();
67+
},
68+
69+
testAllowedExpression: function () {
70+
let q = `WITH ${vn} FOR v, e, p IN 1..3 OUTBOUND '${vn}/test1' ${en} PRUNE e.value == '0' RETURN v`;
71+
let res = db._query(q).toArray();
72+
assertEqual(2, res.length);
73+
},
74+
75+
testV8Expression: function () {
76+
let q = `WITH ${vn} FOR v, e, p IN 1..3 OUTBOUND '${vn}/test1' ${en} PRUNE e.value == V8('0') RETURN v`;
77+
try {
78+
db._query(q);
79+
fail();
80+
} catch (err) {
81+
assertEqual(err.errorNum, errors.ERROR_QUERY_PARSE.code);
82+
}
83+
},
84+
85+
testUDFExpression: function () {
86+
let q = `WITH ${vn} FOR v, e, p IN 1..3 OUTBOUND '${vn}/test1' ${en} PRUNE e.value == ${udf}() RETURN v`;
87+
try {
88+
db._query(q);
89+
fail();
90+
} catch (err) {
91+
assertEqual(err.errorNum, errors.ERROR_QUERY_PARSE.code);
92+
}
93+
},
94+
95+
testSubqueryExpression: function () {
96+
let q = `WITH ${vn} FOR v, e, p IN 1..3 OUTBOUND '${vn}/test1' ${en} PRUNE e.value == (FOR i IN 1..1 RETURN '0') RETURN v`;
97+
try {
98+
db._query(q);
99+
fail();
100+
} catch (err) {
101+
assertEqual(err.errorNum, errors.ERROR_QUERY_PARSE.code);
102+
}
103+
},
104+
};
105+
}
106+
107+
jsunity.run(PruneExpressionsSuite);
108+
return jsunity.done();

0 commit comments

Comments
 (0)
0