Prechádzať zdrojové kódy

webui: fix config option tail space issue, add some init log and fix memory leak when exit.

Nick Peng 2 mesiacov pred
rodič
commit
0cfa5d69be

+ 1 - 1
etc/smartdns/smartdns.conf

@@ -443,7 +443,7 @@ log-level info
 # plugin smartdns_ui.so
 # smartdns-ui.www-root /usr/share/smartdns/wwwroot
 # smartdns-ui.ip http://0.0.0.0:6080
-# smartdns-ui.ip https://0.0.0.0:6080
+# smartdns-ui.ip https://[::]:6080
 # smartdns-ui.token-expire 600
 # smartdns-ui.max-query-log-age 86400
 # smartdns-ui.enable-terminal yes

+ 1 - 1
package/openwrt/files/etc/init.d/smartdns

@@ -871,7 +871,7 @@ load_service()
 
 		conf_append "plugin" "smartdns_ui.so"
 		conf_append "smartdns-ui.www-root" "/usr/share/smartdns/wwwroot"
-		conf_append "smartdns-ui.ip" "http://0.0.0.0:$ui_port"
+		conf_append "smartdns-ui.ip" "http://[::]:$ui_port"
 		conf_append "data-dir" "$ui_data_dir"
 		conf_append "smartdns-ui.max-query-log-age" "$ui_log_max_age_s"
 	}

+ 69 - 22
plugin/smartdns-ui/src/data_server.rs

@@ -32,6 +32,7 @@ use crate::whois::WhoIsInfo;
 use std::collections::HashMap;
 use std::error::Error;
 use std::sync::atomic::AtomicBool;
+use std::sync::Weak;
 use std::sync::{Arc, Mutex, RwLock};
 use tokio::sync::mpsc;
 use tokio::task::JoinHandle;
@@ -43,6 +44,7 @@ pub const DEFAULT_MAX_LOG_AGE_MS: u64 = DEFAULT_MAX_LOG_AGE * 1000;
 pub const MAX_LOG_AGE_VALUE_MIN: u64 = 3600;
 pub const MAX_LOG_AGE_VALUE_MAX: u64 = 365 * 24 * 60 * 60 * 10;
 pub const MIN_FREE_DISK_SPACE: u64 = 1024 * 1024 * 8;
