Skip to content

Commit 3b4e7f3

Browse files
fix(workflows): quote-aware interpolation so a literal }} in a filter arg doesn't break multi-expression templates (#3307)
* fix(workflows): quote-aware interpolation so a literal }} in a filter arg doesn't break multi-expression templates #3208/#3228 hardened the single-expression fast path (_is_single_expression) so a literal {{ or }} inside a string argument like `| default('}}')` stays on the typed path. the multi-expression interpolation path was left on the old _EXPR_PATTERN regex, whose non-greedy `(.+?)}}` body stops at the first }} regardless of quoting. so a multi-expression template with a literal }} in any block captured a truncated body, hit the filter parser malformed, and raised ValueError. e.g. `{{ inputs.name }}: {{ inputs.missing | default('}}') }}` raised instead of interpolating. replace _EXPR_PATTERN.sub with _interpolate_expressions, which scans each block for a }} outside string literals - the same quote handling _is_single_expression already uses. plain-value passthrough (a literal }} in a resolved value, not an expression) is unchanged. add regression tests for a literal }} in the second block and in the first block, plus a literal {{ guard. * fix(workflows): surface malformed templates in interpolation instead of emitting verbatim address copilot review on #3307: when the quote-aware scan finds no block-closing `}}` (e.g. an unbalanced quote in a filter arg swallowed the delimiter), fall back to the first raw `}}` in the tail and evaluate it, so the parser raises ValueError just as the old _EXPR_PATTERN.sub path did. only when there is no `}}` at all is the tail left verbatim (a genuinely unterminated `{{`, which the regex also could not match). keeps a typo failing loudly rather than being silently hidden. add a regression test for an unbalanced quote in a multi-expression template.
1 parent f494a8e commit 3b4e7f3

2 files changed

Lines changed: 118 additions & 6 deletions

File tree

src/specify_cli/workflows/expressions.py

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,65 @@ def _is_single_expression(stripped: str) -> bool:
183183
return True
184184

185185

