Error on integer overflows in expressions (#82)

* use exceptions in expression evaluation * handle overflows in expression evaluation
biscuit-auth · Jan 20, 2024 · f8ad566 · f8ad566
1 parent ac7015c
commit f8ad566
Show file tree

Hide file tree

Showing 8 changed files with 135 additions and 112 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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");
         }
     }