Skip to content

Commit 75bfde6

Browse files
committed
Reject expressions with attached keyword tokens
Keywords (word operators, states, event states, flags, integers, datetimes) matched via bare prefix rules, so glued tokens like "completeand" or "515and" were silently split in two and misparsed instead of rejected. Require a trailing word boundary on each, via an inlined zero-width lookahead kept inside the lexeme scope; the boundary also counts '.' so a keyword glued to a relative path ("and./12") is rejected. Also make variable paths contiguous (no space around ':'). Re ECFLOW-2112
1 parent 4dfed05 commit 75bfde6

1 file changed

Lines changed: 49 additions & 22 deletions

File tree

libs/node/src/ecflow/node/ExprParser.cpp

Lines changed: 49 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,23 @@ using namespace boost::spirit;
5757
using namespace phoenix;
5858
using namespace BOOST_SPIRIT_CLASSIC_NS;
5959

60+
// ECFLOW-2112:
61+
// A keyword/word-operator token (and/or/eq/ne/ge/gt/le/lt, a node state, an event state, a flag name, an
62+
// integer or a datetime) must be a complete token: it must NOT be immediately followed by a character that
63+
// could continue an identifier. Otherwise 'glued' tokens such as 'completeand', '515and', 'andb' or
64+
// 'and../ret' would be silently (mis)parsed as two separate tokens, which silently changes the meaning of the
65+
// expression.
66+
//
67+
// The identifier-continuation characters are those allowed inside a name (letter, digit, '_' or '.') -- the
68+
// '.' is included because a name may contain it (e.g. '4dvar') and a relative path begins with it, so a
69+
// keyword glued to a following '.'/path (e.g. 'and./12') must also be rejected.
70+
//
71+
// WORD_BOUNDARY is a zero-width negative-lookahead assertion (it consumes no input) that fails when the next
72+
// character is an identifier-continuation character. It MUST be used inline, inside a lexeme/leaf/token
73+
// directive (never via a separate rule<>), so that the phrase-level whitespace skipper is not re-enabled and
74+
// the assertion is evaluated at the exact character immediately following the token.
75+
#define WORD_BOUNDARY (~epsilon_p(alnum_p | ch_p('_') | ch_p('.')))
76+
6077
/////////////////////////////////////////////////////////////////////////////
6178
using treematch_t = tree_match<const char*>;
6279
using tree_iter_t = treematch_t::tree_iterator;
@@ -224,7 +241,7 @@ struct ExpressionGrammer : public grammar<ExpressionGrammer>
224241

225242
// Integer is distinct from task/family names that are integers, since nodes with integer
226243
// names that occur in trigger/complete expression must have path ./0 ./1
227-
integer = leaf_node_d[uint_p];
244+
integer = leaf_node_d[uint_p >> WORD_BOUNDARY];
228245
plus = root_node_d[str_p("+")];
229246
minus = root_node_d[str_p("-")];
230247
divide = root_node_d[str_p("/")];
@@ -233,19 +250,19 @@ struct ExpressionGrammer : public grammar<ExpressionGrammer>
233250
operators = plus | minus | divide | multiply | modulo;
234251

235252
equal_1 = root_node_d[str_p("==")];
236-
equal_2 = root_node_d[str_p("eq")];
253+
equal_2 = lexeme_d[root_node_d[str_p("eq")] >> WORD_BOUNDARY];
237254
not_equal_1 = root_node_d[str_p("!=")];
238-
not_equal_2 = root_node_d[str_p("ne")];
255+
not_equal_2 = lexeme_d[root_node_d[str_p("ne")] >> WORD_BOUNDARY];
239256
equality_comparible = equal_1 | equal_2 | not_equal_2 | not_equal_1;
240257

241258
greater_equals_1 = root_node_d[str_p(">=")];
242-
greater_equals_2 = root_node_d[str_p("ge")];
259+
greater_equals_2 = lexeme_d[root_node_d[str_p("ge")] >> WORD_BOUNDARY];
243260
less_equals_1 = root_node_d[str_p("<=")];
244-
less_equals_2 = root_node_d[str_p("le")];
261+
less_equals_2 = lexeme_d[root_node_d[str_p("le")] >> WORD_BOUNDARY];
245262
less_than_1 = root_node_d[str_p("<")];
246-
less_than_2 = root_node_d[str_p("lt")];
263+
less_than_2 = lexeme_d[root_node_d[str_p("lt")] >> WORD_BOUNDARY];
247264
greater_than_1 = root_node_d[str_p(">")];
248-
greater_than_2 = root_node_d[str_p("gt")];
265+
greater_than_2 = lexeme_d[root_node_d[str_p("gt")] >> WORD_BOUNDARY];
249266
// Prioritise to most common first, to speed up parsing
250267
less_than_comparable = greater_equals_2 | less_equals_2 | greater_than_2 | less_than_2 | greater_equals_1 |
251268
less_equals_1 | less_than_1 | greater_than_1;
@@ -255,28 +272,34 @@ struct ExpressionGrammer : public grammar<ExpressionGrammer>
255272
not3_r = root_node_d[str_p("!")];
256273
not_r = not1_r | not3_r | not2_r;
257274

