fix(semantics): fix incorrect type checking that prevented `IpAddr in

src/ast.rs

@@ -54,6 +54,7 @@ impl PartialEq for Value {
             }
             (Self::String(s1), Self::String(s2)) => s1 == s2,
             (Self::IpCidr(i1), Self::IpCidr(i2)) => i1 == i2,
+            (Self::IpAddr(i1), Self::IpAddr(i2)) => i1 == i2,
             (Self::Int(i1), Self::Int(i2)) => i1 == i2,
             _ => false,
         }

src/atc_grammar.pest

@@ -26,7 +26,7 @@ ipv6_literal = @{
 }
 ipv4_cidr_literal = @{ ipv4_literal ~ "/" ~ ASCII_DIGIT{1,2} }
 ipv6_cidr_literal = @{ ipv6_literal ~ "/" ~ ASCII_DIGIT{1,3} }
-ip_literal = { ipv4_cidr_literal | ipv6_cidr_literal | ipv4_literal | ipv6_literal }
+ip_literal = _{ ipv4_cidr_literal | ipv6_cidr_literal | ipv4_literal | ipv6_literal }
 
 
 binary_operator = { "==" | "!=" | "~" | "^=" | "=^" | ">=" |

src/interpreter.rs

@@ -216,16 +216,19 @@ impl Execute for Predicate {
                     }
                 }
                 BinaryOperator::In => match (lhs_value, &self.rhs) {
-                    (Value::String(l), Value::String(r)) => {
+                    (Value::IpAddr(l), Value::IpCidr(r)) => {
                         if r.contains(l) {
                             matched = true;
                             if any {
                                 return true;
                             }
                         }
                     }
+                    _ => unreachable!(),
+                },
+                BinaryOperator::NotIn => match (lhs_value, &self.rhs) {
                     (Value::IpAddr(l), Value::IpCidr(r)) => {
-                        if r.contains(l) {
+                        if !r.contains(l) {
                             matched = true;
                             if any {
                                 return true;
@@ -234,24 +237,6 @@ impl Execute for Predicate {
                     }
                     _ => unreachable!(),
                 },
-                BinaryOperator::NotIn => {
-                    let rhs = match &self.rhs {
-                        Value::String(s) => s,
-                        _ => unreachable!(),
-                    };
-                    let lhs = match lhs_value {
-                        Value::String(s) => s,
-                        _ => unreachable!(),
-                    };
-
-                    if !rhs.contains(lhs) {
-                        if any {
-                            return true;
-                        }
-
-                        matched = true;
-                    }
-                }
                 BinaryOperator::Contains => {
                     let rhs = match &self.rhs {
                         Value::String(s) => s,
@@ -277,7 +262,7 @@ impl Execute for Predicate {
         // if we reached here, it means that `any` did not find a match,
         // or we passed all matches for `all`. So we simply need to return
         // !any && lhs_values.len() > 0 to cover both cases
-        return !any && lhs_values.len() > 0;
+        !any && !lhs_values.is_empty()
     }
 }
 

src/parser.rs

@@ -9,6 +9,7 @@ use pest::error::ErrorVariant;
 use pest::prec_climber::{Assoc, Operator, PrecClimber};
 use pest_consume::{match_nodes, Error as ParseError, Parser};
 use regex::Regex;
+use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
 
 type ParseResult<T> = Result<T, ParseError<Rule>>;
 /// cbindgen:ignore
@@ -90,7 +91,7 @@ impl ATCParser {
     }
     fn rawstr_literal(input: Node) -> ParseResult<String> {
         let mut s = String::new();
-       
+
         for node in input.into_children() {
             match node.as_rule() {
                 Rule::rawstr_char => s.push_str(node.as_str()),
@@ -109,25 +110,12 @@ impl ATCParser {
         input.as_str().parse().into_parse_result(&input)
     }
 
-    fn ipv4_literal(input: Node) -> ParseResult<Ipv4Cidr> {
-        format!("{}/32", input.as_str())
-            .parse()
-            .into_parse_result(&input)
-    }
-
-    fn ipv6_literal(input: Node) -> ParseResult<Ipv6Cidr> {
-        format!("{}/128", input.as_str())
-            .parse()
-            .into_parse_result(&input)
+    fn ipv4_literal(input: Node) -> ParseResult<Ipv4Addr> {
+        input.as_str().parse().into_parse_result(&input)
     }
 
-    fn ip_literal(input: Node) -> ParseResult<IpCidr> {
-        Ok(match_nodes! { input.children();
-            [ipv4_cidr_literal(c)] => IpCidr::V4(c),
-            [ipv6_cidr_literal(c)] => IpCidr::V6(c),
-            [ipv4_literal(c)] => IpCidr::V4(c),
-            [ipv6_literal(c)] => IpCidr::V6(c),
-        })
+    fn ipv6_literal(input: Node) -> ParseResult<Ipv6Addr> {
+        input.as_str().parse().into_parse_result(&input)
     }
 
     fn int_literal(input: Node) -> ParseResult<i64> {
@@ -155,7 +143,10 @@ impl ATCParser {
         Ok(match_nodes! { input.children();
             [str_literal(s)] => Value::String(s),
             [rawstr_literal(s)] => Value::String(s),
-            [ip_literal(ip)] => Value::IpCidr(ip),
+            [ipv4_cidr_literal(c)] => Value::IpCidr(IpCidr::V4(c)),
+            [ipv6_cidr_literal(c)] => Value::IpCidr(IpCidr::V6(c)),
+            [ipv4_literal(i)] => Value::IpAddr(IpAddr::V4(i)),
+            [ipv6_literal(i)] => Value::IpAddr(IpAddr::V6(i)),
             [int_literal(i)] => Value::Int(i),
         })
     }

src/semantics.rs

@@ -81,7 +81,11 @@ impl Validate for Expression {
                 }
                 let lhs_type = lhs_type.unwrap();
 
-                if p.op != BinaryOperator::Regex && lhs_type != &p.rhs.my_type() {
+                if p.op != BinaryOperator::Regex // Regex RHS is always Regex, and LHS is always String
+                    && p.op != BinaryOperator::In // In/NotIn supports IPAddr in IpCidr
+                    && p.op != BinaryOperator::NotIn
+                    && lhs_type != &p.rhs.my_type()
+                {
                     return Err(
                         "Type mismatch between the LHS and RHS values of predicate".to_string()
                     );
@@ -123,11 +127,11 @@ impl Validate for Expression {
                         }
                     },
                     BinaryOperator::In | BinaryOperator::NotIn => {
-                        match p.rhs {
-                            Value::String(_) | Value::IpCidr(_) => {
+                        match (lhs_type, &p.rhs,) {
+                            (Type::IpAddr, Value::IpCidr(_)) => {
                                 Ok(())
                             }
-                            _ => Err("In/NotIn operators only supports string/IP cidr operands".to_string())
+                            _ => Err("In/NotIn operators only supports IP in CIDR".to_string())
                         }
                     },
                     BinaryOperator::Contains => {

t/07-in_notin.t

@@ -0,0 +1,107 @@
+# vim:set ft= ts=4 sw=4 et:
+
+use Test::Nginx::Socket::Lua;
+use Cwd qw(cwd);
+
+repeat_each(2);
+
+plan tests => repeat_each() * blocks() * 5;
+
+my $pwd = cwd();
+
+our $HttpConfig = qq{
+    lua_package_path "$pwd/lib/?.lua;;";
+    lua_package_cpath "$pwd/target/debug/?.so;;";
+};
+
+no_long_string();
+no_diff();
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: in operator has correct type check
+--- http_config eval: $::HttpConfig
+--- config
+    location = /t {
+        content_by_lua_block {
+            local schema = require("resty.router.schema")
+            local router = require("resty.router.router")
+            local context = require("resty.router.context")
+
+            local s = schema.new()
+
+            s:add_field("http.path", "String")
+            s:add_field("tcp.port", "Int")
+
+            local r = router.new(s)
+            ngx.say(r:add_matcher(0, "a921a9aa-ec0e-4cf3-a6cc-1aa5583d150c",
+                                  "tcp.port in 80"))
+
+            ngx.say(r:add_matcher(0, "a921a9aa-ec0e-4cf3-a6cc-1aa5583d150c",
+                                  "http.path in 80"))
+
+            ngx.say(r:add_matcher(0, "a921a9aa-ec0e-4cf3-a6cc-1aa5583d150c",
+                                  "http.path in \"foo\""))
+        }
+    }
+--- request
+GET /t
+--- response_body
+nilIn/NotIn operators only supports IP in CIDR
+nilIn/NotIn operators only supports IP in CIDR
+nilIn/NotIn operators only supports IP in CIDR
+--- no_error_log
+[error]
+[warn]
+[crit]
+
+
+
+=== TEST 2: in operator works with IPAddr and IpCidr operands
+--- http_config eval: $::HttpConfig
+--- config
+    location = /t {
+        content_by_lua_block {
+            local schema = require("resty.router.schema")
+            local router = require("resty.router.router")
+            local context = require("resty.router.context")
+
+            local s = schema.new()
+
+            s:add_field("l3.ip", "IpAddr")
+
+            local r = router.new(s)
+            assert(r:add_matcher(0, "a921a9aa-ec0e-4cf3-a6cc-1aa5583d150c",
+                                 "l3.ip in 192.168.12.0/24"))
+
+            local c = context.new(s)
+            c:add_value("l3.ip", "192.168.12.1")
+
+            local matched = r:execute(c)
+            ngx.say(matched)
+
+            c = context.new(s)
+            c:add_value("l3.ip", "192.168.1.1")
+
+            local matched = r:execute(c)
+            ngx.say(matched)
+
+            assert(r:remove_matcher("a921a9aa-ec0e-4cf3-a6cc-1aa5583d150c"))
+            assert(r:add_matcher(0, "a921a9aa-ec0e-4cf3-a6cc-1aa5583d150c",
+                                 "l3.ip not in 192.168.12.0/24"))
+            local matched = r:execute(c)
+            ngx.say(matched)
+        }
+    }
+--- request
+GET /t
+--- response_body
+true
+false
+true
+--- no_error_log
+[error]
+[warn]
+[crit]