diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index cc63be523c..35f6486b2c 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -183,6 +183,65 @@ def _is_single_expression(stripped: str) -> bool: return True +def _interpolate_expressions(template: str, namespace: dict[str, Any]) -> str: + """Substitute every top-level ``{{ ... }}`` block in *template*, quote-aware. + + Walks the template and, for each block, finds the closing ``}}`` that lies + outside string literals -- the same quote-scanning used by + ``_is_single_expression``. This keeps a literal ``}}`` inside a string + argument (e.g. ``| default('}}')``) from prematurely closing a block. + + ``_EXPR_PATTERN.sub`` cannot do this: its non-greedy body stops at the first + ``}}`` regardless of quoting, so in a multi-expression template any block + whose argument contains a literal ``}}`` is captured truncated and mis-parsed + (raising ``ValueError`` from the filter parser). #3208/#3228 fixed exactly + this for the single-expression fast path but left the interpolation path on + the old regex. + """ + out: list[str] = [] + i = 0 + n = len(template) + while i < n: + start = template.find("{{", i) + if start == -1: + out.append(template[i:]) + break + out.append(template[i:start]) + # Scan for the block-closing ``}}`` that is outside any string literal. + j = start + 2 + quote: str | None = None + close = -1 + while j < n: + ch = template[j] + if quote is not None: + if ch == quote: + quote = None + elif ch in ("'", '"'): + quote = ch + elif ch == "}" and j + 1 < n and template[j + 1] == "}": + close = j + break + j += 1 + if close == -1: + # No quote-aware close. Two sub-cases, both kept identical to the old + # regex so a malformed template is never silently hidden: + # * a raw ``}}`` still exists in the tail (e.g. an unbalanced quote + # in a filter arg swallowed the real delimiter) -- fall back to + # that first raw ``}}`` and evaluate, letting the parser surface + # a ValueError just as ``_EXPR_PATTERN.sub`` would have. + # * no ``}}`` at all -- a genuinely unterminated ``{{``; leave the + # tail verbatim, again matching the regex (which cannot match). + raw_close = template.find("}}", start + 2) + if raw_close == -1: + out.append(template[start:]) + break + close = raw_close + val = _evaluate_simple_expression(template[start + 2:close].strip(), namespace) + out.append(str(val) if val is not None else "") + i = close + 2 + return "".join(out) + + def _split_top_level_commas(text: str) -> list[str]: """Split *text* on commas that are not inside quotes or nested brackets. @@ -472,12 +531,11 @@ def evaluate_expression(template: str, context: Any) -> Any: if _is_single_expression(stripped): return _evaluate_simple_expression(stripped[2:-2].strip(), namespace) - # Multi-expression: string interpolation - def _replacer(m: re.Match[str]) -> str: - val = _evaluate_simple_expression(m.group(1).strip(), namespace) - return str(val) if val is not None else "" - - return _EXPR_PATTERN.sub(_replacer, template) + # Multi-expression: interpolate each block inline. Uses a quote-aware scan + # (not ``_EXPR_PATTERN.sub``) so a literal ``}}`` inside a string argument + # in any block does not close that block early -- matching the handling the + # single-expression path above already got in #3208/#3228. + return _interpolate_expressions(template, namespace) def evaluate_condition(condition: str, context: Any) -> bool: diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 2fdbf887b3..c0d8d22374 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -260,6 +260,60 @@ def test_single_expression_with_literal_braces_preserves_type(self): ctx = StepContext(inputs={"text": "uses }} syntax"}) assert evaluate_expression("{{ inputs.text | contains('}}') }}", ctx) is True + def test_multi_expression_with_literal_close_brace_in_argument(self): + """A multi-expression template with a literal ``}}`` inside a string + argument must interpolate, not raise. #3208/#3228 hardened the single- + expression fast path for literal braces but left the interpolation path + on ``_EXPR_PATTERN``, whose non-greedy body stops at the first ``}}`` -- + so the block was captured truncated and the filter parser raised + ValueError.""" + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"name": "Bob", "missing": None}) + # ``}}`` in the default fallback of the second block. + result = evaluate_expression( + "{{ inputs.name }}: {{ inputs.missing | default('}}') }}", ctx + ) + assert result == "Bob: }}" + # ``}}`` in the first block, expression following it. + result = evaluate_expression( + "{{ inputs.missing | default('}}') }} / {{ inputs.name }}", ctx + ) + assert result == "}} / Bob" + + def test_multi_expression_with_literal_open_brace_in_argument(self): + """A literal ``{{`` inside a string argument in a multi-expression + template must not confuse block detection either.""" + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"name": "Bob", "missing": None}) + result = evaluate_expression( + "{{ inputs.name }} {{ inputs.missing | default('{{') }}", ctx + ) + assert result == "Bob {{" + + def test_multi_expression_unbalanced_quote_still_raises(self): + """A malformed block (an unbalanced quote in a filter arg) must still + surface a ValueError, not be silently emitted verbatim. + + The quote-aware scan never finds a block-closing ``}}`` when a quote is + left open, but a raw ``}}`` is still present in the tail. It must fall + back to that raw delimiter and evaluate — same as the old regex path — + so a typo fails loudly instead of being hidden (Copilot review on + #3307).""" + import pytest + + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"name": "Bob", "missing": None}) + with pytest.raises(ValueError): + evaluate_expression( + "{{ inputs.name }} {{ inputs.missing | default('oops }}", ctx + ) + def test_comparison_equals(self): from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext