From 07c4642c630bcd59cecf3f8b82a267ec4ece08d2 Mon Sep 17 00:00:00 2001
From: Geoffroy Couprie <contact@geoffroycouprie.com>
Date: Sat, 13 Jan 2024 15:06:52 +0100
Subject: [PATCH 1/2] use exceptions in expression evaluation

---
 .../biscuit/datalog/MatchedVariables.java     |  13 +--
 .../org/biscuitsec/biscuit/datalog/Rule.java  |  31 +++---
 .../org/biscuitsec/biscuit/datalog/World.java |   2 +-
 .../datalog/expressions/Expression.java       |  11 +-
 .../biscuit/datalog/expressions/Op.java       | 102 +++++++-----------
 .../org/biscuitsec/biscuit/error/Error.java   |  16 ++-
 .../biscuit/builder/parser/ParserTest.java    |  10 +-
 .../biscuit/datalog/ExpressionTest.java       |  17 +--
 8 files changed, 95 insertions(+), 107 deletions(-)

diff --git a/src/main/java/org/biscuitsec/biscuit/datalog/MatchedVariables.java b/src/main/java/org/biscuitsec/biscuit/datalog/MatchedVariables.java
index a5e9d408..7b444145 100644
--- a/src/main/java/org/biscuitsec/biscuit/datalog/MatchedVariables.java
+++ b/src/main/java/org/biscuitsec/biscuit/datalog/MatchedVariables.java
@@ -2,6 +2,7 @@
 
 import org.biscuitsec.biscuit.datalog.expressions.Expression;
 import io.vavr.control.Option;
+import org.biscuitsec.biscuit.error.Error;
 
 import java.io.Serializable;
 import java.util.*;
@@ -60,21 +61,17 @@ public MatchedVariables(final Set<Long> ids) {
       }
    }
 
