intern FFI call names

Instead of storing strings directly in the ops, do as we do for everything else and use the symbol table. This required duplicating `biscuit_parser::builder::Binary` and `Unary` in the `biscuit_auth::builder` module (which previously used the definitions from the `datalog` module directly). There is a lot of duplication between `biscuit_parser::builder` and `biscuit_auth::builder`, with a circular-ish dependency (biscuit_auth depends on biscuit parser, but code generated by the `ToTokens` impl blocks in biscuit parser depend on `biscuit_auth::builder`).
biscuit-auth · Nov 13, 2024 · 99c0718 · 99c0718
1 parent bf923cb
commit 99c0718
Show file tree

Hide file tree

Showing 9 changed files with 234 additions and 76 deletions.
diff --git a/biscuit-auth/samples/README.md b/biscuit-auth/samples/README.md
@@ -3146,7 +3146,7 @@ result: `Ok(0)`
 ### token
 
 authority:
-symbols: ["a", "equal strings"]
+symbols: ["test", "a", "equal strings"]
 
 public keys: []
 
@@ -3162,7 +3162,7 @@ allow if true;
 ```
 
 revocation ids:
-- `b1696fd9f9ec456d65a863df034cb132dc7dca076d16f5bc3e73986a4cc88cc4e7902dc8519cb60961e3f33799c147f874c7e0d7e12ef1b461e361e0c0aa580b`
+- `faf26fe6f5dfa08c114a0a29321405b6fb7be79b0d80694d27925f7deb01effe5707600e42fd74f9a1d2920466446d51949155f4548f0fd68f3e9326c7e12404`
 
 authorizer world:
 ```

diff --git a/biscuit-auth/samples/samples.json b/biscuit-auth/samples/samples.json
@@ -2920,6 +2920,7 @@
       "token": [
         {
           "symbols": [
+            "test",
             "a",
             "equal strings"
           ],
@@ -2950,7 +2951,7 @@
           },
           "authorizer_code": "allow if true;\n",
           "revocation_ids": [
-            "b1696fd9f9ec456d65a863df034cb132dc7dca076d16f5bc3e73986a4cc88cc4e7902dc8519cb60961e3f33799c147f874c7e0d7e12ef1b461e361e0c0aa580b"
+            "faf26fe6f5dfa08c114a0a29321405b6fb7be79b0d80694d27925f7deb01effe5707600e42fd74f9a1d2920466446d51949155f4548f0fd68f3e9326c7e12404"
           ]
         }
       }

diff --git a/biscuit-auth/samples/test035_ffi.bc b/biscuit-auth/samples/test035_ffi.bc
diff --git a/biscuit-auth/src/datalog/expression.rs b/biscuit-auth/src/datalog/expression.rs
@@ -1,6 +1,6 @@
 use crate::{builder, error};
 
-use super::{MapKey, Term};
+use super::{MapKey, SymbolIndex, Term};
 use super::{SymbolTable, TemporarySymbolTable};
 use regex::Regex;
 use std::sync::Arc;
