clarkmcc / cel-rust

Common Expression Language interpreter written in Rust
https://crates.io/crates/cel-interpreter
MIT License
377 stars 21 forks source link

Wrong evaluation result #19

Closed inikolaev closed 1 year ago

inikolaev commented 1 year ago

I've been playing around with this CEL implementation and I noticed one odd thing with the following expressions:

b && (c == "string")

b && c == "string"

c == "string" && b

Given this context

{"b": True, "c": "string"}

they should all evaluate to true, but this is not what's happening:

True <= b && (c == "string")
False <= b && c == "string"
True <= c == "string" && b

Here's a simple reproducer:

use cel_interpreter::{Context,Program, Value};

fn main() {
    let expressions = [
        "b && (c == \"string\")",
        "b && c == \"string\"",
        "c == \"string\" && b",
    ];

    for expression in expressions {
        let program = Program::compile(expression).unwrap();
        let mut context = Context::default();
        context.add_variable("b", Value::Bool(true));
        context.add_variable("c", Value::String(String::from("string").into()));

        let result = program.execute(&context);

        println!("{:?} <= {}", result, expression)
    }
}

It produces the following output:

Ok(Bool(true)) <= b && (c == "string")
Ok(Bool(false)) <= b && c == "string"
Ok(Bool(true)) <= c == "string" && b

It seems like in the case of b && c == "string" the interpreter effectively evaluates this expression

(b && c) == "string"

I'm also using a Python version of CEL interpreter and it evaluates it properly:

import celpy

expressions = [
    'b && (c == "string")',
    'b && c == "string"',
    'c == "string" && b',
]

for expression in expressions:
    env = celpy.Environment()
    ast = env.compile(expression)
    prgm = env.program(ast)

    activation = celpy.json_to_cel({"a": 1, "b": True, "c": "string"})
    result = prgm.evaluate(activation)
    print(f"{result} <= {expression}")

Produces

True <= b && (c == "string")
True <= b && c == "string"
True <= c == "string" && b
clarkmcc commented 1 year ago

Thank you for uncovering this issue! It looks like operator precedence is incorrect, where precedence should be given to == when it is instead given to the &&. I'll need to review our parser and see what I can find. As a side note, I checked the spec and Go implementation and they behave in the same way as your Python implementation confirming the bug in this Rust implementation.

Edit: Okay, I think I've got a fix that resolves the issue and passes all the existing tests. I'll implement additional tests and final cleanup tomorrow.