fix rule match order with ip resolve

clash_lib/src/app/router/mod.rs

@@ -9,7 +9,7 @@ use crate::{
 use crate::{
     common::mmdb::Mmdb,
     config::internal::{config::RuleProviderDef, rule::RuleType},
-    session::{Session, SocksAddr},
+    session::Session,
 };
 
 use crate::app::router::rules::final_::Final;
@@ -102,8 +102,7 @@ impl Router {
                     .resolve(sess.destination.domain().unwrap(), false)
                     .await
                 {
-                    sess_dup.destination =
-                        SocksAddr::from((ip, sess.destination.port()));
+                    sess_dup.resolved_ip = Some(ip);
                     sess_resolved = true;
                 }
             }
@@ -309,9 +308,157 @@ pub fn map_rule_type(
                     .clone(),
             )),
             None => {
-                unreachable!("you shouldn't next rule-set within another rule-set")
+                unreachable!("you shouldn't nest rule-set within another rule-set")
             }
         },
         RuleType::Match { target } => Box::new(Final { target }),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::sync::Arc;
+
+    use anyhow::Ok;
+
+    use crate::{
+        app::dns::{MockClashResolver, SystemResolver},
+        common::{geodata::GeoData, http::new_http_client, mmdb::Mmdb},
+        config::internal::rule::RuleType,
+        session::Session,
+    };
+
+    const GEO_DATA_DOWNLOAD_URL:&str = "https://github.com/Watfaq/v2ray-rules-dat/releases/download/test/geosite.dat";
+    const MMDB_DOWNLOAD_URL:&str = "https://github.com/Loyalsoldier/geoip/releases/download/202307271745/Country.mmdb";
+
+    #[tokio::test]
+    async fn test_route_match() {
+        let mut mock_resolver = MockClashResolver::new();
+        mock_resolver.expect_resolve().returning(|host, _| {
+            if host == "china.com" {
+                Ok(Some("114.114.114.114".parse().unwrap()))
+            } else if host == "t.me" {
+                Ok(Some("149.154.0.1".parse().unwrap()))
+            } else if host == "git.io" {
+                Ok(Some("8.8.8.8".parse().unwrap()))
+            } else {
+                Ok(None)
+            }
+        });
+        let mock_resolver = Arc::new(mock_resolver);
+
+        let real_resolver = Arc::new(SystemResolver::new(false).unwrap());
+
+        let client = new_http_client(real_resolver.clone()).unwrap();
+
+        let temp_dir = tempfile::tempdir().unwrap();
+
+        let mmdb = Mmdb::new(
+            temp_dir.path().join("mmdb.mmdb"),
+            Some(MMDB_DOWNLOAD_URL.to_string()),
+            client,
+        )
+        .await
+        .unwrap();
+
+        let client = new_http_client(real_resolver.clone()).unwrap();
+
+        let geodata = GeoData::new(
+            temp_dir.path().join("geodata.geodata"),
+            Some(GEO_DATA_DOWNLOAD_URL.to_string()),
+            client,
+        )
+        .await
+        .unwrap();
+
+        let router = super::Router::new(
+            vec![
+                RuleType::GeoIP {
+                    target: "DIRECT".to_string(),
+                    country_code: "CN".to_string(),
+                    no_resolve: false,
+                },
+                RuleType::DomainSuffix {
+                    domain_suffix: "t.me".to_string(),
+                    target: "DS".to_string(),
+                },
+                RuleType::IpCidr {
+                    ipnet: "149.154.0.0/16".parse().unwrap(),
+                    target: "IC".to_string(),
+                    no_resolve: false,
+                },
+                RuleType::DomainSuffix {
+                    domain_suffix: "git.io".to_string(),
+                    target: "DS2".to_string(),
+                },
+            ],
+            Default::default(),
+            mock_resolver,
+            Arc::new(mmdb),
+            Arc::new(geodata),
+            temp_dir.path().to_str().unwrap().to_string(),
+        )
+        .await;
+
+        assert_eq!(
+            router
+                .match_route(&Session {
+                    destination: crate::session::SocksAddr::Domain(
+                        "china.com".to_string(),
+                        1111,
+                    ),
+                    ..Default::default()
+                })
+                .await
+                .0,
+            "DIRECT",
+            "should resolve and match IP"
+        );
+
+        assert_eq!(
+            router
+                .match_route(&Session {
+                    destination: crate::session::SocksAddr::Domain(
+                        "t.me".to_string(),
+                        1111,
+                    ),
+                    ..Default::default()
+                })
+                .await
+                .0,
+            "DS",
+            "should match domain"
+        );
+
+        assert_eq!(
+            router
+                .match_route(&Session {
+                    destination: crate::session::SocksAddr::Domain(
+                        "git.io".to_string(),
+                        1111
+                    ),
+                    ..Default::default()
+                })
+                .await
+                .0,
+            "DS2",
+            "should still match domain after previous rule resolved IP and non \
+             match"
+        );
+
+        assert_eq!(
+            router
+                .match_route(&Session {
+                    destination: crate::session::SocksAddr::Domain(
+                        "no-match".to_string(),
+                        1111
+                    ),
+                    ..Default::default()
+                })
+                .await
+                .0,
+            "MATCH",
+            "should fallback to MATCH when nothing matched"
+        );
+    }
+}

