diff --git a/CHANGELOG.md b/CHANGELOG.md index 13a49884dc3..bda7d8464d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,3 +6,4 @@ - Batched function deletions across instances when uninstalling a Function Kit (#11189). - Fixed an issue where 2nd-gen functions with parameterized trigger event filters failed default region resolution (#11020). - Fixed `functions:lifecycle:list` and `functions:lifecycle:run` failing to detect Function Kit instances (#11240). +- 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..e3c2a9bc178 100644 --- a/src/deploy/functions/cel.spec.ts +++ b/src/deploy/functions/cel.spec.ts @@ -430,6 +430,24 @@ describe("CEL evaluation", () => { }), ).to.be.true; }); + + it("raises when a comparison is resolved as a type other than boolean", () => { + expect(() => { + resolveExpression("string", '{{ params.FOO == "bar" }}', { FOO: stringV("bar") }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("number", "{{ params.FOO == params.BAR }}", { + FOO: numberV(22), + BAR: numberV(22), + }); + }).to.throw(ExprParseError); + // An unescaped quote hides the " ? ", so this reads as a comparison. + expect(() => { + resolveExpression("string", '{{ params.FOO == "a"b" ? "x" : "y" }}', { + FOO: stringV('a"b'), + }); + }).to.throw(ExprParseError); + }); }); describe("Dual comparison expressions", () => { @@ -1018,6 +1036,24 @@ describe("CEL evaluation", () => { ).to.equal("baz"); }); + 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 +1206,223 @@ describe("CEL evaluation", () => { }), ).to.equal("baz"); }); + + 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("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("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("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("resolves an expression with ternaries nested in both branches", () => { + const expr = '{{ params.A ? params.B ? "1" : "2" : params.C ? "3" : "4" }}'; + expect( + resolveExpression("string", expr, { A: boolV(true), B: boolV(true), C: boolV(false) }), + ).to.equal("1"); + expect( + resolveExpression("string", expr, { A: boolV(true), B: boolV(false), C: boolV(false) }), + ).to.equal("2"); + expect( + resolveExpression("string", expr, { A: boolV(false), B: boolV(false), C: boolV(true) }), + ).to.equal("3"); + expect( + resolveExpression("string", expr, { A: boolV(false), B: boolV(false), C: boolV(false) }), + ).to.equal("4"); + }); + + 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("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("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("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']); + }); + + it("doesn't end a literal at an escaped double quote", () => { + expect( + resolveExpression("string", '{{ params.FLAG ? "a\\"b" : "c" }}', { + FLAG: boolV(false), + }), + ).to.equal("c"); + }); + + 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"); + expect( + resolveExpression("boolean", '{{ params.FOO == "a ? b" }}', { FOO: stringV("a ? b") }), + ).to.equal(true); + }); + + 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" ? "x" }}', { FOO: stringV("a") }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("string", '{{ params.FOO == "a" ? "x" ? "y" : "z" }}', { + FOO: stringV("a"), + }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("string", '{{ params.FOO ? "x" : "y" ? "z" }}', { FOO: boolV(false) }); + }).to.throw(ExprParseError); + expect(() => { + resolveExpression("string", '{{ params.FOO == "a" : "b" }}', { FOO: stringV("a") }); + }).to.throw(ExprParseError); + }); }); }); diff --git a/src/deploy/functions/cel.ts b/src/deploy/functions/cel.ts index 823fa1c59e7..b3903a4e76a 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 EXPR_PREFIX = "{{ "; +const EXPR_SUFFIX = " }}"; +const QUESTION_TOKEN = " ? "; +const COLON_TOKEN = " : "; /** * An array equality test for use on resolved list literal ParamValues only; @@ -53,18 +49,81 @@ 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); -} -function isLiteralTernaryExpression(value: CelExpression): value is LiteralTernaryExpression { - return literalTernaryRegexp.test(value); -} -function isDualTernaryExpression(value: CelExpression): value is DualTernaryExpression { - return dualTernaryRegexp.test(value); -} export class ExprParseError extends FirebaseError {} +interface TernaryParts { + condition: string; + ifTrue: string; + ifFalse: string; +} + +/** + * Splits a ternary into its condition and branches, or returns undefined if the + * expression isn't a ternary. Ternaries are right associative and the SDK emits + * them without parentheses, so the " : " that pairs with the first " ? " is + * found by counting delimiters outside of string literals. + */ +function parseTernary(expr: CelExpression): TernaryParts | undefined { + if (!expr.startsWith(EXPR_PREFIX) || !expr.endsWith(EXPR_SUFFIX)) { + return undefined; + } + const body = expr.slice(EXPR_PREFIX.length, -EXPR_SUFFIX.length); + + let inQuotes = false; + let depth = 0; + let question = -1; + let colon = -1; + + for (let i = 0; i < body.length; i++) { + if (inQuotes && body[i] === "\\") { + i++; // skip escaped character + continue; + } + if (body[i] === '"') { + inQuotes = !inQuotes; + continue; + } + if (inQuotes) continue; + + if (body.startsWith(QUESTION_TOKEN, i)) { + if (depth === 0) { + question = i; + } + depth++; + i += QUESTION_TOKEN.length - 1; + } else if (body.startsWith(COLON_TOKEN, i)) { + if (depth > 0) { + depth--; + if (depth === 0) { + colon = i; + break; // found matching colon for the condition's question mark + } + } + i += COLON_TOKEN.length - 1; + } + } + + if (question === -1) { + return undefined; + } + // A " ? " without its " : " raises here. Returning undefined would let the + // comparison forms, or a nested branch, read the broken ternary as a value. + if (colon === -1) { + throw new ExprParseError(`Malformed CEL ternary expression '${expr}'`); + } + + const condition = body.slice(0, question).trim(); + const ifTrue = body.slice(question + QUESTION_TOKEN.length, colon).trim(); + const ifFalse = body.slice(colon + COLON_TOKEN.length).trim(); + + if (!condition || !ifTrue || !ifFalse) { + throw new ExprParseError(`Malformed CEL ternary expression '${expr}'`); + } + + return { condition, ifTrue, ifFalse }; +} + /** * Resolves a CEL expression of a supported form, guaranteeing the provided primitive type: * - {{ params.foo }} @@ -73,6 +132,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 @@ -92,15 +152,13 @@ export function resolveExpression( // params\.(\S+) is also (.+)--the order in which they are tested matters 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); - } else if (isDualComparisonExpression(expr)) { + } + const ternary = parseTernary(expr); + if (ternary) { + return resolveTernary({ wantType, expr, parts: ternary, params }); + } else if (wantType === "boolean" && isDualComparisonExpression(expr)) { return resolveDualComparison(expr, params); - } else if (isComparisonExpression(expr)) { + } else if (wantType === "boolean" && isComparisonExpression(expr)) { return resolveComparison(expr, params); } else { throw new ExprParseError("CEL expression '" + expr + "' is of an unsupported form"); @@ -366,81 +424,87 @@ function resolveDualComparison( } } -/** - * {{ params.foo == 24 ? "asdf" : params.jkl }} - */ -function resolveTernary( - wantType: L, - expr: TernaryExpression, - 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); - } +interface ResolveTernaryOptions { + wantType: L; + expr: TernaryExpression; + parts: TernaryParts; + params: Record; } /** + * {{ 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 resolveDualTernary( - wantType: L, - expr: DualTernaryExpression, - 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); - } +function resolveTernary(options: ResolveTernaryOptions): Literal { + const { wantType, expr, parts, params } = options; + const isTrue = resolveTernaryCondition(expr, parts.condition, params); + return resolveTernaryBranch({ + wantType, + expr, + branch: isTrue ? parts.ifTrue : parts.ifFalse, + params, + }); } /** - * {{ params.foo ? "asdf" : params.jkl }} - * only when the paramValue associated with params.foo is validBoolean + * The condition of a ternary is one of the comparison forms, or a bare + * reference to a param of boolean type. */ -function resolveLiteralTernary( - wantType: L, +function resolveTernaryCondition( expr: TernaryExpression, + condition: CelExpression, params: Record, -): Literal { - const match = literalTernaryRegexp.exec(expr); - if (!match) { - throw new ExprParseError("Malformed CEL ternary expression '" + expr + "'"); +): boolean { + const conditionExpr = `${EXPR_PREFIX}${condition}${EXPR_SUFFIX}`; + if (isDualComparisonExpression(conditionExpr)) { + return resolveDualComparison(conditionExpr, params); + } else if (isComparisonExpression(conditionExpr)) { + return resolveComparison(conditionExpr, params); } + const match = identityRegexp.exec(conditionExpr); + if (!match) { + 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, + `CEL ternary expression '${expr}' references missing param ${paramName}`, ); } if (!paramValue.legalBoolean) { throw new ExprParseError( - "CEL ternary expression '" + expr + "' is conditional on non-boolean param " + paramName, + `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); +interface ResolveTernaryBranchOptions { + wantType: L; + expr: TernaryExpression; + branch: CelExpression; + params: Record; +} + +/** + * A branch of a ternary is either another ternary or a reference to a param, + * a list, or a literal. + */ +function resolveTernaryBranch(options: ResolveTernaryBranchOptions): Literal { + const { wantType, expr, branch, params } = options; + const nested = parseTernary(`${EXPR_PREFIX}${branch}${EXPR_SUFFIX}`); + if (nested) { + return resolveTernary({ wantType, expr, parts: 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( 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 }}", {