+pub const DB_FILE_NAME: &str = "smartdns.db";
 
 #[derive(Clone)]
 pub struct OverviewData {
@@ -68,14 +70,16 @@ pub struct MetricsData {
 
 #[derive(Clone)]
 pub struct DataServerConfig {
-    pub data_root: String,
+    pub db_file: String,
+    pub data_path: String,
     pub max_log_age_ms: u64,
 }
 
 impl DataServerConfig {
     pub fn new() -> Self {
         DataServerConfig {
-            data_root: Plugin::dns_conf_data_dir() + "/smartdns.db",
+            data_path: Plugin::dns_conf_data_dir(),
+            db_file: Plugin::dns_conf_data_dir() + "/" + DB_FILE_NAME,
             max_log_age_ms: DEFAULT_MAX_LOG_AGE_MS,
         }
     }
@@ -110,7 +114,7 @@ pub struct DataServerControl {
     server_thread: Mutex<Option<JoinHandle<()>>>,
     is_init: Mutex<bool>,
     is_run: Mutex<bool>,
-    plugin: Mutex<Option<Arc<SmartdnsPlugin>>>,
+    plugin: Mutex<Weak<SmartdnsPlugin>>,
 }
 
 impl DataServerControl {
@@ -120,7 +124,7 @@ impl DataServerControl {
             server_thread: Mutex::new(None),
             is_init: Mutex::new(false),
             is_run: Mutex::new(false),
-            plugin: Mutex::new(None),
+            plugin: Mutex::new(Weak::new()),
         }
     }
 
@@ -129,12 +133,19 @@ impl DataServerControl {
     }
 
     pub fn set_plugin(&self, plugin: Arc<SmartdnsPlugin>) {
-        *self.plugin.lock().unwrap() = Some(plugin);
+        *self.plugin.lock().unwrap() = Arc::downgrade(&plugin);
     }
 
-    pub fn get_plugin(&self) -> Arc<SmartdnsPlugin> {
-        let plugin = self.plugin.lock().unwrap();
-        Arc::clone(&plugin.as_ref().unwrap())
+    pub fn get_plugin(&self) -> Result<Arc<SmartdnsPlugin>, Box<dyn Error>> {
+        let plugin = match self.plugin.lock() {
+            Ok(plugin) => plugin,
+            Err(_) => return Err("Failed to lock plugin mutex".into()),
+        };
+
+        if let Some(plugin) = plugin.upgrade() {
+            return Ok(plugin);
+        }
+        Err("Plugin is not set".into())
     }
 
     pub fn init_db(&self, conf: &DataServerConfig) -> Result<(), Box<dyn Error>> {
@@ -155,8 +166,9 @@ impl DataServerControl {
             return Err("data server not init".into());
         }
 
-        self.data_server.set_plugin(self.get_plugin());
-        let rt = self.get_plugin().get_runtime();
+        let plugin = self.get_plugin()?;
+        self.data_server.set_plugin(plugin.clone());
+        let rt = plugin.get_runtime();
 
         let server_thread = rt.spawn(async move {
             let ret = DataServer::data_server_loop(inner_clone).await;
@@ -181,7 +193,18 @@ impl DataServerControl {
         self.data_server.stop_data_server();
         let _server_thread = self.server_thread.lock().unwrap().take();
         if let Some(server_thread) = _server_thread {
-            let rt = self.get_plugin().get_runtime();
+            let plugin = self.get_plugin();
+            if plugin.is_err() {
+                dns_log!(
+                    LogLevel::ERROR,
+                    "get plugin error: {}",
+                    plugin.err().unwrap()
+                );
+                return;
+            }
+
+            let plugin = plugin.unwrap();
+            let rt = plugin.get_runtime();
             tokio::task::block_in_place(|| {
                 if let Err(e) = rt.block_on(server_thread) {
                     dns_log!(LogLevel::ERROR, "http server stop error: {}", e);
@@ -233,7 +256,7 @@ pub struct DataServer {
     disable_handle_request: AtomicBool,
     stat: Arc<DataStats>,
     server_log: ServerLog,
-    plugin: Mutex<Option<Arc<SmartdnsPlugin>>>,
+    plugin: Mutex<Weak<SmartdnsPlugin>>,
     whois: whois::WhoIs,
     startup_timestamp: u64,
     recv_in_batch: Mutex<bool>,
@@ -252,7 +275,7 @@ impl DataServer {
             db: db.clone(),
             stat: DataStats::new(db, conf.clone()),
             server_log: ServerLog::new(),
-            plugin: Mutex::new(None),
+            plugin: Mutex::new(Weak::new()),
             whois: whois::WhoIs::new(),
             startup_timestamp: get_utc_time_ms(),
             disable_handle_request: AtomicBool::new(false),
@@ -284,8 +307,17 @@ impl DataServer {
 
         smartdns::smartdns_enable_update_neighbour(true);
 
-        dns_log!(LogLevel::INFO, "open db: {}", conf_clone.data_root);
-        let ret = self.db.open(&conf_clone.data_root);
+        if utils::is_dir_writable(&conf_clone.data_path) == false {
+            return Err(format!(
+                "data path '{}' is not exist or writable.",
+                conf_clone.data_path
+            )
+            .into());
+        }
+
+        conf_clone.db_file = conf_clone.data_path.clone() + "/" + DB_FILE_NAME;
+        dns_log!(LogLevel::INFO, "open db: {}", conf_clone.db_file);
+        let ret = self.db.open(&conf_clone.db_file);
         if let Err(e) = ret {
             return Err(e);
         }
@@ -299,12 +331,19 @@ impl DataServer {
     }
 
     pub fn set_plugin(&self, plugin: Arc<SmartdnsPlugin>) {
-        *self.plugin.lock().unwrap() = Some(plugin);
+        *self.plugin.lock().unwrap() = Arc::downgrade(&plugin);
     }
 
-    pub fn get_plugin(&self) -> Arc<SmartdnsPlugin> {
-        let plugin = self.plugin.lock().unwrap();
-        Arc::clone(&plugin.as_ref().unwrap())
+    pub fn get_plugin(&self) -> Result<Arc<SmartdnsPlugin>, Box<dyn Error>> {
+        let plugin = match self.plugin.lock() {
+            Ok(plugin) => plugin,
+            Err(_) => return Err("Failed to lock plugin mutex".into()),
+        };
+
+        if let Some(plugin) = plugin.upgrade() {
+            return Ok(plugin);
+        }
+        Err("Plugin is not set".into())
     }
 
     pub fn get_data_server_config(&self) -> DataServerConfig {
@@ -445,7 +484,7 @@ impl DataServer {
     }
 
     pub fn get_free_disk_space(&self) -> u64 {
-        utils::get_free_disk_space(&self.get_data_server_config().data_root)
+        utils::get_free_disk_space(&self.get_data_server_config().db_file)
     }
 
     pub fn get_overview(&self) -> Result<OverviewData, Box<dyn Error + Send>> {
@@ -705,7 +744,15 @@ impl DataServer {
 
     fn stop_data_server(&self) {
         if let Some(tx) = self.notify_tx.as_ref().cloned() {
-            let rt = self.get_plugin().get_runtime();
+            let plugin = match self.get_plugin() {
+                Ok(plugin) => plugin,
+                Err(e) => {
+                    dns_log!(LogLevel::ERROR, "get plugin error: {}", e);
+                    return;
+                }
+            };
+
+            let rt = plugin.get_runtime();
             tokio::task::block_in_place(|| {
                 let _ = rt.block_on(async {
                     let _ = tx.send(()).await;
@@ -722,7 +769,7 @@ impl DataServer {
         F: FnOnce() -> R + Send + 'static,
         R: Send + 'static,
     {
-        let rt = this.get_plugin().get_runtime();
+        let rt = this.get_plugin().unwrap().get_runtime();
 
         let ret = rt.spawn_blocking(move || -> R {
             return func();

+ 0 - 1
plugin/smartdns-ui/src/data_stats.rs

@@ -249,7 +249,6 @@ impl DataStats {
     pub fn init(self: &Arc<Self>) -> Result<(), Box<dyn Error>> {
         dns_log!(LogLevel::DEBUG, "init data stats");
         self.load_status_data()?;
-        dns_log!(LogLevel::DEBUG, "init data stats complete");
         Ok(())
     }
 

+ 59 - 27
plugin/smartdns-ui/src/http_server.rs

@@ -42,6 +42,7 @@ use std::net::SocketAddr;
 use std::path::PathBuf;
 use std::path::{Component, Path};
 use std::sync::MutexGuard;
+use std::sync::Weak;
 use std::sync::{Arc, Mutex};
 use std::time::Duration;
 use std::time::Instant;
@@ -61,7 +62,7 @@ cfg_if::cfg_if! {
 const HTTP_SERVER_DEFAULT_PASSWORD: &str = "password";
 const HTTP_SERVER_DEFAULT_USERNAME: &str = "admin";
 const HTTP_SERVER_DEFAULT_WWW_ROOT: &str = "/usr/share/smartdns/wwwroot";
-const HTTP_SERVER_DEFAULT_IP: &str = "http://0.0.0.0:6080";
+const HTTP_SERVER_DEFAULT_IP: &str = "http://[::]:6080";
 
 #[derive(Clone)]
 pub struct HttpServerConfig {
@@ -145,7 +146,7 @@ impl HttpServerConfig {
 pub struct HttpServerControl {
     http_server: Arc<HttpServer>,
     server_thread: Mutex<Option<JoinHandle<()>>>,
-    plugin: Mutex<Option<Arc<SmartdnsPlugin>>>,
+    plugin: Mutex<Weak<SmartdnsPlugin>>,
 }
 
 #[allow(dead_code)]
@@ -154,17 +155,24 @@ impl HttpServerControl {
         HttpServerControl {
             http_server: Arc::new(HttpServer::new()),
             server_thread: Mutex::new(None),
-            plugin: Mutex::new(None),
+            plugin: Mutex::new(Weak::new()),
         }
     }
 
     pub fn set_plugin(&self, plugin: Arc<SmartdnsPlugin>) {
-        *self.plugin.lock().unwrap() = Some(plugin);
+        *self.plugin.lock().unwrap() = Arc::downgrade(&plugin);
     }
 
-    pub fn get_plugin(&self) -> Arc<SmartdnsPlugin> {
-        let plugin = self.plugin.lock().unwrap();
-        Arc::clone(&plugin.as_ref().unwrap())
+    pub fn get_plugin(&self) -> Result<Arc<SmartdnsPlugin>, Box<dyn Error>> {
+        let plugin = match self.plugin.lock() {
+            Ok(plugin) => plugin,
+            Err(_) => return Err("Failed to lock plugin mutex".into()),
+        };
+
+        if let Some(plugin) = plugin.upgrade() {
+            return Ok(plugin);
+        }
+        Err("Plugin is not set".into())
     }
 
     pub fn get_http_server(&self) -> Arc<HttpServer> {
@@ -180,22 +188,24 @@ impl HttpServerControl {
             return Err(e);
         }
 
-        inner_clone.set_plugin(self.get_plugin());
+        let plugin = self.get_plugin()?;
+        inner_clone.set_plugin(plugin.clone());
 
-        let (tx, rx) = std::sync::mpsc::channel::<i32>();
-        let rt = self.get_plugin().get_runtime();
+        let (tx, rx) = tokio::sync::oneshot::channel::<i32>();
+        let rt = plugin.get_runtime();
 
         let server_thread = rt.spawn(async move {
-            let ret = HttpServer::http_server_loop(inner_clone, &tx).await;
+            let ret = HttpServer::http_server_loop(inner_clone, tx).await;
             if let Err(e) = ret {
-                _ = tx.send(0);
                 dns_log!(LogLevel::ERROR, "http server error: {}", e);
                 Plugin::smartdns_exit(1);
             }
             dns_log!(LogLevel::INFO, "http server exit.");
         });
 
-        rx.recv().unwrap();
+        tokio::task::block_in_place(|| {
+            let _ = rt.block_on(rx);
+        });
 
         *self.server_thread.lock().unwrap() = Some(server_thread);
 
@@ -213,15 +223,23 @@ impl HttpServerControl {
         self.http_server.stop_http_server();
 
         if let Some(server_thread) = server_thread.take() {
-            let rt = self.get_plugin().get_runtime();
+            let plugin = self.get_plugin();
+            if plugin.is_err() {
+                dns_log!(
+                    LogLevel::ERROR,
+                    "get plugin error: {}",
+                    plugin.err().unwrap()
+                );
+                return;
+            }
+            let plugin = plugin.unwrap();
+            let rt = plugin.get_runtime();
             tokio::task::block_in_place(|| {
                 if let Err(e) = rt.block_on(server_thread) {
                     dns_log!(LogLevel::ERROR, "http server stop error: {}", e);
                 }
             });
         }
-
-        *server_thread = None;
     }
 }
 
@@ -252,7 +270,7 @@ pub struct HttpServer {
     local_addr: Mutex<Option<SocketAddr>>,
     mime_map: std::collections::HashMap<&'static str, &'static str>,
     login_attempts: Mutex<(i32, Instant)>,
-    plugin: Mutex<Option<Arc<SmartdnsPlugin>>>,
+    plugin: Mutex<Weak<SmartdnsPlugin>>,
 }
 
 #[allow(dead_code)]
@@ -265,7 +283,7 @@ impl HttpServer {
             api: API::new(),
             local_addr: Mutex::new(None),
             login_attempts: Mutex::new((0, Instant::now())),
-            plugin: Mutex::new(None),
+            plugin: Mutex::new(Weak::new()),
             mime_map: std::collections::HashMap::from([
                 /* text */
                 ("htm", "text/html"),
@@ -319,7 +337,7 @@ impl HttpServer {
         conf.clone()
     }
 
-    pub fn get_conf_mut(&self) -> MutexGuard<HttpServerConfig> {
+    pub fn get_conf_mut(&'_ self) -> MutexGuard<'_, HttpServerConfig> {
         self.conf.lock().unwrap()
     }
 
@@ -374,16 +392,23 @@ impl HttpServer {
 
     fn set_plugin(&self, plugin: Arc<SmartdnsPlugin>) {
         let mut _plugin = self.plugin.lock().unwrap();
-        *_plugin = Some(plugin)
+        *_plugin = Arc::downgrade(&plugin);
     }
 
-    fn get_plugin(&self) -> Arc<SmartdnsPlugin> {
-        let plugin = self.plugin.lock().unwrap();
-        Arc::clone(&plugin.as_ref().unwrap())
+    fn get_plugin(&self) -> Result<Arc<SmartdnsPlugin>, Box<dyn Error>> {
+        let plugin = match self.plugin.lock() {
+            Ok(plugin) => plugin,
+            Err(_) => return Err("Failed to lock plugin mutex".into()),
+        };
+
+        if let Some(plugin) = plugin.upgrade() {
+            return Ok(plugin);
+        }
+        Err("Plugin is not set".into())
     }
 
     pub fn get_data_server(&self) -> Arc<DataServer> {
-        self.get_plugin().get_data_server()
+        self.get_plugin().unwrap().get_data_server()
     }
 
     pub fn get_token_from_header(
@@ -773,7 +798,7 @@ impl HttpServer {
 
     async fn http_server_loop(
         this: Arc<HttpServer>,
-        kickoff_tx: &std::sync::mpsc::Sender<i32>,
+        kickoff_tx: tokio::sync::oneshot::Sender<i32>,
     ) -> Result<(), Box<dyn Error>> {
         let addr: String;
         let mut rx: mpsc::Receiver<()>;
@@ -836,7 +861,7 @@ impl HttpServer {
         *this.local_addr.lock().unwrap() = Some(addr);
         dns_log!(LogLevel::INFO, "http server listen at {}", url);
 
-        _ = kickoff_tx.send(0);
+        let _ = kickoff_tx.send(0);
         loop {
             tokio::select! {
                 _ = rx.recv() => {
@@ -889,7 +914,14 @@ impl HttpServer {
 
     fn stop_http_server(&self) {
         if let Some(tx) = self.notify_tx.as_ref().cloned() {
-            let rt = self.get_plugin().get_runtime();
+            let plugin = match self.get_plugin() {
+                Ok(plugin) => plugin,
+                Err(e) => {
+                    dns_log!(LogLevel::ERROR, "get plugin error: {}", e);
+                    return;
+                }
+            };
+            let rt = plugin.get_runtime();
             tokio::task::block_in_place(|| {
                 let _ = rt.block_on(async {
                     let _ = tx.send(()).await;

+ 1 - 1
plugin/smartdns-ui/src/http_server_api.rs

@@ -1198,7 +1198,7 @@ impl API {
         F: FnOnce() -> R + Send + 'static,
         R: Send + 'static,
     {
-        let rt = this.get_data_server().get_plugin().get_runtime();
+        let rt = this.get_data_server().get_plugin().unwrap().get_runtime();
 
         let ret = rt.spawn_blocking(move || -> R {
             return func();

+ 1 - 1
plugin/smartdns-ui/src/plugin.rs

@@ -130,7 +130,7 @@ impl SmartdnsPlugin {
         }
 
         if let Some(data_dir) = matches.opt_str("data-dir") {
-            data_conf.data_root = data_dir;
+            data_conf.data_path = data_dir;
         }
 
         Ok(())

+ 2 - 1
plugin/smartdns-ui/src/smartdns.rs

@@ -350,7 +350,8 @@ extern "C" fn dns_plugin_init(plugin: *mut smartdns_c::dns_plugin) -> i32 {
             .unwrap()
             .server_init((*plugin_addr).get_args());
         if let Err(e) = ret {
-            dns_log!(LogLevel::ERROR, "server init error: {}", e);
+            dns_log!(LogLevel::ERROR, "{}", e.to_string());
+            dns_log!(LogLevel::ERROR, "server init failed.");
             return -1;
         }
     }

+ 5 - 0
plugin/smartdns-ui/src/utils.rs

@@ -92,3 +92,8 @@ pub fn get_page_size() -> usize {
     // Use libc to get the system page size
     unsafe { libc::sysconf(libc::_SC_PAGESIZE) as usize }
 }
+
+pub fn is_dir_writable(path: &str) -> bool {
+    let path = std::ffi::CString::new(path).unwrap();
+    unsafe { libc::access(path.as_ptr(), libc::W_OK) == 0 }
+}

+ 5 - 0
plugin/smartdns-ui/tests/common/client.rs

@@ -57,6 +57,11 @@ impl TestClient {
             password: password.to_string(),
         });
         let resp = self.client.post(&url).body(body).send()?;
+
+        if resp.status().as_u16() != 200 {
+            return Err(resp.text()?.into());
+        }
+
         let text = resp.text()?;
 
         let token = http_api_msg::api_msg_parse_auth_token(&text)?;

+ 1 - 1
plugin/smartdns-ui/tests/common/server.rs

@@ -300,7 +300,7 @@ impl TestServer {
         self.args.insert(1, self.ip.clone());
 
         self.args.insert(0, "--data-dir".to_string());
-        self.args.insert(1, self.workdir.clone() + "/data.db");
+        self.args.insert(1, self.workdir.clone());
 
         self.args.insert(0, "--www-root".to_string());
         self.www_root = self.workdir.clone() + "/www";

+ 12 - 36
plugin/smartdns-ui/tests/restapi_test.rs

@@ -20,33 +20,20 @@ mod common;
 
 use common::TestDnsRequest;
 use nix::libc::c_char;
-use reqwest;
-use serde_json::json;
 use smartdns_ui::{http_api_msg, http_jwt::JwtClaims, smartdns::LogLevel};
 use std::ffi::CString;
 
-#[tokio::test(flavor = "multi_thread")]
-async fn test_rest_api_login() {
+#[test]
+fn test_rest_api_login() {
     let mut server = common::TestServer::new();
     server.set_log_level(LogLevel::DEBUG);
     assert!(server.start().is_ok());
 
-    let c = reqwest::Client::new();
-    let body = json!({
-        "username": "admin",
-        "password": "password",
-    });
-
-    let res = c
-        .post(server.get_url("/api/auth/login"))
-        .body(body.to_string())
-        .send()
-        .await
-        .unwrap();
-    let code = res.status();
-    let body = res.text().await.unwrap();
+    let mut client = common::TestClient::new(&server.get_host());
+    let res = client.login("admin", "password");
+    assert!(res.is_ok());
+    let body = res.unwrap();
     println!("res: {}", body);
-    assert_eq!(code, 200);
 
     let result = http_api_msg::api_msg_parse_auth_token(&body);
     assert!(result.is_ok());
@@ -92,28 +79,17 @@ fn test_rest_api_logout() {
     assert_eq!(code, 401);
 }
 
-#[tokio::test(flavor = "multi_thread")]
-async fn test_rest_api_login_incorrect() {
+#[test]
+fn test_rest_api_login_incorrect() {
     let mut server = common::TestServer::new();
     server.set_log_level(LogLevel::DEBUG);
     assert!(server.start().is_ok());
 
-    let c = reqwest::Client::new();
-    let body = json!({
-        "username": "admin",
-        "password": "wrongpassword",
-    });
-
-    let res = c
-        .post(server.get_url("/api/auth/login"))
-        .body(body.to_string())
-        .send()
-        .await
-        .unwrap();
-    let code = res.status();
-    let body = res.text().await.unwrap();
+    let mut client = common::TestClient::new(&server.get_host());
+    let res = client.login("admin", "wrongpassword");
+    assert!(!res.is_ok());
+    let body = res.err().unwrap().to_string();
     println!("res: {}", body);
-    assert_eq!(code, 401);
 
     let result = http_api_msg::api_msg_parse_error(&body);
     assert!(result.is_ok());

+ 4 - 2
src/dns_plugin.c

@@ -204,13 +204,15 @@ static int _dns_plugin_load_library(struct dns_plugin *plugin)
 
 	init_func = (dns_plugin_init_func)dlsym(handle, DNS_PLUGIN_INIT_FUNC);
 	if (!init_func) {
-		tlog(TLOG_ERROR, "load plugin %s failed: %s", plugin->file, dlerror());
+		tlog(TLOG_ERROR, "load plugin failed: %s", dlerror());
+		tlog(TLOG_ERROR, "%s is not a valid smartdns plugin, please check 'plugin' option.", plugin->file);
 		goto errout;
 	}
 
 	exit_func = (dns_plugin_exit_func)dlsym(handle, DNS_PLUGIN_EXIT_FUNC);
 	if (!exit_func) {
-		tlog(TLOG_ERROR, "load plugin %s failed: %s", plugin->file, dlerror());
+		tlog(TLOG_ERROR, "load plugin failed: %s", dlerror());
+		tlog(TLOG_ERROR, "%s not a valid smartdns plugin, please check 'plugin' option.", plugin->file);
 		goto errout;
 	}
 

+ 18 - 1
src/lib/conf.c

@@ -475,6 +475,8 @@ static int load_conf_file(const char *file, const struct config_item *items, con
 	char line[MAX_LINE_LEN + MAX_KEY_LEN];
 	char key[MAX_KEY_LEN];
 	char value[MAX_LINE_LEN];
+	int value_begin = 0, value_end = 0;
+	int value_len = 0;
 	int filed_num = 0;
 	int i = 0;
 	int last_item_index = -1;
@@ -541,11 +543,26 @@ static int load_conf_file(const char *file, const struct config_item *items, con
 		is_last_line_wrap = 0;
 		key[0] = '\0';
 		value[0] = '\0';
-		filed_num = sscanf(line, "%63s %4095[^\r\n]s", key, value);
+		filed_num = sscanf(line, "%63s %n%4095[^\r\n]%n", key, &value_begin, value, &value_end);
 		if (filed_num <= 0) {
 			continue;
 		}
 
+		/* remove suffix space of value */
+		value_len = value_end - value_begin;
+		if (value_len < 0) {
+			continue;
+		}
+
+		for (i = value_len - 1; i >= 0; i--) {
+			if (value[i] == ' ' || value[i] == '\t') {
+				value[i] = '\0';
+				value_len--;
+			} else {
+				break;
+			}
+		}
+
 		/* comment, skip */
 		if (key[0] == '#') {
 			continue;

+ 12 - 3
src/smartdns.c

@@ -823,7 +823,12 @@ static int _smartdns_create_datadir(void)
 	}
 
 	mkdir(data_dir, 0750);
-	if (stat(data_dir, &sb) == 0 && sb.st_uid == uid && sb.st_gid == gid && (sb.st_mode & 0700) == 0700) {
+	if (stat(data_dir, &sb) != 0) {
+		tlog(TLOG_DEBUG, "create dir %s failed, %s", data_dir, strerror(errno));
+		return -1;
+	}
+
+	if (sb.st_uid == uid && sb.st_gid == gid && (sb.st_mode & 0700) == 0700) {
 		return 0;
 	}
 
@@ -853,9 +858,13 @@ static int _set_rlimit(void)
 
 static int _smartdns_init_pre(void)
 {
+	int ret = -1;
 	_smartdns_create_logdir();
 	_smartdns_create_cache_dir();
-	_smartdns_create_datadir();
+	ret = _smartdns_create_datadir();
+	if (ret != 0) {
+		tlog(TLOG_DEBUG, "create data dir failed.");
+	}
 
 	_set_rlimit();
 
@@ -1202,7 +1211,7 @@ int smartdns_main(int argc, char *argv[])
 
 	ret = _smartdns_init_pre();
 	if (ret != 0) {
-		fprintf(stderr, "init failed.\n");
+		fprintf(stderr, "smartdns init failed.\n");
 		goto errout;
 	}
 

+ 102 - 0
test/cases/test-server.cc

@@ -451,3 +451,105 @@ nameserver /a.com/a
 	EXPECT_EQ(client.GetAnswer()[0].GetName(), "a.com");
 	EXPECT_EQ(client.GetAnswer()[0].GetData(), "1.2.3.4");
 }
+
+
+TEST_F(Server, bad_block_ip)
+{
+	smartdns::MockServer server_upstream1;
+	smartdns::MockServer server_upstream2;
+	smartdns::MockServer server_upstream3;
+	smartdns::Server server;
+
+	server_upstream1.Start("udp://0.0.0.0:61053", [](struct smartdns::ServerRequestContext *request) {
+		if (request->qtype != DNS_T_A) {
+			return smartdns::SERVER_REQUEST_SOA;
+		}
+		smartdns::MockServer::AddIP(request, request->domain.c_str(), "127.0.0.1", 611);
+		return smartdns::SERVER_REQUEST_OK;
+	});
+
+	server_upstream2.Start("udp://0.0.0.0:61054", [](struct smartdns::ServerRequestContext *request) {
+		if (request->qtype != DNS_T_A) {
+			return smartdns::SERVER_REQUEST_SOA;
+		}
+		smartdns::MockServer::AddIP(request, request->domain.c_str(), "::", 611);
+		return smartdns::SERVER_REQUEST_OK;
+	});
+
+	server_upstream3.Start("udp://0.0.0.0:61055", [](struct smartdns::ServerRequestContext *request) {
+		if (request->qtype != DNS_T_A) {
+			return smartdns::SERVER_REQUEST_SOA;
+		}
+		usleep(100000); 
+		smartdns::MockServer::AddIP(request, request->domain.c_str(), "1.2.3.6", 611);
+		return smartdns::SERVER_REQUEST_OK;
+	});
+
+	server.MockPing(PING_TYPE_ICMP, "1.2.3.4", 128, 10);
+	server.MockPing(PING_TYPE_ICMP, "1.2.3.5", 128, 10);
+	server.MockPing(PING_TYPE_ICMP, "1.2.3.6", 128, 10);
+	server.Start(R"""(bind [::]:60053
+bind-tcp [::]:60053
+server 127.0.0.1:61053 
+server 127.0.0.1:61054 
+server 127.0.0.1:61055
+)""");
+	smartdns::Client client;
+	ASSERT_TRUE(client.Query("a.com", 60053));
+	std::cout << client.GetResult() << std::endl;
+	ASSERT_EQ(client.GetAnswerNum(), 1);
+	EXPECT_EQ(client.GetStatus(), "NOERROR");
+	EXPECT_EQ(client.GetAnswer()[0].GetName(), "a.com");
+	EXPECT_EQ(client.GetAnswer()[0].GetData(), "1.2.3.6");
+}
+
+TEST_F(Server, bad_block_ip_no_check_speed)
+{
+	smartdns::MockServer server_upstream1;
+	smartdns::MockServer server_upstream2;
+	smartdns::MockServer server_upstream3;
+	smartdns::Server server;
+
+	server_upstream1.Start("udp://0.0.0.0:61053", [](struct smartdns::ServerRequestContext *request) {
+		if (request->qtype != DNS_T_A) {
+			return smartdns::SERVER_REQUEST_SOA;
+		}
+		smartdns::MockServer::AddIP(request, request->domain.c_str(), "127.0.0.1", 611);
+		return smartdns::SERVER_REQUEST_OK;
+	});
+
+	server_upstream2.Start("udp://0.0.0.0:61054", [](struct smartdns::ServerRequestContext *request) {
+		if (request->qtype != DNS_T_A) {
+			return smartdns::SERVER_REQUEST_SOA;
+		}
+		smartdns::MockServer::AddIP(request, request->domain.c_str(), "::", 611);
+		return smartdns::SERVER_REQUEST_OK;
+	});
+
+	server_upstream3.Start("udp://0.0.0.0:61055", [](struct smartdns::ServerRequestContext *request) {
+		if (request->qtype != DNS_T_A) {
+			return smartdns::SERVER_REQUEST_SOA;
+		}
+		usleep(100000); 
+		smartdns::MockServer::AddIP(request, request->domain.c_str(), "1.2.3.6", 611);
+		return smartdns::SERVER_REQUEST_OK;
+	});
+
+	server.MockPing(PING_TYPE_ICMP, "1.2.3.4", 128, 10);
+	server.MockPing(PING_TYPE_ICMP, "1.2.3.5", 128, 10);
+	server.MockPing(PING_TYPE_ICMP, "1.2.3.6", 128, 10);
+	server.Start(R"""(bind [::]:60053
+bind-tcp [::]:60053
+server 127.0.0.1:61053 
+server 127.0.0.1:61054 
+server 127.0.0.1:61055
+speed-check-mode none
+)""");
+	smartdns::Client client;
+	ASSERT_TRUE(client.Query("a.com", 60053));
+	std::cout << client.GetResult() << std::endl;
+	ASSERT_EQ(client.GetAnswerNum(), 1);
+	EXPECT_EQ(client.GetStatus(), "NOERROR");
+	EXPECT_EQ(client.GetAnswer()[0].GetName(), "a.com");
+	EXPECT_EQ(client.GetAnswer()[0].GetData(), "1.2.3.6");
+}