fix(functions): resolve nested ternary CEL expressions in params - #11093
Open
Om-singhaI wants to merge 10 commits into
Open
Om-singhaI wants to merge 10 commits into
Om-singhaI wants to merge 10 commits into
Conversation
The ternary regexps used a greedy (.+) for the right hand side of the
comparison and for each branch, so the last " ? " and " : " in the string
were treated as the delimiters. A nested ternary like
{{ params.PROJECT_ID == "xxx" ? "aaa" : params.PROJECT_ID == "yyy" ? "bbb" : "ccc" }}
parsed as though the condition were params.PROJECT_ID == ("xxx" ? "aaa" :
params.PROJECT_ID == "yyy"), and the emulator then failed to load the
function with "CEL tried to evaluate param. ... in a context which only
permits literal values". The two sibling ternary forms were worse. Their
conditions still parsed, so they selected the wrong branch and returned a
plausible wrong value with no error at all.
A regexp can't express this. Ternaries nest to any depth and the SDK emits
them without parentheses, so pairing each " ? " with the " : " that belongs
to it means counting, and a quoted string literal is allowed to contain
either token. Both need a left to right scan, so this adds a small
splitTernary() helper that tracks quoting and delimiter depth. The condition
it returns is dispatched to the existing comparison evaluators, which leaves
their semantics, type checks and error messages alone, and branches now
resolve recursively so that a branch can be a ternary itself.
Quoted branches containing " ? " or " : " now parse correctly too, which
falls out of the same scan. A backslash inside a literal escapes whatever
follows it, so the quotes that preprocessLists() writes into a list branch
don't pull the scan out of step.
The SDK builds string operands as "${value}" without escaping, so a value
holding a double quote leaves quotes in the body that delimit nothing. The
scan spots those, since a quote that really does open or close a literal sits
next to a space, a bracket or a comma. Once they show up the pairing is
ambiguous, so the scan collects every candidate " : " and takes the first one
whose two branches are both whole. A candidate that cuts a value in half
leaves a branch holding a delimiter that pairs with nothing, and when no
candidate survives that test the body is rejected instead of resolving to a
truncated value.
A " : " with nothing to pair it to anywhere in the body was never a ternary,
so it stays with the comparison evaluators that have always handled it. One
that does pair with a " ? " and still doesn't line up is now an error rather
than something those evaluators get to reinterpret, which used to hand a
string field a boolean.
resolveLiteral() now reports a list it can't parse with this module's own
error type, so a bad split can't surface as a raw SyntaxError from JSON.parse.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request replaces the regular-expression-based parsing of CEL ternary expressions with a manual scanner and parser to support nested ternaries of any depth and correctly handle string literals containing double quotes or delimiters. It also adds comprehensive unit tests for these scenarios. The review feedback points out a redundant check in splitTernary where rescanned.kind === "misquoted" is evaluated, which can be simplified since scanTernary with ignoreQuotes set to true will never return a misquoted status.
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
Author
ajperel
self-requested a review
October 1, 2026 19:43
ajperel
reviewed
Oct 1, 2026
ajperel
left a comment
ajperel
left a comment
Contributor
There was a problem hiding this comment.
I appreciate the fix but I think we can support this with a much simpler solution.
Use the depth counting parser suggested in review in place of the quote recovery scanner. An unpaired " ? " still raises, and the comparison forms now resolve only where a boolean is wanted, so malformed input raises instead of coming back as false.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #7755.
A v1
runWithoption built from a nestedthenElsemakesfirebase-functionsemit a chained CEL ternary into functions.yaml, and the emulator then fails to load the function:The ternary regexps in
src/deploy/functions/cel.tsused a greedy(.+), so the last?and:were taken as the delimiters. The boolean param and dual param forms split in the wrong place too, but silently: on main,{{ params.FLAG ? "a" : params.OTHER ? "b" : "c" }}with FLAG false and OTHER true returns"c".This replaces the three ternary regexps with
parseTernary(), a single pass that skips string literals and pairs the first?with its:by counting depth, as suggested in review. The condition goes to the existing comparison and boolean param evaluators, so their semantics and error messages don't change. A branch that is itself a ternary is resolved recursively with the samewantType.Two behavior changes, both so that malformed input raises
ExprParseErrorinstead of resolving to the wrong type:?with no matching:now raises. On main it fell through to the comparison form.resolveExpression()only resolves the comparison forms when a boolean is wanted. On main,resolveComparison()returned a boolean whatever type the field wanted, so{{ params.FOO == "a" ? "x" }}in a string field came back asfalse. The doc comment onresolveExpression()already calls that an error.Values with unescaped double quotes aren't valid CEL and aren't specially handled.
Scenarios Tested
npx mocha src/deploy/functions/cel.spec.ts src/deploy/functions/params.spec.ts src/deploy/functions/build.spec.ts: 117 passing. cel.spec.ts goes from 41 to 59 tests. With onlycel.tsswapped back to main, 13 fail (12 in cel.spec.ts and 1 in params.spec.ts), including the exact error from the issue.Covered: the issue's expression for all three PROJECT_ID values; nesting in the true branch, the false branch and both; a three level chain; comparison, dual param and boolean param conditions; params, lists and literals as nested branches, with type checking;
?and:inside quoted literals; escaped quotes; a nested ternary interpolated throughresolveString(); a comparison resolved where a boolean isn't wanted; and malformed input (missing or stray delimiters, empty branches,{{ }}).npm run test:compileis clean,prettier --checkpasses, and eslint reports no new warnings.Sample Commands
No commands or flags change.