258-
and_r = root_node_d[str_p("and")] || root_node_d[str_p("&&")] || root_node_d[str_p("AND")];
259-
or_r = root_node_d[str_p("or")] || root_node_d[str_p("||")] || root_node_d[str_p("OR")];
275+
and_r = lexeme_d[root_node_d[str_p("and")] >> WORD_BOUNDARY] || root_node_d[str_p("&&")] ||
276+
lexeme_d[root_node_d[str_p("AND")] >> WORD_BOUNDARY];
277+
or_r = lexeme_d[root_node_d[str_p("or")] >> WORD_BOUNDARY] || root_node_d[str_p("||")] ||
278+
lexeme_d[root_node_d[str_p("OR")] >> WORD_BOUNDARY];
260279
and_or = and_r | or_r;
261280

262-
event_state = leaf_node_d[str_p("set")] || leaf_node_d[str_p("clear")];
281+
event_state = leaf_node_d[str_p("set") >> WORD_BOUNDARY] || leaf_node_d[str_p("clear") >> WORD_BOUNDARY];
263282

264-
node_state_unknown = root_node_d[str_p("unknown")];
265-
node_state_complete = root_node_d[str_p("complete")];
266-
node_state_queued = root_node_d[str_p("queued")];
267-
node_state_submitted = root_node_d[str_p("submitted")];
268-
node_state_active = root_node_d[str_p("active")];
269-
node_state_aborted = root_node_d[str_p("aborted")];
283+
node_state_unknown = lexeme_d[root_node_d[str_p("unknown")] >> WORD_BOUNDARY];
284+
node_state_complete = lexeme_d[root_node_d[str_p("complete")] >> WORD_BOUNDARY];
285+
node_state_queued = lexeme_d[root_node_d[str_p("queued")] >> WORD_BOUNDARY];
286+
node_state_submitted = lexeme_d[root_node_d[str_p("submitted")] >> WORD_BOUNDARY];
287+
node_state_active = lexeme_d[root_node_d[str_p("active")] >> WORD_BOUNDARY];
288+
node_state_aborted = lexeme_d[root_node_d[str_p("aborted")] >> WORD_BOUNDARY];
270289
nodestate = node_state_complete | node_state_aborted | node_state_queued | node_state_active |
271290
node_state_submitted | node_state_unknown;
272291

273-
flag_late = root_node_d[str_p("late")];
274-
flag_zombie = root_node_d[str_p("zombie")];
275-
flag_archived = root_node_d[str_p("archived")];
292+
flag_late = lexeme_d[root_node_d[str_p("late")] >> WORD_BOUNDARY];
293+
flag_zombie = lexeme_d[root_node_d[str_p("zombie")] >> WORD_BOUNDARY];
294+
flag_archived = lexeme_d[root_node_d[str_p("archived")] >> WORD_BOUNDARY];
276295
flag = flag_late | flag_zombie | flag_archived;
277296

278-
variable = leaf_node_d[nodename];
279-
basic_variable_path = nodepath >> discard_node_d[ch_p(':')] >> variable;
297+
variable = leaf_node_d[nodename];
298+
// ECFLOW-2112: a variable path must be a contiguous token, i.e. no whitespace is allowed around the
299+
// ':' separator (a legitimate variable path is always written as 'node:VAR'). This prevents a glued
300+
// token such as '515and' (a valid node name, like '4dvar') from being combined with a following
301+
// ' :VAR' to (mis)parse '<number>and :VAR' as a variable reference.
302+
basic_variable_path = lexeme_d[nodepath >> discard_node_d[ch_p(':')] >> variable];
280303
parent_variable =
281304
ch_p(':') >> variable; // if we discard_node, then we get just 'variable' and NOT parent_variable
282305

@@ -288,7 +311,7 @@ struct ExpressionGrammer : public grammar<ExpressionGrammer>
288311
str_p("cal::date_to_julian") >> discard_node_d[ch_p('(')] >> cal_argument >> discard_node_d[ch_p(')')];
289312
cal_julian_to_date =
290313
str_p("cal::julian_to_date") >> discard_node_d[ch_p('(')] >> cal_argument >> discard_node_d[ch_p(')')];
291-
datetime = leaf_node_d[lexeme_d[repeat_p(8)[digit_p] >> 'T' >> repeat_p(6)[digit_p]]];
314+
datetime = leaf_node_d[lexeme_d[repeat_p(8)[digit_p] >> 'T' >> repeat_p(6)[digit_p] >> WORD_BOUNDARY]];
292315

293316
calc_factor = datetime | integer | basic_variable_path |
294317
discard_node_d[ch_p('(')] >> calc_expression >> discard_node_d[ch_p(')')] | flag_path |
@@ -356,6 +379,10 @@ struct ExpressionGrammer : public grammar<ExpressionGrammer>
356379
};
357380
};
358381

382+
// WORD_BOUNDARY is only needed by the grammar definition above; undefine it so the
383+
// macro does not leak into the rest of this translation unit.
384+
#undef WORD_BOUNDARY
385+
359386
/////////////////////////////////////////////////////////////////////////////////////////////
360387

361388
void print(tree_parse_info<> info, const std::string& expr, const std::map<parser_id, std::string>& rule_names);

0 commit comments

Comments
 (0)