From 01ec444fecbb3198565916b3d4acfe7cdafb8549 Mon Sep 17 00:00:00 2001 From: Om Singhal Date: Wed, 16 Sep 2026 01:29:57 -0400 Subject: [PATCH] fix(functions): resolve nested ternary CEL expressions in params 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. --- CHANGELOG.md | 1 + src/deploy/functions/cel.spec.ts | 324 +++++++++++++++++++++++++++ src/deploy/functions/cel.ts | 329 ++++++++++++++++++++++------ src/deploy/functions/params.spec.ts | 15 ++ 4 files changed, 598 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94dacb219c5..4107277c728 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,2 +1,3 @@ - [changed] Improve formatting and user experience for Cloud Functions parameter and secret prompts. - Updated the Firebase SQL Connect local toolkit to v3.4.20. +- [fixed] Resolve nested ternary CEL expressions in function parameters, which previously failed to load or selected the wrong branch. (#7755) diff --git a/src/deploy/functions/cel.spec.ts b/src/deploy/functions/cel.spec.ts index 09d6549e50f..1d9f7d5d248 100644 --- a/src/deploy/functions/cel.spec.ts +++ b/src/deploy/functions/cel.spec.ts @@ -1018,6 +1018,24 @@ describe("CEL evaluation", () => { ).to.equal("baz"); }); + it("it resolves a ternary nested in a branch of a dual comparison ternary", () => { + const expr = '{{ params.FOO == params.BAR ? "a" : params.FOO == params.BAZ ? "b" : "c" }}'; + expect( + resolveExpression("string", expr, { + FOO: stringV("a"), + BAR: stringV("q"), + BAZ: stringV("a"), + }), + ).to.equal("b"); + expect( + resolveExpression("string", expr, { + FOO: stringV("a"), + BAR: stringV("q"), + BAZ: stringV("r"), + }), + ).to.equal("c"); + }); + it("it knows how to handle non-== comparisons by delegating to the Comparison expression evaluators", () => { expect( resolveExpression("number", "{{ params.FOO != params.BAR ? params.IF_T : params.IF_F }}", { @@ -1170,5 +1188,311 @@ describe("CEL evaluation", () => { }), ).to.equal("baz"); }); + + it("it resolves a ternary nested in a branch of a boolean conditioned ternary", () => { + const expr = '{{ params.FLAG ? "a" : params.OTHER ? "b" : "c" }}'; + expect(resolveExpression("string", expr, { FLAG: boolV(true), OTHER: boolV(true) })).to.equal( + "a", + ); + expect( + resolveExpression("string", expr, { FLAG: boolV(false), OTHER: boolV(true) }), + ).to.equal("b"); + expect( + resolveExpression("string", expr, { FLAG: boolV(false), OTHER: boolV(false) }), + ).to.equal("c"); + }); + }); + + describe("Nested ternary expressions", () => { + it("it resolves a ternary nested in the false branch", () => { + const expr = + '{{ params.PROJECT_ID == "xxx" ? "aaa" : params.PROJECT_ID == "yyy" ? "bbb" : "ccc" }}'; + expect(resolveExpression("string", expr, { PROJECT_ID: stringV("xxx") })).to.equal("aaa"); + expect(resolveExpression("string", expr, { PROJECT_ID: stringV("yyy") })).to.equal("bbb"); + expect(resolveExpression("string", expr, { PROJECT_ID: stringV("zzz") })).to.equal("ccc"); + }); + + it("it resolves a nested ternary with number branches", () => { + const expr = '{{ params.PROJECT_ID == "xxx" ? 1 : params.PROJECT_ID == "yyy" ? 2 : 3 }}'; + expect(resolveExpression("number", expr, { PROJECT_ID: stringV("xxx") })).to.equal(1); + expect(resolveExpression("number", expr, { PROJECT_ID: stringV("yyy") })).to.equal(2); + expect(resolveExpression("number", expr, { PROJECT_ID: stringV("zzz") })).to.equal(3); + }); + + it("it resolves a ternary nested in the true branch", () => { + const expr = '{{ params.FOO == "x" ? params.BAR == "u" ? "a" : "b" : "c" }}'; + expect(resolveExpression("string", expr, { FOO: stringV("x"), BAR: stringV("u") })).to.equal( + "a", + ); + expect(resolveExpression("string", expr, { FOO: stringV("x"), BAR: stringV("v") })).to.equal( + "b", + ); + expect(resolveExpression("string", expr, { FOO: stringV("y"), BAR: stringV("u") })).to.equal( + "c", + ); + }); + + it("it resolves a chain three levels deep", () => { + const expr = + '{{ params.FOO == "a" ? 1 : params.FOO == "b" ? 2 : params.FOO == "c" ? 3 : 4 }}'; + expect(resolveExpression("number", expr, { FOO: stringV("a") })).to.equal(1); + expect(resolveExpression("number", expr, { FOO: stringV("b") })).to.equal(2); + expect(resolveExpression("number", expr, { FOO: stringV("c") })).to.equal(3); + expect(resolveExpression("number", expr, { FOO: stringV("d") })).to.equal(4); + }); + + it("it provides resolved parameters from a nested branch", () => { + const expr = + '{{ params.FOO == "x" ? params.IF_A : params.FOO == "y" ? params.IF_B : params.IF_C }}'; + const params = { + FOO: stringV("y"), + IF_A: numberV(1), + IF_B: numberV(2), + IF_C: numberV(3), + }; + expect(resolveExpression("number", expr, params)).to.equal(2); + }); + + it("it resolves list branches in a nested ternary", () => { + const expr = '{{ params.FOO == "x" ? ["a"] : params.FOO == "y" ? [params.BAR] : [] }}'; + expect( + resolveExpression("string[]", expr, { FOO: stringV("y"), BAR: stringV("b") }), + ).to.deep.equal(["b"]); + expect( + resolveExpression("string[]", expr, { FOO: stringV("z"), BAR: stringV("b") }), + ).to.deep.equal([]); + }); + + it("it resolves a list branch holding a value that contains a double quote", () => { + expect( + resolveExpression("string[]", '{{ params.FOO == "x" ? [params.Q] : [] }}', { + FOO: stringV("x"), + Q: stringV('a"b'), + }), + ).to.deep.equal(['a"b']); + expect( + resolveExpression("string[]", "{{ params.FLAG ? [params.Q] : [] }}", { + FLAG: boolV(true), + Q: stringV('a"b'), + }), + ).to.deep.equal(['a"b']); + // The quote and the delimiter are in different values here, so the split + // has to land between the branches and not inside the list. + expect( + resolveExpression("string[]", '{{ params.FOO == "a"b" ? [params.Q] : [] }}', { + FOO: stringV('a"b'), + Q: stringV("x : y"), + }), + ).to.deep.equal(["x : y"]); + }); + + it("it resolves values that contain a double quote", () => { + expect( + resolveExpression("string", '{{ params.MSG == "a"b" ? "sa-prod" : "sa-dev" }}', { + MSG: stringV('a"b'), + }), + ).to.equal("sa-prod"); + expect( + resolveExpression("number", '{{ params.MSG == "a"b" ? 1 : 2 }}', { + MSG: stringV('a"b'), + }), + ).to.equal(1); + expect( + resolveExpression("string", '{{ params.FLAG ? "a"b" : "c" }}', { + FLAG: boolV(false), + }), + ).to.equal("c"); + expect( + resolveExpression("string", '{{ params.FLAG ? "x" : "a"b" }}', { + FLAG: boolV(true), + }), + ).to.equal("x"); + expect( + resolveExpression("string", '{{ params.FLAG ? "a\\"b" : "c" }}', { + FLAG: boolV(false), + }), + ).to.equal("c"); + }); + + it("it resolves an expression holding more than one unescaped double quote", () => { + const expr = '{{ params.FLAG ? "6" pipe" : "8" pipe" }}'; + expect(resolveExpression("string", expr, { FLAG: boolV(true) })).to.equal('6" pipe'); + expect(resolveExpression("string", expr, { FLAG: boolV(false) })).to.equal('8" pipe'); + + const cmp = '{{ params.M == "a"b" ? "c" : "e"f" }}'; + expect(resolveExpression("string", cmp, { M: stringV('a"b') })).to.equal("c"); + expect(resolveExpression("string", cmp, { M: stringV("zz") })).to.equal('e"f'); + + expect( + resolveExpression("string", '{{ params.A == params.B ? "p"q" : "r"s" }}', { + A: stringV("1"), + B: stringV("2"), + }), + ).to.equal('r"s'); + }); + + it("it doesn't split on a ? or a : inside a string literal", () => { + expect( + resolveExpression("string", '{{ params.FOO == "q" ? "x : y" : "z" }}', { + FOO: stringV("a"), + }), + ).to.equal("z"); + expect( + resolveExpression("string", '{{ params.FOO == "a" ? "x ? y" : "z" }}', { + FOO: stringV("a"), + }), + ).to.equal("x ? y"); + expect( + resolveExpression("string", '{{ params.FOO == "a : b" ? "z" : "w" }}', { + FOO: stringV("a : b"), + }), + ).to.equal("z"); + expect( + resolveExpression("string", '{{ params.FOO == "a" ? "x ? y : z" : "w" }}', { + FOO: stringV("a"), + }), + ).to.equal("x ? y : z"); + expect( + resolveExpression("string", '{{ params.FOO == "q" ? "z" : "x : y" }}', { + FOO: stringV("a"), + }), + ).to.equal("x : y"); + }); + + it("raises when a nested branch references a missing param", () => { + expect(() => { + resolveExpression( + "string", + '{{ params.FOO == "x" ? "a" : params.FOO == "y" ? params.MISSING : "c" }}', + { FOO: stringV("y") }, + ); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression( + "string", + '{{ params.FOO == "x" ? "a" : params.MISSING == "y" ? "b" : "c" }}', + { FOO: stringV("z") }, + ); + }).to.throw(ExprParseError); + }); + + it("raises when a nested branch resolves to a param of the wrong type", () => { + expect(() => { + resolveExpression( + "string", + '{{ params.FOO == "x" ? "a" : params.FOO == "y" ? params.NUM : "c" }}', + { FOO: stringV("y"), NUM: numberV(2) }, + ); + }).to.throw(ExprParseError); + }); + + it("raises when a nested branch isn't a legal literal", () => { + expect(() => { + resolveExpression("number", '{{ params.FOO == "x" ? 1 : params.FOO == "y" ? abc : 3 }}', { + FOO: stringV("y"), + }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression( + "string", + '{{ params.FOO == "x" ? "a" : params.FOO == "y" ? bare : "c" }}', + { FOO: stringV("y") }, + ); + }).to.throw(ExprParseError); + }); + + it("raises on ternaries with a missing or a stray delimiter", () => { + expect(() => { + resolveExpression("number", "{{ }}", {}); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("number", "{{ params.FOO ?? 10 : 0 }}", { FOO: numberV(22) }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("number", "{{ params.FOO == 22 : 0 }}", { FOO: numberV(22) }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("number", "{{ params.FOO == 22 ? 10 }}", { FOO: numberV(22) }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("number", "{{ params.FOO == 22 ? 10 : }}", { FOO: numberV(22) }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("string", '{{ params.FOO ? "a" : "b" : "c" }}', { FOO: boolV(false) }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("string", '{{ params.FOO == "a" ? "x" ? "y" : "z" }}', { + FOO: stringV("a"), + }); + }).to.throw(ExprParseError); + }); + + it("raises when a value holds both a double quote and a delimiter", () => { + expect(() => { + resolveExpression("string", '{{ params.MSG == "a"b : c" ? "x" : "y" }}', { + MSG: stringV('a"b : c'), + }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("string", '{{ params.FLAG ? "a"b ? c" : "z" }}', { FLAG: boolV(false) }); + }).to.throw(ExprParseError); + }); + + it("raises on a branch whose value pairs a double quote with a delimiter", () => { + // The true branch here is the single value a" : "b, which the SDK writes + // out unescaped. Whichever way the delimiters are paired up, some branch + // is left holding one that belongs to nothing, so this raises either way + // rather than resolving to a truncated value on one of them. + const flag = '{{ params.FLAG ? "a" : "b" : "" }}'; + expect(() => { + resolveExpression("string", flag, { FLAG: boolV(true) }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("string", flag, { FLAG: boolV(false) }); + }).to.throw(ExprParseError); + + const cmp = '{{ params.ENV == "prod" ? "a" : "b" : "fallback" }}'; + expect(() => { + resolveExpression("string", cmp, { ENV: stringV("prod") }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("string", cmp, { ENV: stringV("dev") }); + }).to.throw(ExprParseError); + }); + + it("it leaves a comparison holding a stray delimiter to the comparison evaluators", () => { + // No " ? " anywhere, so this was never a ternary and the delimiter is + // just part of the value being compared against. + expect( + resolveExpression("boolean", '{{ params.MSG == "a"b : c" }}', { + MSG: stringV('a"b : c'), + }), + ).to.equal(true); + // Quotes that do line up, on the other hand, make this a ternary with a + // branch delimiter and no condition delimiter, which stays rejected. + expect(() => { + resolveExpression("string", '{{ params.S == "a" : "b" }}', { S: stringV("a") }); + }).to.throw(ExprParseError); + }); + + it("raises when the expression isn't delimited by single spaces", () => { + expect(() => { + resolveExpression("number", "{{ params.FOO == 22 ? 10 : 0 }}", { + FOO: numberV(22), + }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("number", "{{ params.FOO == 22? 10 : 0 }}", { FOO: numberV(22) }); + }).to.throw(ExprParseError); + }); + + it("raises when a ternary isn't the whole expression", () => { + expect(() => { + resolveExpression("string", '{{ params.FLAG ? "a" : "b" }}extra', { FLAG: boolV(true) }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("string", 'junk{{ params.FLAG ? "a" : "b" }}', { FLAG: boolV(true) }); + }).to.throw(ExprParseError); + }); }); }); diff --git a/src/deploy/functions/cel.ts b/src/deploy/functions/cel.ts index 823fa1c59e7..59d70078361 100644 --- a/src/deploy/functions/cel.ts +++ b/src/deploy/functions/cel.ts @@ -7,8 +7,6 @@ type IdentityExpression = CelExpression; type ComparisonExpression = CelExpression; type DualComparisonExpression = CelExpression; type TernaryExpression = CelExpression; -type LiteralTernaryExpression = CelExpression; -type DualTernaryExpression = CelExpression; type Literal = string | number | boolean | string[]; type L = "string" | "number" | "boolean" | "string[]"; @@ -20,13 +18,11 @@ const dualComparisonRegexp = new RegExp( /{{ params\.(\S+) CMP params\.(\S+) }}/.source.replace("CMP", CMP), ); const comparisonRegexp = new RegExp(/{{ params\.(\S+) CMP (.+) }}/.source.replace("CMP", CMP)); -const dualTernaryRegexp = new RegExp( - /{{ params\.(\S+) CMP params\.(\S+) \? (.+) : (.+) }/.source.replace("CMP", CMP), -); -const ternaryRegexp = new RegExp( - /{{ params\.(\S+) CMP (.+) \? (.+) : (.+) }/.source.replace("CMP", CMP), -); -const literalTernaryRegexp = /{{ params\.(\S+) \? (.+) : (.+) }/; + +const exprPrefix = "{{ "; +const exprSuffix = " }}"; +const questionToken = " ? "; +const colonToken = " : "; /** * An array equality test for use on resolved list literal ParamValues only; @@ -53,17 +49,201 @@ function isComparisonExpression(value: CelExpression): value is ComparisonExpres function isDualComparisonExpression(value: CelExpression): value is DualComparisonExpression { return dualComparisonRegexp.test(value); } -function isTernaryExpression(value: CelExpression): value is TernaryExpression { - return ternaryRegexp.test(value); + +export class ExprParseError extends FirebaseError {} + +interface TernaryParts { + condition: CelExpression; + ifTrue: CelExpression; + ifFalse: CelExpression; +} + +type TernarySplit = + // The body is a ternary and here are its parts. + | ({ kind: "ternary" } & TernaryParts) + // The body holds no delimiter, so it's one of the other forms of expression + // or a terminal branch value. + | { kind: "none" } + // The body holds delimiters that don't pair up, so it's a broken ternary and + // nothing else. + | { kind: "malformed" }; + +type TernaryScan = + | TernarySplit + // The scan gave up because the body's quote characters don't line up with its + // tokens, so nothing it worked out about literals can be trusted. + | { kind: "misquoted" } + // The body holds a " : " with no " ? " in front of it anywhere, so it isn't a + // ternary at all rather than being a broken one. + | { kind: "strayColon" }; + +/** + * Tests whether a quote character sits where a string literal can start or end. + * + * The SDK writes every string operand as `"${value}"` without escaping the + * value, so a value holding a double quote leaves quotes in the body that open + * and close nothing. A quote that really does delimit a literal is at the edge + * of a token, which means it's next to a space, a list bracket, a comma, or the + * end of the body. Anything else is a value's own quote, and it means the scan + * has lost track of where the literals are. + */ +function isLiteralEdge(body: string, i: number, closing: boolean): boolean { + const neighbor = closing ? body[i + 1] : body[i - 1]; + return neighbor === undefined || [" ", ",", "[", "]"].includes(neighbor); +} + +/** + * Scans the body of an expression, meaning everything between the "{{ " and + * the " }}", for the delimiters of a ternary. + * + * This can't be done with a regexp. Ternaries nest, the SDK emits them without + * parentheses, and they associate to the right, so pairing a " ? " with the + * " : " that belongs to it means counting delimiters. A quoted string literal + * is also allowed to contain either token. Both of those need a left to right + * scan. + * + * Quote tracking is skipped entirely when ignoreQuotes is set, which is how + * splitTernary() copes with a body whose quotes don't line up. + */ +function scanTernary(body: string, ignoreQuotes: boolean): TernaryScan { + let inLiteral = false; + let depth = 0; + let question = -1; + let strayColon = false; + const candidates: number[] = []; + + for (let i = 0; i < body.length; i++) { + if (!ignoreQuotes) { + if (inLiteral && body[i] === "\\") { + i++; // whatever follows is escaped, so it's content and not a delimiter + continue; + } + if (body[i] === '"') { + if (!isLiteralEdge(body, i, inLiteral)) { + return { kind: "misquoted" }; + } + inLiteral = !inLiteral; + continue; + } + if (inLiteral) { + continue; + } + } + if (body.startsWith(questionToken, i)) { + if (question === -1) { + question = i; + } else { + depth++; + } + i += questionToken.length - 1; // skip past the token we just consumed + continue; + } + if (!body.startsWith(colonToken, i)) { + continue; + } + if (question === -1) { + // A branch delimiter with no condition delimiter in front of it. The scan + // carries on, because whether a " ? " turns up later is what separates a + // broken ternary from an expression that was never a ternary. + strayColon = true; + } else if (depth > 0) { + depth--; + } else { + candidates.push(i); + } + i += colonToken.length - 1; // skip past the token we just consumed + } + + if (inLiteral) { + return { kind: "misquoted" }; + } + if (question === -1) { + return strayColon ? { kind: "strayColon" } : { kind: "none" }; + } + if (strayColon) { + // A " : " ahead of the " ? " belongs to neither this ternary nor a nested one. + return { kind: "malformed" }; + } + // Delimiters that pair up cleanly leave one candidate, because everything + // after it is part of the false branch. More than one means a value's own + // double quote hid the real delimiter from the scan and offered one of its + // own, so a candidate that cuts a branch in half gives way to the next. + for (const colon of candidates) { + const ifTrue = body.slice(question + questionToken.length, colon); + const ifFalse = body.slice(colon + colonToken.length); + if (isSoundBranch(ifTrue) && isSoundBranch(ifFalse)) { + return { kind: "ternary", condition: body.slice(0, question), ifTrue, ifFalse }; + } + } + // A condition delimiter that never found its branch delimiter is a broken + // ternary. Calling it a non ternary would let the comparison evaluators + // reinterpret it, and they'd return a boolean for whatever type was asked for. + return { kind: "malformed" }; } -function isLiteralTernaryExpression(value: CelExpression): value is LiteralTernaryExpression { - return literalTernaryRegexp.test(value); + +/** + * Tests that a branch is something a ternary can really have as a branch: a + * ternary itself, or a value with no delimiter left loose in it. A branch that + * fails this was cut out of the middle of a value, which is what a value's own + * double quote does to the scan. + * + * A loose " : " is read strictly here, unlike in splitTernary(), where it means + * the body was never a ternary and belongs to another form of expression. A + * branch has nowhere else to go, so the only thing a loose delimiter can tell + * us is that the split which produced it was the wrong one. + */ +function isSoundBranch(branch: CelExpression): boolean { + const scanned = scanTernary(branch, false); + const kind = scanned.kind === "misquoted" ? scanTernary(branch, true).kind : scanned.kind; + return kind === "ternary" || kind === "none"; } -function isDualTernaryExpression(value: CelExpression): value is DualTernaryExpression { - return dualTernaryRegexp.test(value); + +/** + * Splits the body of an expression as a ternary. + * + * A body holding a value with a double quote in it has quotes that delimit no + * literal, and the scan says so rather than guessing which ones are real. The + * regexps this replaced had no notion of quoting at all, so reading such a body + * again with quote tracking off keeps those expressions resolving to what + * they've always resolved to. + */ +function splitTernary(body: string): TernarySplit { + const scanned = scanTernary(body, false); + if (scanned.kind === "strayColon") { + // The quotes line up, so this really is a branch delimiter with no + // condition delimiter in front of it, which is a broken ternary. + return { kind: "malformed" }; + } + if (scanned.kind !== "misquoted") { + return scanned; + } + const rescanned = scanTernary(body, true); + if (rescanned.kind === "strayColon") { + // Reading the quotes some other way is the only thing that could pair this + // " : " up, and the scan has just given up on them, so the body goes to the + // forms of expression that never had a notion of quoting. A comparison + // against a value holding a " : " lands here and resolves as it always has. + return { kind: "none" }; + } + // The second scan doesn't track quoting, so it can't come back misquoted. + return rescanned.kind === "misquoted" ? { kind: "none" } : rescanned; } -export class ExprParseError extends FirebaseError {} +/** + * Pulls the body out of a whole CEL expression and splits it as a ternary, + * returning null if the expression isn't a ternary. A ternary whose delimiters + * don't pair up raises instead, so that it can't fall through to another form. + */ +function parseTernary(expr: CelExpression): TernaryParts | null { + if (!expr.startsWith(exprPrefix) || !expr.endsWith(exprSuffix)) { + return null; + } + const split = splitTernary(expr.slice(exprPrefix.length, -exprSuffix.length)); + if (split.kind === "malformed") { + throw new ExprParseError("Malformed CEL ternary expression '" + expr + "'"); + } + return split.kind === "ternary" ? split : null; +} /** * Resolves a CEL expression of a supported form, guaranteeing the provided primitive type: @@ -73,6 +253,7 @@ export class ExprParseError extends FirebaseError {} * - {{ params.foo == 24 ? "asdf" : params.jkl }} * - {{ params.foo > params.bar ? "asdf" : params.jkl }} * - {{ params.foo ? "asdf" : params.jkl }}, when foo is of boolean type + * Either branch of a ternary can be a ternary itself, to any depth. * Values interpolated from params retain their type defined in the param; * it is an error to provide a CEL expression that coerces param types * (i.e testing equality between a IntParam and a BooleanParam). It is also @@ -88,16 +269,16 @@ export function resolveExpression( // first and resolve them. This isn't (and can't be) recursive, but the fact that // we only support string[] types mostly saves us here. expr = preprocessLists(wantType, expr, params); - // N.B: Since some of these regexps are supersets of others--anything that is - // params\.(\S+) is also (.+)--the order in which they are tested matters + // N.B: Some of these regexps are supersets of others (anything that is + // params\.(\S+) is also (.+)), so the order in which they are tested matters. + // The ternary is parsed ahead of the chain below, rather than tested inside + // it, because the split it produces is reused to evaluate the expression. if (isIdentityExpression(expr)) { return resolveIdentity(wantType, expr, params); - } else if (isDualTernaryExpression(expr)) { - return resolveDualTernary(wantType, expr, params); - } else if (isLiteralTernaryExpression(expr)) { - return resolveLiteralTernary(wantType, expr, params); - } else if (isTernaryExpression(expr)) { - return resolveTernary(wantType, expr, params); + } + const ternary = parseTernary(expr); + if (ternary) { + return resolveTernary(wantType, expr, ternary, params); } else if (isDualComparisonExpression(expr)) { return resolveDualComparison(expr, params); } else if (isComparisonExpression(expr)) { @@ -368,63 +549,44 @@ function resolveDualComparison( /** * {{ params.foo == 24 ? "asdf" : params.jkl }} + * {{ params.foo > params.bar ? "asdf" : params.jkl }} + * {{ params.foo ? "asdf" : params.jkl }}, when foo is of boolean type + * Either branch can be a ternary itself, to any depth. */ function resolveTernary( wantType: L, expr: TernaryExpression, + parts: TernaryParts, params: Record, ): Literal { - const match = ternaryRegexp.exec(expr); - if (!match) { - throw new ExprParseError("malformed CEL ternary expression '" + expr + "'"); - } - - const comparisonExpr = `{{ params.${match[1]} ${match[2]} ${match[3]} }}`; - const isTrue = resolveComparison(comparisonExpr, params); - if (isTrue) { - return resolveParamListOrLiteral(wantType, match[4], params); - } else { - return resolveParamListOrLiteral(wantType, match[5], params); - } + const isTrue = resolveTernaryCondition(expr, parts.condition, params); + return resolveTernaryBranch(wantType, expr, isTrue ? parts.ifTrue : parts.ifFalse, params); } /** - * {{ params.foo > params.bar ? "asdf" : params.jkl }} + * The condition of a ternary is one of the comparison forms, or a bare + * reference to a param of boolean type. */ -function resolveDualTernary( - wantType: L, - expr: DualTernaryExpression, +function resolveTernaryCondition( + expr: TernaryExpression, + condition: CelExpression, params: Record, -): Literal { - const match = dualTernaryRegexp.exec(expr); - if (!match) { - throw new ExprParseError("Malformed CEL ternary expression '" + expr + "'"); - } - const comparisonExpr = `{{ params.${match[1]} ${match[2]} params.${match[3]} }}`; - const isTrue = resolveDualComparison(comparisonExpr, params); - if (isTrue) { - return resolveParamListOrLiteral(wantType, match[4], params); - } else { - return resolveParamListOrLiteral(wantType, match[5], params); +): boolean { + const conditionExpr = `${exprPrefix}${condition}${exprSuffix}`; + if (isDualComparisonExpression(conditionExpr)) { + return resolveDualComparison(conditionExpr, params); + } else if (isComparisonExpression(conditionExpr)) { + return resolveComparison(conditionExpr, params); } -} -/** - * {{ params.foo ? "asdf" : params.jkl }} - * only when the paramValue associated with params.foo is validBoolean - */ -function resolveLiteralTernary( - wantType: L, - expr: TernaryExpression, - params: Record, -): Literal { - const match = literalTernaryRegexp.exec(expr); + const match = identityRegexp.exec(conditionExpr); if (!match) { - throw new ExprParseError("Malformed CEL ternary expression '" + expr + "'"); + throw new ExprParseError( + "CEL ternary expression '" + expr + "' is conditioned on an unsupported form", + ); } - const paramName = match[1]; - const paramValue = params[match[1]]; + const paramValue = params[paramName]; if (!paramValue) { throw new ExprParseError( "CEL ternary expression '" + expr + "' references missing param " + paramName, @@ -435,12 +597,31 @@ function resolveLiteralTernary( "CEL ternary expression '" + expr + "' is conditional on non-boolean param " + paramName, ); } + return paramValue.asBoolean(); +} - if (paramValue.asBoolean()) { - return resolveParamListOrLiteral(wantType, match[2], params); - } else { - return resolveParamListOrLiteral(wantType, match[3], params); +/** + * A branch of a ternary is either another ternary or a terminal value: a + * reference to a param, a list, or a literal. A branch left holding a delimiter + * that pairs with nothing is neither, so it raises rather than being read as a + * value with punctuation in it. + */ +function resolveTernaryBranch( + wantType: L, + expr: TernaryExpression, + branch: CelExpression, + params: Record, +): Literal { + const nested = splitTernary(branch); + if (nested.kind === "malformed") { + throw new ExprParseError("Malformed CEL ternary expression '" + expr + "'"); } + if (nested.kind === "ternary") { + return resolveTernary(wantType, expr, nested, params); + } + // N.B: lists were already expanded by the preprocessLists() call that started + // this resolution, so a branch must not be run through it a second time. + return resolveParamListOrLiteral(wantType, branch, params); } function resolveParamListOrLiteral( @@ -468,8 +649,14 @@ function resolveLiteral(wantType: L, value: string): Literal { if (wantType === "string[]") { // N.B: value being a literal list that can just be JSON.parsed should be guaranteed - // by the preprocessLists() invocation at the beginning of CEL resolution - const parsed = JSON.parse(value); + // by the preprocessLists() invocation at the beginning of CEL resolution, so + // reaching the catch means something upstream handed this a fragment of one. + let parsed: unknown; + try { + parsed = JSON.parse(value); + } catch { + throw new ExprParseError("CEL literal " + value + " does not seem to be a list"); + } if (!Array.isArray(parsed)) { throw new ExprParseError(`CEL tried to read non-list ${JSON.stringify(parsed)} as a list`); } diff --git a/src/deploy/functions/params.spec.ts b/src/deploy/functions/params.spec.ts index ef7dfbec5ea..7ad9a5a34c9 100644 --- a/src/deploy/functions/params.spec.ts +++ b/src/deploy/functions/params.spec.ts @@ -56,6 +56,21 @@ describe("CEL resolution", () => { ).to.equal("asdf jkl;"); }); + it("can interpolate a nested ternary into a CEL expression", () => { + const ternary = + '{{ params.PROJECT_ID == "xxx" ? "aaa" : params.PROJECT_ID == "yyy" ? "bbb" : "ccc" }}'; + const projectId = { + PROJECT_ID: new params.ParamValue("yyy", false, { string: true }), + }; + expect(params.resolveString(`sa-${ternary}@proj.iam`, projectId)).to.equal("sa-bbb@proj.iam"); + expect( + params.resolveString(`${ternary}/{{ params.REGION }}`, { + ...projectId, + REGION: new params.ParamValue("west1", false, { string: true }), + }), + ).to.equal("bbb/west1"); + }); + it("throws instead of coercing a param value with the wrong type", () => { expect(() => params.resolveString("{{ params.foo }}", {