diff --git a/src/groups/bmq/bmqeval/bmqeval_simpleevaluator.cpp b/src/groups/bmq/bmqeval/bmqeval_simpleevaluator.cpp index fa953a8cc..b88ef3d47 100644 --- a/src/groups/bmq/bmqeval/bmqeval_simpleevaluator.cpp +++ b/src/groups/bmq/bmqeval/bmqeval_simpleevaluator.cpp @@ -82,6 +82,13 @@ void SimpleEvaluator::parse(const bsl::string& expression, context.d_numProperties = 0; context.d_os.reset(); + if (expression.length() > k_MAX_EXPRESSION_LENGTH) { + context.d_os << "expression is too long (" << expression.length() + << "), max allowed length: " << k_MAX_EXPRESSION_LENGTH; + context.d_lastError = ErrorType::e_TOO_LONG; + return; // RETURN + } + bsl::istringstream is(expression, context.d_allocator); SimpleEvaluatorScanner scanner; scanner.switch_streams(&is, &context.d_os); @@ -90,28 +97,27 @@ void SimpleEvaluator::parse(const bsl::string& expression, if (parser.parse() != 0) { context.d_lastError = ErrorType::e_SYNTAX; - return; // RETURN } if (context.d_numOperators > k_MAX_OPERATORS) { - context.d_os << "too many operators"; + context.d_os << "too many operators (" << context.d_numOperators + << "), max allowed operators: " << k_MAX_OPERATORS; context.d_lastError = ErrorType::e_TOO_COMPLEX; - return; // RETURN } if (context.d_numProperties == 0) { context.d_os << "expression does not use any properties"; context.d_lastError = ErrorType::e_NO_PROPERTIES; - return; // RETURN } if (context.d_numProperties > k_MAX_PROPERTIES) { - context.d_os << "expression uses too many properties"; + context.d_os << "expression uses too many properties (" + << context.d_numProperties + << "), max allowed properties: " << k_MAX_PROPERTIES; context.d_lastError = ErrorType::e_TOO_COMPLEX; - return; // RETURN } } @@ -204,8 +210,8 @@ SimpleEvaluator::IntegerLiteral::evaluate(EvaluationContext& context) const // class SimpleEvaluator::BooleanLiteral // ------------------------------------- -bdld::Datum -SimpleEvaluator::BooleanLiteral::evaluate(EvaluationContext& context) const +bdld::Datum SimpleEvaluator::BooleanLiteral::evaluate( + BSLS_ANNOTATION_UNUSED EvaluationContext& context) const { return bdld::Datum::createBoolean(d_value); } diff --git a/src/groups/bmq/bmqeval/bmqeval_simpleevaluator.h b/src/groups/bmq/bmqeval/bmqeval_simpleevaluator.h index 1a071ced6..9222df563 100644 --- a/src/groups/bmq/bmqeval/bmqeval_simpleevaluator.h +++ b/src/groups/bmq/bmqeval/bmqeval_simpleevaluator.h @@ -79,17 +79,25 @@ class EvaluationContext; struct ErrorType { enum Enum { - e_OK = 0 // no error - , + /// no error + e_OK = 0, + e_COMPILATION_FIRST = -100, - e_SYNTAX = -100 // syntax error - , - e_NO_PROPERTIES = -101 // expression does not use any property - , - e_TOO_COMPLEX = -102 // too many properties - // or operators in expression - , - e_COMPILATION_LAST = -102, + + /// syntax error + e_SYNTAX = -100, + + /// expression does not use any property + e_NO_PROPERTIES = -101, + + /// too many properties or operators in expression + e_TOO_COMPLEX = -102, + + /// expression string is too long + e_TOO_LONG = -103, + + e_COMPILATION_LAST = -103, + e_EVALUATION_FIRST = -1, e_NAME = -1, e_TYPE = -2, @@ -492,10 +500,14 @@ class SimpleEvaluator { public: // PUBLIC CONSTANTS enum { + /// The maximum length of an expression string. + k_MAX_EXPRESSION_LENGTH = 128, + /// The maximum number of operators allowed in a single expression. - k_MAX_OPERATORS = 10, + k_MAX_OPERATORS = 10, + + /// The maximum number of properties allowed in a single expression. k_MAX_PROPERTIES = 10 - // The maximum number of properties allowed in a single expression. }; // CREATORS @@ -706,19 +718,36 @@ class EvaluationContext { inline const char* ErrorType::toString(ErrorType::Enum value) { switch (value) { - case 0: return ""; // RETURN - case bmqeval::ErrorType::e_SYNTAX: return "syntax error"; // RETURN - case bmqeval::ErrorType::e_TOO_COMPLEX: - return "subscription expression is too complex"; // RETURN - case bmqeval::ErrorType::e_NO_PROPERTIES: + case bmqeval::ErrorType::e_OK: { + return ""; // RETURN + } + case bmqeval::ErrorType::e_SYNTAX: { + return "syntax error"; // RETURN + } + case bmqeval::ErrorType::e_NO_PROPERTIES: { return "expression does not use any property"; // RETURN - case bmqeval::ErrorType::e_NAME: + } + case bmqeval::ErrorType::e_TOO_COMPLEX: { + return "subscription expression is too complex"; // RETURN + } + case bmqeval::ErrorType::e_TOO_LONG: { + return "subscription expression is too long"; // RETURN + } + case bmqeval::ErrorType::e_NAME: { return "undefined name in subscription expression"; // RETURN - case bmqeval::ErrorType::e_TYPE: + } + case bmqeval::ErrorType::e_TYPE: { return "type error in expression"; // RETURN - case bmqeval::ErrorType::e_BINARY: + } + case bmqeval::ErrorType::e_BINARY: { return "binary properties are not supported"; // RETURN - default: return "unknown error"; // RETURN + } + case bmqeval::ErrorType::e_UNDEFINED: { + return "undefined error"; // RETURN + } + default: { + return "unknown error"; // RETURN + } } } diff --git a/src/groups/bmq/bmqeval/bmqeval_simpleevaluator.t.cpp b/src/groups/bmq/bmqeval/bmqeval_simpleevaluator.t.cpp index 0bb8048e9..7af8e45f8 100644 --- a/src/groups/bmq/bmqeval/bmqeval_simpleevaluator.t.cpp +++ b/src/groups/bmq/bmqeval/bmqeval_simpleevaluator.t.cpp @@ -61,9 +61,9 @@ class MockPropertiesReader : public PropertiesReader { // MANIPULATORS /// Return a `bdld::Datum` object with value for the specified `name`. - /// Use the specified `allocator` for any memory allocation. - virtual bdld::Datum get(const bsl::string& name, - bslma::Allocator* allocator) + /// The `bslma::Allocator*` argument is unused. + bdld::Datum get(const bsl::string& name, + bslma::Allocator*) BSLS_KEYWORD_OVERRIDE { bsl::unordered_map::const_iterator iter = d_map.find(name); @@ -124,6 +124,30 @@ static bsl::string makeTooManyOperators() return os.str(); } +static bsl::string makeTooLongExpression() +{ + mwcu::MemOutStream os(s_allocator_p); + + // Note that we want to create `k_STACK_SIZE` nested NOT objects in AST, + // and when we call destructor chain for all these objects, we'll need + // much more memory on a stack, since each destructor address is not a + // single byte, and also we need to call shared_ptr and Not destructors in + // pairs. But the initial guess of `k_STACK_SIZE` is more than sufficient + // to cause segfault if this case is not handled properly. + const size_t k_STACK_SIZE = 1024 * 1024; + ASSERT(SimpleEvaluator::k_MAX_EXPRESSION_LENGTH < k_STACK_SIZE); + + for (size_t i = 0; i < k_STACK_SIZE; i += 16) { + // Combining `!` and `~` differently in case we want to introduce + // "NOT" optimization one day (remove double sequential "NOTs") + os << "!!!!~~~~!~!~!!~~"; + } + + os << "x"; + + return os.str(); +} + static void test1_compilationErrors() { struct TestParameters { @@ -139,10 +163,13 @@ static void test1_compilationErrors() "syntax error, unexpected invalid character at offset 3"}, {makeTooManyOperators(), ErrorType::e_TOO_COMPLEX, - "too many operators"}, + "too many operators (12), max allowed operators: 10"}, {"true && true", ErrorType::e_NO_PROPERTIES, "expression does not use any properties"}, + {makeTooLongExpression(), + ErrorType::e_TOO_LONG, + "expression is too long (1048577), max allowed length: 128"}, // only C-style operator variants supported {"val = 42", @@ -157,6 +184,27 @@ static void test1_compilationErrors() {"val <> 42", ErrorType::e_SYNTAX, "syntax error, unexpected > at offset 5"}, + + // unsupported_ints + {"i_0 != 9223372036854775808", + ErrorType::e_SYNTAX, + "integer overflow at offset 7"}, // 2 ** 63 + {"i_0 != -170141183460469231731687303715884105728", + ErrorType::e_SYNTAX, + "integer overflow at offset 7"}, // -(2 ** 127) + {"i_0 != 170141183460469231731687303715884105727", + ErrorType::e_SYNTAX, + "integer overflow at offset 7"}, // 2 ** 127 - 1 + {"i_0 != " + "-5789604461865809771178549250434395392663499233282028201972879200395" + "6564819968", + ErrorType::e_SYNTAX, + "integer overflow at offset 7"}, // -(2 ** 255) + {"i_0 != " + "57896044618658097711785492504343953926634992332820282019728792003956" + "564819967", + ErrorType::e_SYNTAX, + "integer overflow at offset 7"}, // 2 ** 255 - 1 }; const TestParameters* testParametersEnd = testParameters + sizeof(testParameters) / @@ -287,7 +335,6 @@ static void test3_evaluation() struct TestParameters { const char* expression; bool expected; - bool expectCompilationFailure; } testParameters[] = { {"b_true", true}, @@ -427,25 +474,6 @@ static void test3_evaluation() {"i_0 != -9223372036854775807", true}, // -(2 ** 63) + 1 {"i_0 != 9223372036854775807", true}, // 2 ** 63 - 1 {"i_0 != -9223372036854775808", true}, // -(2 ** 63) - - // unsupported_ints - {"i_0 != 9223372036854775808", false, true}, // 2 ** 63 - {"i_0 != -170141183460469231731687303715884105728", - false, - true}, // -(2 ** 127) - {"i_0 != 170141183460469231731687303715884105727", - false, - true}, // 2 ** 127 - 1 - {"i_0 != " - "-5789604461865809771178549250434395392663499233282028201972879200395" - "6564819968", - false, - true}, // -(2 ** 255) - {"i_0 != " - "57896044618658097711785492504343953926634992332820282019728792003956" - "564819967", - false, - true}, // 2 ** 255 - 1 }; const TestParameters* testParametersEnd = testParameters + sizeof(testParameters) / @@ -461,20 +489,12 @@ static void test3_evaluation() ASSERT(!evaluator.isValid()); - if (int rc = evaluator.compile(parameters->expression, - compilationContext)) { - if (!parameters->expectCompilationFailure) { - PV(bsl::string("UNEXPECTED: ") + - compilationContext.lastErrorMessage()); - ASSERT(false); - } + if (evaluator.compile(parameters->expression, compilationContext)) { + PV(bsl::string("UNEXPECTED: ") + + compilationContext.lastErrorMessage()); + ASSERT(false); } else { - if (parameters->expectCompilationFailure) { - PV("Expected compilation failure"); - ASSERT(false); - } - ASSERT(evaluator.isValid()); ASSERT_EQ(evaluator.evaluate(evaluationContext), parameters->expected); diff --git a/src/groups/bmq/bmqeval/bmqeval_simpleevaluatorparser.y b/src/groups/bmq/bmqeval/bmqeval_simpleevaluatorparser.y index 1e1f1aaca..cfcd81e15 100644 --- a/src/groups/bmq/bmqeval/bmqeval_simpleevaluatorparser.y +++ b/src/groups/bmq/bmqeval/bmqeval_simpleevaluatorparser.y @@ -112,7 +112,7 @@ expression { $$ = ctx.makeLiteral($1); } | OVERFLOW { - ctx.d_os << " integer overflow at offset " << scanner.lastTokenLocation(); + ctx.d_os << "integer overflow at offset " << scanner.lastTokenLocation(); YYABORT; } | STRING