@@ -71,7 +71,7 @@ pub enum Unary {
     Parens,
     Length,
     TypeOf,
-    Ffi(String),
+    Ffi(SymbolIndex),
 }
 
 impl Unary {
@@ -109,10 +109,14 @@ impl Unary {
                 Ok(Term::Str(sym))
             }
             (Unary::Ffi(name), i) => {
+                let name = symbols
+                    .get_symbol(*name)
+                    .ok_or(error::Expression::UnknownSymbol(*name))?
+                    .to_owned();
                 let fun = extern_funcs
-                    .get(name)
+                    .get(&name)
                     .ok_or(error::Expression::UndefinedExtern(name.to_owned()))?;
-                fun.call(symbols, name, i, None)
+                fun.call(symbols, &name, i, None)
             }
             _ => {
                 //println!("unexpected value type on the stack");
@@ -121,13 +125,15 @@ impl Unary {
         }
     }
 
-    pub fn print(&self, value: String, _symbols: &SymbolTable) -> String {
+    pub fn print(&self, value: String, symbols: &SymbolTable) -> String {
         match self {
             Unary::Negate => format!("!{}", value),
             Unary::Parens => format!("({})", value),
             Unary::Length => format!("{}.length()", value),
             Unary::TypeOf => format!("{}.type()", value),
-            Unary::Ffi(name) => format!("{value}.extern::{name}()"),
+            Unary::Ffi(name) => {
+                format!("{value}.extern::{}()", symbols.print_symbol_default(*name))
+            }
         }
     }
 }
@@ -163,7 +169,7 @@ pub enum Binary {
     All,
     Any,
     Get,
-    Ffi(String),
+    Ffi(SymbolIndex),
 }
 
 impl Binary {
@@ -501,10 +507,14 @@ impl Binary {
 
             // FFI
             (Binary::Ffi(name), left, right) => {
+                let name = symbols
+                    .get_symbol(*name)
+                    .ok_or(error::Expression::UnknownSymbol(*name))?
+                    .to_owned();
                 let fun = extern_funcs
-                    .get(name)
+                    .get(&name)
                     .ok_or(error::Expression::UndefinedExtern(name.to_owned()))?;
-                fun.call(symbols, name, left, Some(right))
+                fun.call(symbols, &name, left, Some(right))
             }
 
             _ => {
@@ -514,7 +524,7 @@ impl Binary {
         }
     }
 
-    pub fn print(&self, left: String, right: String, _symbols: &SymbolTable) -> String {
+    pub fn print(&self, left: String, right: String, symbols: &SymbolTable) -> String {
         match self {
             Binary::LessThan => format!("{} < {}", left, right),
             Binary::GreaterThan => format!("{} > {}", left, right),
@@ -544,7 +554,10 @@ impl Binary {
             Binary::All => format!("{left}.all({right})"),
             Binary::Any => format!("{left}.any({right})"),
             Binary::Get => format!("{left}.get({right})"),
-            Binary::Ffi(name) => format!("{left}.extern::{name}({right})"),
+            Binary::Ffi(name) => format!(
+                "{left}.extern::{}({right})",
+                symbols.print_symbol_default(*name)
+            ),
         }
     }
 }
@@ -1676,72 +1689,77 @@ mod tests {
         let mut symbols = SymbolTable::new();
         let i = symbols.insert("test");
         let j = symbols.insert("TeSt");
+        let test_bin = symbols.insert("test_bin");
+        let test_un = symbols.insert("test_un");
+        let test_closure = symbols.insert("test_closure");
+        let test_fn = symbols.insert("test_fn");
+        let id_fn = symbols.insert("id");
         let mut tmp_symbols = TemporarySymbolTable::new(&symbols);
         let ops = vec![
             Op::Value(Term::Integer(60)),
             Op::Value(Term::Integer(0)),
-            Op::Binary(Binary::Ffi("test_bin".to_owned())),
+            Op::Binary(Binary::Ffi(test_bin)),
             Op::Value(Term::Str(i)),
             Op::Value(Term::Str(j)),
-            Op::Binary(Binary::Ffi("test_bin".to_owned())),
+            Op::Binary(Binary::Ffi(test_bin)),
             Op::Binary(Binary::And),
             Op::Value(Term::Integer(42)),
-            Op::Unary(Unary::Ffi("test_un".to_owned())),
+            Op::Unary(Unary::Ffi(test_un)),
             Op::Binary(Binary::And),
             Op::Value(Term::Integer(42)),
-            Op::Unary(Unary::Ffi("test_closure".to_owned())),
+            Op::Unary(Unary::Ffi(test_closure)),
             Op::Binary(Binary::And),
             Op::Value(Term::Str(i)),
-            Op::Unary(Unary::Ffi("test_closure".to_owned())),
+            Op::Unary(Unary::Ffi(test_closure)),
             Op::Binary(Binary::And),
             Op::Value(Term::Integer(42)),
-            Op::Unary(Unary::Ffi("test_fn".to_owned())),
+            Op::Unary(Unary::Ffi(test_fn)),
             Op::Binary(Binary::And),
             Op::Value(Term::Integer(42)),
-            Op::Unary(Unary::Ffi("id".to_owned())),
+            Op::Unary(Unary::Ffi(id_fn)),
             Op::Value(Term::Integer(42)),
             Op::Binary(Binary::HeterogeneousEqual),
             Op::Binary(Binary::And),
             Op::Value(Term::Str(i)),
-            Op::Unary(Unary::Ffi("id".to_owned())),
+            Op::Unary(Unary::Ffi(id_fn)),
             Op::Value(Term::Str(i)),
             Op::Binary(Binary::HeterogeneousEqual),
             Op::Binary(Binary::And),
             Op::Value(Term::Bool(true)),
-            Op::Unary(Unary::Ffi("id".to_owned())),
+            Op::Unary(Unary::Ffi(id_fn)),
             Op::Value(Term::Bool(true)),
             Op::Binary(Binary::HeterogeneousEqual),
             Op::Binary(Binary::And),
             Op::Value(Term::Date(0)),
-            Op::Unary(Unary::Ffi("id".to_owned())),
+            Op::Unary(Unary::Ffi(id_fn)),
             Op::Value(Term::Date(0)),
             Op::Binary(Binary::HeterogeneousEqual),
             Op::Binary(Binary::And),
             Op::Value(Term::Bytes(vec![42])),
-            Op::Unary(Unary::Ffi("id".to_owned())),
+            Op::Unary(Unary::Ffi(id_fn)),
             Op::Value(Term::Bytes(vec![42])),
             Op::Binary(Binary::HeterogeneousEqual),
             Op::Binary(Binary::And),
             Op::Value(Term::Null),
-            Op::Unary(Unary::Ffi("id".to_owned())),
+            Op::Unary(Unary::Ffi(id_fn)),
             Op::Value(Term::Null),
             Op::Binary(Binary::HeterogeneousEqual),
             Op::Binary(Binary::And),
             Op::Value(Term::Array(vec![Term::Null])),
-            Op::Unary(Unary::Ffi("id".to_owned())),
+            Op::Unary(Unary::Ffi(id_fn)),
             Op::Value(Term::Array(vec![Term::Null])),
             Op::Binary(Binary::HeterogeneousEqual),
             Op::Binary(Binary::And),
             Op::Value(Term::Set(BTreeSet::from([Term::Null]))),
-            Op::Unary(Unary::Ffi("id".to_owned())),
+            Op::Unary(Unary::Ffi(id_fn)),
             Op::Value(Term::Set(BTreeSet::from([Term::Null]))),
             Op::Binary(Binary::HeterogeneousEqual),
             Op::Binary(Binary::And),
             Op::Value(Term::Map(BTreeMap::from([
                 (MapKey::Integer(42), Term::Null),
                 (MapKey::Str(i), Term::Null),
             ]))),
-            Op::Unary(Unary::Ffi("id".to_owned())),
+            Op::Unary(Unary::Ffi(id_fn)),
             Op::Value(Term::Map(BTreeMap::from([
                 (MapKey::Integer(42), Term::Null),
                 (MapKey::Str(i), Term::Null),

diff --git a/biscuit-auth/src/format/convert.rs b/biscuit-auth/src/format/convert.rs
@@ -744,7 +744,7 @@ pub mod v2 {
                     (Some(op_unary::Kind::Parens), None) => Op::Unary(Unary::Parens),
                     (Some(op_unary::Kind::Length), None) => Op::Unary(Unary::Length),
                     (Some(op_unary::Kind::TypeOf), None) => Op::Unary(Unary::TypeOf),
-                    (Some(op_unary::Kind::Ffi), Some(n)) => Op::Unary(Unary::Ffi(n.to_owned())),
+                    (Some(op_unary::Kind::Ffi), Some(n)) => Op::Unary(Unary::Ffi(*n)),
                     (Some(op_unary::Kind::Ffi), None) => {
                         return Err(error::Format::DeserializationError(
                             "deserialization error: missing ffi name".to_string(),
@@ -799,7 +799,7 @@ pub mod v2 {
                     (Some(op_binary::Kind::All), None) => Op::Binary(Binary::All),
                     (Some(op_binary::Kind::Any), None) => Op::Binary(Binary::Any),
                     (Some(op_binary::Kind::Get), None) => Op::Binary(Binary::Get),
-                    (Some(op_binary::Kind::Ffi), Some(n)) => Op::Binary(Binary::Ffi(n.to_owned())),
+                    (Some(op_binary::Kind::Ffi), Some(n)) => Op::Binary(Binary::Ffi(*n)),
                     (Some(op_binary::Kind::Ffi), None) => {
                         return Err(error::Format::DeserializationError(
                             "deserialization error: missing ffi name".to_string(),

diff --git a/biscuit-auth/src/format/schema.proto b/biscuit-auth/src/format/schema.proto
@@ -151,7 +151,7 @@ message OpUnary {
   }
 
   required Kind kind = 1;
-  optional string ffiName = 2;
+  optional uint64 ffiName = 2;
 }
 
 message OpBinary {
@@ -188,7 +188,7 @@ message OpBinary {
   }
 
   required Kind kind = 1;
-  optional string ffiName = 2;
+  optional uint64 ffiName = 2;
 }
 
 message OpClosure {

diff --git a/biscuit-auth/src/format/schema.rs b/biscuit-auth/src/format/schema.rs
@@ -233,8 +233,8 @@ pub mod op {
 pub struct OpUnary {
     #[prost(enumeration="op_unary::Kind", required, tag="1")]
     pub kind: i32,
-    #[prost(string, optional, tag="2")]
-    pub ffi_name: ::core::option::Option<::prost::alloc::string::String>,
+    #[prost(uint64, optional, tag="2")]
+    pub ffi_name: ::core::option::Option<u64>,
 }
 /// Nested message and enum types in `OpUnary`.
 pub mod op_unary {
@@ -252,8 +252,8 @@ pub mod op_unary {
 pub struct OpBinary {
     #[prost(enumeration="op_binary::Kind", required, tag="1")]
     pub kind: i32,
-    #[prost(string, optional, tag="2")]
-    pub ffi_name: ::core::option::Option<::prost::alloc::string::String>,
+    #[prost(uint64, optional, tag="2")]
+    pub ffi_name: ::core::option::Option<u64>,
 }
 /// Nested message and enum types in `OpBinary`.
 pub mod op_binary {