clash_lib/src/app/router/rules/geoip.rs

@@ -22,9 +22,14 @@ impl std::fmt::Display for GeoIP {
 
 impl RuleMatcher for GeoIP {
     fn apply(&self, sess: &Session) -> bool {
-        match sess.destination {
-            crate::session::SocksAddr::Ip(addr) => match self.mmdb.lookup(addr.ip())
-            {
+        let ip = if self.no_resolve {
+            sess.destination.ip()
+        } else {
+            sess.resolved_ip
+        };
+
+        if let Some(ip) = ip {
+            match self.mmdb.lookup(ip) {
                 Ok(country) => {
                     country
                         .country
@@ -37,8 +42,9 @@ impl RuleMatcher for GeoIP {
                     debug!("GeoIP lookup failed: {}", e);
                     false
                 }
-            },
-            crate::session::SocksAddr::Domain(..) => false,
+            }
+        } else {
+            false
         }
     }
 

clash_lib/src/app/router/rules/ipcidr.rs

@@ -1,7 +1,4 @@
-use crate::{
-    app::router::rules::RuleMatcher,
-    session::{Session, SocksAddr},
-};
+use crate::{app::router::rules::RuleMatcher, session::Session};
 
 #[derive(Clone)]
 pub struct IpCidr {
@@ -25,12 +22,20 @@ impl std::fmt::Display for IpCidr {
 
 impl RuleMatcher for IpCidr {
     fn apply(&self, sess: &Session) -> bool {
-        match self.match_src {
-            true => self.ipnet.contains(&sess.source.ip()),
-            false => match &sess.destination {
-                SocksAddr::Ip(ip) => self.ipnet.contains(&ip.ip()),
-                SocksAddr::Domain(..) => false,
-            },
+        if self.match_src {
+            self.ipnet.contains(&sess.source.ip())
+        } else {
+            let ip = if self.no_resolve {
+                sess.destination.ip()
+            } else {
+                sess.resolved_ip
+            };
+
+            if let Some(ip) = ip {
+                self.ipnet.contains(&ip)
+            } else {
+                false
+            }
         }
     }
 

clash_lib/src/proxy/tun/inbound.rs

@@ -45,6 +45,7 @@ async fn handle_inbound_stream(
                 );
             }),
         so_mark: Some(so_mark),
+        ..Default::default()
     };
 
     debug!("new tun TCP session assigned: {}", sess);

clash_lib/src/session.rs

@@ -392,6 +392,8 @@ pub struct Session {
     pub source: SocketAddr,
     /// The proxy target address of a proxy connection.
     pub destination: SocksAddr,
+    /// The locally resolved IP address of the destination domain.
+    pub resolved_ip: Option<IpAddr>,
     /// The packet mark SO_MARK
     pub so_mark: Option<u32>,
     /// The bind interface
@@ -426,6 +428,7 @@ impl Default for Session {
             typ: Type::Http,
             source: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 0),
             destination: SocksAddr::any_ipv4(),
+            resolved_ip: None,
             so_mark: None,
             iface: None,
         }
@@ -461,6 +464,7 @@ impl Clone for Session {
             typ: self.typ,
             source: self.source,
             destination: self.destination.clone(),
+            resolved_ip: self.resolved_ip,
             so_mark: self.so_mark,
             iface: self.iface.as_ref().cloned(),
         }