-   public Option<Map<Long, Term>> check_expressions(List<Expression> expressions, SymbolTable symbols) {
+   public Option<Map<Long, Term>> check_expressions(List<Expression> expressions, SymbolTable symbols) throws Error {
       final Option<Map<Long, Term>> vars = this.complete();
       if (vars.isDefined()) {
          Map<Long, Term> variables = vars.get();
 
 
          for(Expression e: expressions) {
-            Option<Term> res = e.evaluate(variables, new TemporarySymbolTable(symbols));
+            Term term = e.evaluate(variables, new TemporarySymbolTable(symbols));
 
-            if(res.isEmpty()) {
-               return Option.none();
-            }
-
-            if(!res.get().equals(new Term.Bool(true))) {
-               return Option.none();
+            if(!term.equals(new Term.Bool(true))) {
+               throw new Error.InvalidType();
             }
          }
 
diff --git a/src/main/java/org/biscuitsec/biscuit/datalog/Rule.java b/src/main/java/org/biscuitsec/biscuit/datalog/Rule.java
index c580627d..e30bb6dc 100644
--- a/src/main/java/org/biscuitsec/biscuit/datalog/Rule.java
+++ b/src/main/java/org/biscuitsec/biscuit/datalog/Rule.java
@@ -55,9 +55,9 @@ public Stream<Either<Error, Tuple2<Origin, Fact>>> apply(
                  Map<Long, Term> generatedVariables = t._2;
                  TemporarySymbolTable temporarySymbols = new TemporarySymbolTable(symbols);
                  for (Expression e : this.expressions) {
-                    Option<Term> res = e.evaluate(generatedVariables, temporarySymbols);
-                    if (res.isDefined()) {
-                       Term term = res.get();
+                    try {
+                       Term term = e.evaluate(generatedVariables, temporarySymbols);
+
                        if (term instanceof Term.Bool) {
                           Term.Bool b = (Term.Bool) term;
                           if (!b.value()) {
@@ -67,7 +67,10 @@ public Stream<Either<Error, Tuple2<Origin, Fact>>> apply(
                        } else {
                           return Either.left(new Error.InvalidType());
                        }
+                    } catch(Error error) {
+                       return Either.left(error);
                     }
+
                  }
                  return Either.right(new Tuple3(origin, generatedVariables, true));
               })
@@ -130,7 +133,7 @@ public boolean find_match(final FactSet facts, Long origin, TrustedOrigins scope
    }
 
    // verifies that the expressions return true for every matching set of facts
-   public boolean check_match_all(final FactSet facts, TrustedOrigins scope, SymbolTable symbols) throws Error.InvalidType {
+   public boolean check_match_all(final FactSet facts, TrustedOrigins scope, SymbolTable symbols) throws Error {
       MatchedVariables variables = variablesSet();
 
       if(this.body.isEmpty()) {
@@ -148,18 +151,16 @@ public boolean check_match_all(final FactSet facts, TrustedOrigins scope, Symbol
 
            TemporarySymbolTable temporarySymbols = new TemporarySymbolTable(symbols);
            for (Expression e : this.expressions) {
-              Option<Term> res = e.evaluate(generatedVariables, temporarySymbols);
-              if (res.isDefined()) {
-                 Term term = res.get();
-                 if (term instanceof Term.Bool) {
-                    Term.Bool b = (Term.Bool) term;
-                    if (!b.value()) {
-                       return false;
-                    }
-                    // continue evaluating if true
-                 } else {
-                    throw new Error.InvalidType();
+
+              Term term = e.evaluate(generatedVariables, temporarySymbols);
+              if (term instanceof Term.Bool) {
+                 Term.Bool b = (Term.Bool) term;
+                 if (!b.value()) {
+                    return false;
                  }
+                 // continue evaluating if true
+              } else {
+                 throw new Error.InvalidType();
               }
            }
        }
diff --git a/src/main/java/org/biscuitsec/biscuit/datalog/World.java b/src/main/java/org/biscuitsec/biscuit/datalog/World.java
index 217c318e..ca7df6e8 100644
--- a/src/main/java/org/biscuitsec/biscuit/datalog/World.java
+++ b/src/main/java/org/biscuitsec/biscuit/datalog/World.java
@@ -109,7 +109,7 @@ public final boolean query_match(final Rule rule, Long origin, TrustedOrigins sc
       return rule.find_match(this.facts, origin, scope, symbols);
    }
 
-   public final boolean query_match_all(final Rule rule, TrustedOrigins scope, SymbolTable symbols) throws Error.InvalidType {
+   public final boolean query_match_all(final Rule rule, TrustedOrigins scope, SymbolTable symbols) throws Error {
       return rule.check_match_all(this.facts, scope, symbols);
    }
 
diff --git a/src/main/java/org/biscuitsec/biscuit/datalog/expressions/Expression.java b/src/main/java/org/biscuitsec/biscuit/datalog/expressions/Expression.java
index 0b0e7107..98a1b96a 100644
--- a/src/main/java/org/biscuitsec/biscuit/datalog/expressions/Expression.java
+++ b/src/main/java/org/biscuitsec/biscuit/datalog/expressions/Expression.java
@@ -28,17 +28,16 @@ public ArrayList<Op> getOps() {
     }
 
     //FIXME: should return a Result<Term, error::Expression>
-    public Option<Term> evaluate(Map<Long, Term> variables, TemporarySymbolTable symbols) {
+    public Term evaluate(Map<Long, Term> variables, TemporarySymbolTable symbols) throws Error.Execution {
         Deque<Term> stack = new ArrayDeque<Term>(16); //Default value
         for(Op op: ops){
-            if(!op.evaluate(stack,variables, symbols)){
-                return Option.none();
-            }
+            System.out.println("evaluating "+op+": "+stack);
+            op.evaluate(stack,variables, symbols);
         }
         if(stack.size() == 1){
-            return Option.some(stack.pop());
+            return stack.pop();
         } else {
-            return Option.none();
+            throw new Error.Execution(this, "execution");
         }
     }
 
diff --git a/src/main/java/org/biscuitsec/biscuit/datalog/expressions/Op.java b/src/main/java/org/biscuitsec/biscuit/datalog/expressions/Op.java
index a5de31ff..05c7ece0 100644
--- a/src/main/java/org/biscuitsec/biscuit/datalog/expressions/Op.java
+++ b/src/main/java/org/biscuitsec/biscuit/datalog/expressions/Op.java
@@ -16,7 +16,7 @@
 import static io.vavr.API.Right;
 
 public abstract class Op {
-    public abstract boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporarySymbolTable symbols);
+    public abstract void evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporarySymbolTable symbols) throws Error.Execution;
 
     public abstract String print(Deque<String> stack, SymbolTable symbols);
 
@@ -46,19 +46,17 @@ public Term getValue() {
         }
 
         @Override
-        public boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporarySymbolTable symbols) {
+        public void evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporarySymbolTable symbols) throws Error.Execution {
             if (value instanceof Term.Variable) {
                 Term.Variable var = (Term.Variable) value;
                 Term valueVar = variables.get(var.value());
                 if (valueVar != null) {
                     stack.push(valueVar);
-                    return true;
                 } else {
-                    return false;
+                    throw new Error.Execution( "cannot find a variable for index "+value);
                 }
             } else {
                 stack.push(value);
-                return true;
             }
 
         }
@@ -118,7 +116,7 @@ public UnaryOp getOp() {
         }
 
         @Override
-        public boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporarySymbolTable symbols) {
+        public void evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporarySymbolTable symbols) throws Error.Execution {
             Term value = stack.pop();
             switch (this.op) {
                 case Negate:
@@ -126,7 +124,7 @@ public boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporaryS
                         Term.Bool b = (Term.Bool) value;
                         stack.push(new Term.Bool(!b.value()));
                     } else {
-                        return false;
+                        throw new Error.Execution("invalid type for negate op, expected boolean");
                     }
                     break;
                 case Parens:
@@ -136,7 +134,7 @@ public boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporaryS
                     if (value instanceof Term.Str) {
                         Option<String> s = symbols.get_s((int)((Term.Str) value).value());
                         if(s.isEmpty()) {
-                            return false;
+                            throw new Error.Execution("string not found in symbols for id"+value);
                         } else {
                             stack.push(new Term.Integer(s.get().length()));
                         }
@@ -145,10 +143,9 @@ public boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporaryS
                     } else if (value instanceof Term.Set) {
                         stack.push(new Term.Integer(((Term.Set) value).value().size()));
                     } else {
-                        return false;
+                        throw new Error.Execution("invalid type for length op");
                     }
             }
-            return true;
         }
 
         @Override
@@ -265,7 +262,7 @@ public BinaryOp getOp() {
         }
 
         @Override
-        public boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporarySymbolTable symbols) {
+        public void evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporarySymbolTable symbols) throws Error.Execution {
             Term right = stack.pop();
             Term left = stack.pop();
 
@@ -273,100 +270,81 @@ public boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporaryS
                 case LessThan:
                     if (right instanceof Term.Integer && left instanceof Term.Integer) {
                         stack.push(new Term.Bool(((Term.Integer) left).value() < ((Term.Integer) right).value()));
-                        return true;
                     }
                     if (right instanceof Term.Date && left instanceof Term.Date) {
                         stack.push(new Term.Bool(((Term.Date) left).value() < ((Term.Date) right).value()));
-                        return true;
                     }
                     break;
                 case GreaterThan:
                     if (right instanceof Term.Integer && left instanceof Term.Integer) {
                         stack.push(new Term.Bool(((Term.Integer) left).value() > ((Term.Integer) right).value()));
-                        return true;
                     }
                     if (right instanceof Term.Date && left instanceof Term.Date) {
                         stack.push(new Term.Bool(((Term.Date) left).value() > ((Term.Date) right).value()));
-                        return true;
                     }
                     break;
                 case LessOrEqual:
                     if (right instanceof Term.Integer && left instanceof Term.Integer) {
                         stack.push(new Term.Bool(((Term.Integer) left).value() <= ((Term.Integer) right).value()));
-                        return true;
                     }
                     if (right instanceof Term.Date && left instanceof Term.Date) {
                         stack.push(new Term.Bool(((Term.Date) left).value() <= ((Term.Date) right).value()));
-                        return true;
                     }
                     break;
                 case GreaterOrEqual:
                     if (right instanceof Term.Integer && left instanceof Term.Integer) {
                         stack.push(new Term.Bool(((Term.Integer) left).value() >= ((Term.Integer) right).value()));
-                        return true;
                     }
                     if (right instanceof Term.Date && left instanceof Term.Date) {
                         stack.push(new Term.Bool(((Term.Date) left).value() >= ((Term.Date) right).value()));
-                        return true;
                     }
                     break;
                 case Equal:
                     if (right instanceof Term.Bool && left instanceof Term.Bool) {
                         stack.push(new Term.Bool(((Term.Bool) left).value() == ((Term.Bool) right).value()));
-                        return true;
                     }
                     if (right instanceof Term.Integer && left instanceof Term.Integer) {
                         stack.push(new Term.Bool(((Term.Integer) left).value() == ((Term.Integer) right).value()));
-                        return true;
                     }
                     if (right instanceof Term.Str && left instanceof Term.Str) {
                         stack.push(new Term.Bool(((Term.Str) left).value() == ((Term.Str) right).value()));
-                        return true;
                     }
                     if (right instanceof Term.Bytes && left instanceof Term.Bytes) {
                         stack.push(new Term.Bool(Arrays.equals(((Term.Bytes) left).value(), (((Term.Bytes) right).value()))));
-                        return true;
                     }
                     if (right instanceof Term.Date && left instanceof Term.Date) {
                         stack.push(new Term.Bool(((Term.Date) left).value() == ((Term.Date) right).value()));
-                        return true;
                     }
                     if (right instanceof Term.Set && left instanceof Term.Set) {
                         Set<Term> leftSet = ((Term.Set) left).value();
                         Set<Term> rightSet = ((Term.Set) right).value();
                         stack.push(new Term.Bool( leftSet.size() == rightSet.size() && leftSet.containsAll(rightSet)));
-                        return true;
                     }
                     break;
                 case NotEqual:
                     if (right instanceof Term.Bool && left instanceof Term.Bool) {
                         stack.push(new Term.Bool(((Term.Bool) left).value() == ((Term.Bool) right).value()));
-                        return true;
                     }
                     if (right instanceof Term.Integer && left instanceof Term.Integer) {
                         stack.push(new Term.Bool(((Term.Integer) left).value() != ((Term.Integer) right).value()));
-                        return true;
                     }
                     if (right instanceof Term.Str && left instanceof Term.Str) {
                         stack.push(new Term.Bool(((Term.Str) left).value() != ((Term.Str) right).value()));
-                        return true;
                     }
                     if (right instanceof Term.Bytes && left instanceof Term.Bytes) {
                         stack.push(new Term.Bool(!Arrays.equals(((Term.Bytes) left).value(), (((Term.Bytes) right).value()))));
-                        return true;
                     }
                     if (right instanceof Term.Date && left instanceof Term.Date) {
                         stack.push(new Term.Bool(((Term.Date) left).value() != ((Term.Date) right).value()));
-                        return true;
                     }
                     if (right instanceof Term.Set && left instanceof Term.Set) {
                         Set<Term> leftSet = ((Term.Set) left).value();
                         Set<Term> rightSet = ((Term.Set) right).value();
                         stack.push(new Term.Bool( leftSet.size() != rightSet.size() || !leftSet.containsAll(rightSet)));
-                        return true;
                     }
                     break;
                 case Contains:
+                    System.out.println("calling contains op with left "+left+" and right "+right);
                     if (left instanceof Term.Set &&
                             (right instanceof Term.Integer ||
                                     right instanceof Term.Str ||
@@ -375,91 +353,98 @@ public boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporaryS
                                     right instanceof Term.Bool)) {
 
                         stack.push(new Term.Bool(((Term.Set) left).value().contains(right)));
-                        return true;
                     }
                     if (right instanceof Term.Set && left instanceof Term.Set) {
                         Set<Term> leftSet = ((Term.Set) left).value();
                         Set<Term> rightSet = ((Term.Set) right).value();
                         stack.push(new Term.Bool(leftSet.containsAll(rightSet)));
-                        return true;
                     }
                     if (left instanceof Term.Str && right instanceof Term.Str) {
                         Option<String> left_s = symbols.get_s((int)((Term.Str) left).value());
                         Option<String> right_s = symbols.get_s((int)((Term.Str) right).value());
 
-                        if (left_s.isEmpty() || right_s.isEmpty()) {
-                            return false;
-                        } else {
-                            stack.push(new Term.Bool(left_s.get().contains(right_s.get())));
-                            return true;
+                        if(left_s.isEmpty()) {
+                            throw new Error.Execution("cannot find string in symbols for index "+((Term.Str) left).value());
+                        }
+                        if(right_s.isEmpty()) {
+                            throw new Error.Execution("cannot find string in symbols for index "+((Term.Str) right).value());
                         }
+
+
+                        stack.push(new Term.Bool(left_s.get().contains(right_s.get())));
                     }
                     break;
                 case Prefix:
                     if (right instanceof Term.Str && left instanceof Term.Str) {
                         Option<String> left_s = symbols.get_s((int)((Term.Str) left).value());
                         Option<String> right_s = symbols.get_s((int)((Term.Str) right).value());
-                        if(left_s.isEmpty() || right_s.isEmpty()) {
-                            return false;
+                        if(left_s.isEmpty()) {
+                            throw new Error.Execution("cannot find string in symbols for index "+((Term.Str) left).value());
+                        }
+                        if(right_s.isEmpty()) {
+                            throw new Error.Execution("cannot find string in symbols for index "+((Term.Str) right).value());
                         }
 
                         stack.push(new Term.Bool(left_s.get().startsWith(right_s.get())));
-                        return true;
                     }
                     break;
                 case Suffix:
                     if (right instanceof Term.Str && left instanceof Term.Str) {
                         Option<String> left_s = symbols.get_s((int)((Term.Str) left).value());
                         Option<String> right_s = symbols.get_s((int)((Term.Str) right).value());
-                        if(left_s.isEmpty() || right_s.isEmpty()) {
-                            return false;
+                        if(left_s.isEmpty()) {
+                            throw new Error.Execution("cannot find string in symbols for index "+((Term.Str) left).value());
+                        }
+                        if(right_s.isEmpty()) {
+                            throw new Error.Execution("cannot find string in symbols for index "+((Term.Str) right).value());
                         }
                         stack.push(new Term.Bool(left_s.get().endsWith(right_s.get())));
-                        return true;
                     }
                     break;
                 case Regex:
                     if (right instanceof Term.Str && left instanceof Term.Str) {
                         Option<String> left_s = symbols.get_s((int)((Term.Str) left).value());
                         Option<String> right_s = symbols.get_s((int)((Term.Str) right).value());
-                        if(left_s.isEmpty() || right_s.isEmpty()) {
-                            return false;
+                        if(left_s.isEmpty()) {
+                            throw new Error.Execution("cannot find string in symbols for index "+((Term.Str) left).value());
+                        }
+                        if(right_s.isEmpty()) {
+                            throw new Error.Execution("cannot find string in symbols for index "+((Term.Str) right).value());
                         }
 
                         Pattern p = Pattern.compile(right_s.get());
                         Matcher m = p.matcher(left_s.get());
                         stack.push(new Term.Bool(m.find()));
-                        return true;
                     }
                     break;
                 case Add:
                     if (right instanceof Term.Integer && left instanceof Term.Integer) {
                         stack.push(new Term.Integer(((Term.Integer) left).value() + ((Term.Integer) right).value()));
-                        return true;
                     }
                     if (right instanceof Term.Str && left instanceof Term.Str) {
                         Option<String> left_s = symbols.get_s((int)((Term.Str) left).value());
                         Option<String> right_s = symbols.get_s((int)((Term.Str) right).value());
 
-                        if(left_s.isEmpty() || right_s.isEmpty()) {
-                            return false;
+                        if(left_s.isEmpty()) {
+                            throw new Error.Execution("cannot find string in symbols for index "+((Term.Str) left).value());
                         }
+                        if(right_s.isEmpty()) {
+                            throw new Error.Execution("cannot find string in symbols for index "+((Term.Str) right).value());
+                        }
+
                         String concatenation = left_s.get() + right_s.get();
                         long index = symbols.insert(concatenation);
                         stack.push(new Term.Str(index));
-                        return true;
                     }
                     break;
                 case Sub:
                     if (right instanceof Term.Integer && left instanceof Term.Integer) {
                         stack.push(new Term.Integer(((Term.Integer) left).value() - ((Term.Integer) right).value()));
-                        return true;
                     }
                     break;
                 case Mul:
                     if (right instanceof Term.Integer && left instanceof Term.Integer) {
                         stack.push(new Term.Integer(((Term.Integer) left).value() * ((Term.Integer) right).value()));
-                        return true;
                     }
                     break;
                 case Div:
@@ -467,20 +452,17 @@ public boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporaryS
                         long rl = ((Term.Integer) right).value();
                         if (rl != 0) {
                             stack.push(new Term.Integer(((Term.Integer) left).value() / rl));
-                            return true;
                         }
                     }
                     break;
                 case And:
                     if (right instanceof Term.Bool && left instanceof Term.Bool) {
                         stack.push(new Term.Bool(((Term.Bool) left).value() && ((Term.Bool) right).value()));
-                        return true;
                     }
                     break;
                 case Or:
                     if (right instanceof Term.Bool && left instanceof Term.Bool) {
                         stack.push(new Term.Bool(((Term.Bool) left).value() || ((Term.Bool) right).value()));
-                        return true;
                     }
                     break;
                 case Intersection:
@@ -494,7 +476,6 @@ public boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporaryS
                             }
                         }
                         stack.push(new Term.Set(intersec));
-                        return true;
                     }
                     break;
                 case Union:
@@ -505,7 +486,6 @@ public boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporaryS
                         union.addAll(_right);
                         union.addAll(_left);
                         stack.push(new Term.Set(union));
-                        return true;
                     }
                     break;
                 case BitwiseAnd:
@@ -513,7 +493,6 @@ public boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporaryS
                         long r = ((Term.Integer) right).value();
                         long l = ((Term.Integer) left).value();
                         stack.push(new Term.Integer(r & l));
-                        return true;
                     }
                     break;
                 case BitwiseOr:
@@ -521,7 +500,6 @@ public boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporaryS
                         long r = ((Term.Integer) right).value();
                         long l = ((Term.Integer) left).value();
                         stack.push(new Term.Integer(r | l));
-                        return true;
                     }
                     break;
                 case BitwiseXor:
@@ -529,13 +507,11 @@ public boolean evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporaryS
                         long r = ((Term.Integer) right).value();
                         long l = ((Term.Integer) left).value();
                         stack.push(new Term.Integer(r ^ l));
-                        return true;
                     }
                     break;
                 default:
-                    return false;
+                    throw new Error.Execution("binary exec error for op"+this);
             }
-            return false;
         }
 
         @Override
diff --git a/src/main/java/org/biscuitsec/biscuit/error/Error.java b/src/main/java/org/biscuitsec/biscuit/error/Error.java
index d78146d6..75858b7e 100644
--- a/src/main/java/org/biscuitsec/biscuit/error/Error.java
+++ b/src/main/java/org/biscuitsec/biscuit/error/Error.java
@@ -535,10 +535,18 @@ public JsonElement toJson(){
 
     public static class Execution extends Error {
         Expression e;
+        String message;
 
-        public Execution(Expression ex) {
+        public Execution(Expression ex, String msg) {
             e = ex;
+            message = msg;
         }
+
+        public Execution( String msg) {
+            e = null;
+            message = msg;
+        }
+
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;
@@ -548,6 +556,12 @@ public boolean equals(Object o) {
         public JsonElement toJson(){
             return new JsonPrimitive("Execution");
         }
+
+        @Override
+        public String toString() {
+            return "Execution error when evaluating expression '" + e +
+                    "': " + message;
+        }
     }
 
     public static class InvalidType extends Error {
diff --git a/src/test/java/org/biscuitsec/biscuit/builder/parser/ParserTest.java b/src/test/java/org/biscuitsec/biscuit/builder/parser/ParserTest.java
index f912abba..3c76890b 100644
--- a/src/test/java/org/biscuitsec/biscuit/builder/parser/ParserTest.java
+++ b/src/test/java/org/biscuitsec/biscuit/builder/parser/ParserTest.java
@@ -280,7 +280,7 @@ void testExpression() {
     }
 
     @Test
-    void testParens() {
+    void testParens() throws org.biscuitsec.biscuit.error.Error.Execution {
         Either<Error, Tuple2<String, Expression>> res =
                 Parser.expression("  1 + 2 * 3  ");
 
@@ -314,8 +314,8 @@ void testParens() {
         );
 
         Map<Long, org.biscuitsec.biscuit.datalog.Term> variables = new HashMap<>();
-        Option<org.biscuitsec.biscuit.datalog.Term> value = ex.evaluate(variables, new TemporarySymbolTable(s));
-        assertEquals(Option.some(new org.biscuitsec.biscuit.datalog.Term.Integer(7)), value);
+        org.biscuitsec.biscuit.datalog.Term value = ex.evaluate(variables, new TemporarySymbolTable(s));
+        assertEquals(new org.biscuitsec.biscuit.datalog.Term.Integer(7), value);
         assertEquals("1 + 2 * 3", ex.print(s).get());
 
 
@@ -356,8 +356,8 @@ void testParens() {
         );
 
         Map<Long, org.biscuitsec.biscuit.datalog.Term> variables2 = new HashMap<>();
-        Option<org.biscuitsec.biscuit.datalog.Term> value2 = ex2.evaluate(variables2, new TemporarySymbolTable(s2));
-        assertEquals(Option.some(new org.biscuitsec.biscuit.datalog.Term.Integer(9)), value2);
+        org.biscuitsec.biscuit.datalog.Term value2 = ex2.evaluate(variables2, new TemporarySymbolTable(s2));
+        assertEquals(new org.biscuitsec.biscuit.datalog.Term.Integer(9), value2);
         assertEquals("(1 + 2) * 3", ex2.print(s2).get());
     }
 }
\ No newline at end of file
diff --git a/src/test/java/org/biscuitsec/biscuit/datalog/ExpressionTest.java b/src/test/java/org/biscuitsec/biscuit/datalog/ExpressionTest.java
index 06e8a681..c11fbebb 100644
--- a/src/test/java/org/biscuitsec/biscuit/datalog/ExpressionTest.java
+++ b/src/test/java/org/biscuitsec/biscuit/datalog/ExpressionTest.java
@@ -2,6 +2,7 @@
 
 import org.biscuitsec.biscuit.datalog.expressions.Expression;
 import org.biscuitsec.biscuit.datalog.expressions.Op;
+import org.biscuitsec.biscuit.error.Error;
 import org.junit.jupiter.api.Test;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
@@ -12,7 +13,7 @@
 public class ExpressionTest {
 
     @Test
-    public void testNegate() {
+    public void testNegate() throws Error.Execution {
         SymbolTable symbols = new SymbolTable();
         symbols.add("a");
         symbols.add("b");
@@ -36,12 +37,12 @@ public void testNegate() {
 
         assertEquals(
                 new Term.Bool(true),
-                e.evaluate(variables, new TemporarySymbolTable(symbols)).get()
+                e.evaluate(variables, new TemporarySymbolTable(symbols))
         );
     }
 
     @Test
-    public void testAddsStr() {
+    public void testAddsStr() throws Error.Execution {
         SymbolTable symbols = new SymbolTable();
         symbols.add("a");
         symbols.add("b");
@@ -61,12 +62,12 @@ public void testAddsStr() {
 
         assertEquals(
                 new Term.Str(SymbolTable.DEFAULT_SYMBOLS_OFFSET + 2),
-                e.evaluate(new HashMap<>(),  new TemporarySymbolTable(symbols)).get()
+                e.evaluate(new HashMap<>(),  new TemporarySymbolTable(symbols))
         );
     }
 
     @Test
-    public void testContainsStr() {
+    public void testContainsStr() throws Error.Execution {
         SymbolTable symbols = new SymbolTable();
         symbols.add("ab");
         symbols.add("b");
@@ -85,12 +86,12 @@ public void testContainsStr() {
 
         assertEquals(
                 new Term.Bool(true),
-                e.evaluate(new HashMap<>(),  new TemporarySymbolTable(symbols)).get()
+                e.evaluate(new HashMap<>(),  new TemporarySymbolTable(symbols))
         );
     }
 
     @Test
-    public void testNegativeContainsStr() {
+    public void testNegativeContainsStr() throws Error.Execution {
         SymbolTable symbols = new SymbolTable();
         symbols.add("ab");
         symbols.add("b");
@@ -110,7 +111,7 @@ public void testNegativeContainsStr() {
 
         assertEquals(
                 new Term.Bool(false),
-                e.evaluate(new HashMap<>(),  new TemporarySymbolTable(symbols)).get()
+                e.evaluate(new HashMap<>(),  new TemporarySymbolTable(symbols))
         );
     }
 }

From 008ad353e28cb9a985ce16c21c6ea0d707cce08f Mon Sep 17 00:00:00 2001
From: Geoffroy Couprie <contact@geoffroycouprie.com>
Date: Sat, 13 Jan 2024 18:55:40 +0100
Subject: [PATCH 2/2] handle overflows in expression evaluation

---
 .../biscuit/datalog/expressions/Op.java       | 24 ++++++++++++++++---
 .../org/biscuitsec/biscuit/error/Error.java   | 21 ++++++++++++++--
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/src/main/java/org/biscuitsec/biscuit/datalog/expressions/Op.java b/src/main/java/org/biscuitsec/biscuit/datalog/expressions/Op.java
index 05c7ece0..a68ab70e 100644
--- a/src/main/java/org/biscuitsec/biscuit/datalog/expressions/Op.java
+++ b/src/main/java/org/biscuitsec/biscuit/datalog/expressions/Op.java
@@ -419,7 +419,13 @@ public void evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporarySymb
                     break;
                 case Add:
                     if (right instanceof Term.Integer && left instanceof Term.Integer) {
-                        stack.push(new Term.Integer(((Term.Integer) left).value() + ((Term.Integer) right).value()));
+                        try {
+                            stack.push(new Term.Integer(
+                                    Math.addExact(((Term.Integer) left).value(), ((Term.Integer) right).value())
+                            ));
+                        } catch (ArithmeticException e) {
+                            throw new Error.Execution(Error.Execution.Kind.Overflow, "overflow");
+                        }
                     }
                     if (right instanceof Term.Str && left instanceof Term.Str) {
                         Option<String> left_s = symbols.get_s((int)((Term.Str) left).value());
@@ -439,12 +445,24 @@ public void evaluate(Deque<Term> stack, Map<Long, Term> variables, TemporarySymb
                     break;
                 case Sub:
                     if (right instanceof Term.Integer && left instanceof Term.Integer) {
-                        stack.push(new Term.Integer(((Term.Integer) left).value() - ((Term.Integer) right).value()));
+                        try {
+                            stack.push(new Term.Integer(
+                                    Math.subtractExact(((Term.Integer) left).value(), ((Term.Integer) right).value())
+                            ));
+                        } catch (ArithmeticException e) {
+                            throw new Error.Execution(Error.Execution.Kind.Overflow, "overflow");
+                        }
                     }
                     break;
                 case Mul:
                     if (right instanceof Term.Integer && left instanceof Term.Integer) {
-                        stack.push(new Term.Integer(((Term.Integer) left).value() * ((Term.Integer) right).value()));
+                        try {
+                            stack.push(new Term.Integer(
+                                    Math.multiplyExact(((Term.Integer) left).value(), ((Term.Integer) right).value())
+                            ));
+                        } catch (ArithmeticException e) {
+                            throw new Error.Execution(Error.Execution.Kind.Overflow, "overflow");
+                        }
                     }
                     break;
                 case Div:
diff --git a/src/main/java/org/biscuitsec/biscuit/error/Error.java b/src/main/java/org/biscuitsec/biscuit/error/Error.java
index 75858b7e..a30d3014 100644
--- a/src/main/java/org/biscuitsec/biscuit/error/Error.java
+++ b/src/main/java/org/biscuitsec/biscuit/error/Error.java
@@ -18,7 +18,6 @@ public JsonElement toJson() {
         return new JsonObject();
     }
 
-
     public static class InternalError extends Error {}
 
     public static class FormatError extends Error {
@@ -534,17 +533,32 @@ public JsonElement toJson(){
     }
 
     public static class Execution extends Error {
+        public enum Kind {
+            Execution,
+            Overflow
+
+        }
         Expression e;
         String message;
 
+        Kind kind;
+
         public Execution(Expression ex, String msg) {
             e = ex;
             message = msg;
+            kind = Kind.Execution;
         }
 
         public Execution( String msg) {
             e = null;
             message = msg;
+            kind = Kind.Execution;
+        }
+
+        public Execution(Kind kind, String msg) {
+            e = null;
+            this.kind = kind;
+            message = msg;
         }
 
         @Override
@@ -554,7 +568,10 @@ public boolean equals(Object o) {
         }
         @Override
         public JsonElement toJson(){
-            return new JsonPrimitive("Execution");
+            JsonObject jo = new JsonObject();
+            jo.add("Execution", new JsonPrimitive(this.kind.toString()));
+            return jo;
+
         }
 
         @Override
