diff --git a/CHANGELOG.md b/CHANGELOG.md index e69de29bb2d..05d2c6cd40a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -0,0 +1 @@ +- Fixed 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 66ea0b74e18..0e00a757f7c 100644 --- a/src/deploy/functions/params.spec.ts +++ b/src/deploy/functions/params.spec.ts @@ -57,6 +57,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 }}", {