186+
def _interpolate_expressions(template: str, namespace: dict[str, Any]) -> str:
187+
"""Substitute every top-level ``{{ ... }}`` block in *template*, quote-aware.
188+
189+
Walks the template and, for each block, finds the closing ``}}`` that lies
190+
outside string literals -- the same quote-scanning used by
191+
``_is_single_expression``. This keeps a literal ``}}`` inside a string
192+
argument (e.g. ``| default('}}')``) from prematurely closing a block.
193+
194+
``_EXPR_PATTERN.sub`` cannot do this: its non-greedy body stops at the first
195+
``}}`` regardless of quoting, so in a multi-expression template any block
196+
whose argument contains a literal ``}}`` is captured truncated and mis-parsed
197+
(raising ``ValueError`` from the filter parser). #3208/#3228 fixed exactly
198+
this for the single-expression fast path but left the interpolation path on
199+
the old regex.
200+
"""
201+
out: list[str] = []
202+
i = 0
203+
n = len(template)
204+
while i < n:
205+
start = template.find("{{", i)
206+
if start == -1:
207+
out.append(template[i:])
208+
break
209+
out.append(template[i:start])
210+
# Scan for the block-closing ``}}`` that is outside any string literal.
211+
j = start + 2
212+
quote: str | None = None
213+
close = -1
214+
while j < n:
215+
ch = template[j]
216+
if quote is not None:
217+
if ch == quote:
218+
quote = None
219+
elif ch in ("'", '"'):
220+
quote = ch
221+
elif ch == "}" and j + 1 < n and template[j + 1] == "}":
222+
close = j
223+
break
224+
j += 1
225+
if close == -1:
226+
# No quote-aware close. Two sub-cases, both kept identical to the old
227+
# regex so a malformed template is never silently hidden:
228+
# * a raw ``}}`` still exists in the tail (e.g. an unbalanced quote
229+
# in a filter arg swallowed the real delimiter) -- fall back to
230+
# that first raw ``}}`` and evaluate, letting the parser surface
231+
# a ValueError just as ``_EXPR_PATTERN.sub`` would have.
232+
# * no ``}}`` at all -- a genuinely unterminated ``{{``; leave the
233+
# tail verbatim, again matching the regex (which cannot match).
234+
raw_close = template.find("}}", start + 2)
235+
if raw_close == -1:
236+
out.append(template[start:])
237+
break
238+
close = raw_close
239+
val = _evaluate_simple_expression(template[start + 2:close].strip(), namespace)
240+
out.append(str(val) if val is not None else "")
241+
i = close + 2
242+
return "".join(out)
243+
244+
186245
def _split_top_level_commas(text: str) -> list[str]:
187246
"""Split *text* on commas that are not inside quotes or nested brackets.
188247
@@ -472,12 +531,11 @@ def evaluate_expression(template: str, context: Any) -> Any:
472531
if _is_single_expression(stripped):
473532
return _evaluate_simple_expression(stripped[2:-2].strip(), namespace)
474533

475-
# Multi-expression: string interpolation
476-
def _replacer(m: re.Match[str]) -> str:
477-
val = _evaluate_simple_expression(m.group(1).strip(), namespace)
478-
return str(val) if val is not None else ""
479-
480-
return _EXPR_PATTERN.sub(_replacer, template)
534+
# Multi-expression: interpolate each block inline. Uses a quote-aware scan
535+
# (not ``_EXPR_PATTERN.sub``) so a literal ``}}`` inside a string argument
536+
# in any block does not close that block early -- matching the handling the
537+
# single-expression path above already got in #3208/#3228.
538+
return _interpolate_expressions(template, namespace)
481539

482540

483541
def evaluate_condition(condition: str, context: Any) -> bool:

tests/test_workflows.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,60 @@ def test_single_expression_with_literal_braces_preserves_type(self):
260260
ctx = StepContext(inputs={"text": "uses }} syntax"})
261261
assert evaluate_expression("{{ inputs.text | contains('}}') }}", ctx) is True
262262

263+
def test_multi_expression_with_literal_close_brace_in_argument(self):
264+
"""A multi-expression template with a literal ``}}`` inside a string
265+
argument must interpolate, not raise. #3208/#3228 hardened the single-
266+
expression fast path for literal braces but left the interpolation path
267+
on ``_EXPR_PATTERN``, whose non-greedy body stops at the first ``}}`` --
268+
so the block was captured truncated and the filter parser raised
269+
ValueError."""
270+
from specify_cli.workflows.expressions import evaluate_expression
271+
from specify_cli.workflows.base import StepContext
272+
273+
ctx = StepContext(inputs={"name": "Bob", "missing": None})
274+
# ``}}`` in the default fallback of the second block.
275+
result = evaluate_expression(
276+
"{{ inputs.name }}: {{ inputs.missing | default('}}') }}", ctx
277+
)
278+
assert result == "Bob: }}"
279+
# ``}}`` in the first block, expression following it.
280+
result = evaluate_expression(
281+
"{{ inputs.missing | default('}}') }} / {{ inputs.name }}", ctx
282+
)
283+
assert result == "}} / Bob"
284+
285+
def test_multi_expression_with_literal_open_brace_in_argument(self):
286+
"""A literal ``{{`` inside a string argument in a multi-expression
287+
template must not confuse block detection either."""
288+
from specify_cli.workflows.expressions import evaluate_expression
289+
from specify_cli.workflows.base import StepContext
290+
291+
ctx = StepContext(inputs={"name": "Bob", "missing": None})
292+
result = evaluate_expression(
293+
"{{ inputs.name }} {{ inputs.missing | default('{{') }}", ctx
294+
)
295+
assert result == "Bob {{"
296+
297+
def test_multi_expression_unbalanced_quote_still_raises(self):
298+
"""A malformed block (an unbalanced quote in a filter arg) must still
299+
surface a ValueError, not be silently emitted verbatim.
300+
301+
The quote-aware scan never finds a block-closing ``}}`` when a quote is
302+
left open, but a raw ``}}`` is still present in the tail. It must fall
303+
back to that raw delimiter and evaluate — same as the old regex path —
304+
so a typo fails loudly instead of being hidden (Copilot review on
305+
#3307)."""
306+
import pytest
307+
308+
from specify_cli.workflows.expressions import evaluate_expression
309+
from specify_cli.workflows.base import StepContext
310+
311+
ctx = StepContext(inputs={"name": "Bob", "missing": None})
312+
with pytest.raises(ValueError):
313+
evaluate_expression(
314+
"{{ inputs.name }} {{ inputs.missing | default('oops }}", ctx
315+
)
316+
263317
def test_comparison_equals(self):
264318
from specify_cli.workflows.expressions import evaluate_expression
265319
from specify_cli.workflows.base import StepContext

0 commit comments

Comments
 (0)