Fix hosts restore semantics and cleanup fallback

Author: fjh1997

src/gui.rs

@@ -20,6 +20,7 @@ use tray_icon::{MouseButton, MouseButtonState, TrayIconBuilder, TrayIconEvent};
 
 use crate::branding;
 use crate::config::AppConfig;
+use crate::hosts::validate_hosts_backup_file;
 use crate::hosts_store::{BackupState, backup_state};
 #[cfg(target_os = "windows")]
 use crate::paths::AppPaths;
@@ -1340,7 +1341,16 @@ fn service_state_changed(before: &ServiceState, after: &ServiceState) -> bool {
 
 fn load_hosts_backup_state(config_path: &Path) -> BackupState {
     service::resolve_paths(Some(config_path.to_path_buf()))
-        .map(|paths| backup_state(&paths))
+        .map(|paths| match backup_state(&paths) {
+            BackupState::Ready => {
+                if validate_hosts_backup_file(&paths).is_ok() {
+                    BackupState::Ready
+                } else {
+                    BackupState::Inconsistent
+                }
+            }
+            state => state,
+        })
         .unwrap_or(BackupState::Missing)
 }
 

src/hosts.rs

@@ -21,9 +21,10 @@ pub fn apply_hosts(config: &AppConfig, paths: &AppPaths) -> Result<()> {
     #[cfg(not(target_os = "android"))]
     {
         let path = hosts_path();
-        ensure_hosts_backup(paths, &path)?;
         let original = fs::read_to_string(&path)
             .with_context(|| format!("failed to read hosts file {}", path.display()))?;
+        let backup_baseline = backup_baseline_content(&original);
+        ensure_hosts_backup(paths, &path, &backup_baseline)?;
         let content = render_managed_hosts(&original, config);
 
         if content == original {
@@ -68,7 +69,11 @@ pub fn backup_hosts_file(paths: &AppPaths) -> Result<()> {
 
     #[cfg(not(target_os = "android"))]
     {
-        ensure_hosts_backup(paths, &hosts_path())
+        let path = hosts_path();
+        let original = fs::read_to_string(&path)
+            .with_context(|| format!("failed to read hosts file {}", path.display()))?;
+        let backup_baseline = backup_baseline_content(&original);
+        ensure_hosts_backup(paths, &path, &backup_baseline)
     }
 }
 
@@ -126,6 +131,11 @@ fn render_managed_hosts(original: &str, config: &AppConfig) -> String {
     content
 }
 
+#[cfg(not(target_os = "android"))]
+fn backup_baseline_content(original: &str) -> String {
+    strip_managed_block(original)
+}
+
 #[cfg(target_os = "android")]
 fn apply_android_hosts(_config: &AppConfig, paths: &AppPaths) -> Result<()> {
     let marker_path = android_hosts_marker_path(paths);
@@ -235,7 +245,7 @@ fn hosts_path() -> PathBuf {
 
 #[cfg(test)]
 mod tests {
-    use super::{render_managed_hosts, strip_managed_block};
+    use super::{backup_baseline_content, render_managed_hosts, strip_managed_block};
     use crate::config::AppConfig;
 
     #[test]
@@ -293,4 +303,19 @@ mod tests {
         let stripped = strip_managed_block(&content);
         assert_eq!(stripped, content);
     }
+
+    #[test]
+    fn backup_baseline_strips_existing_managed_block() {
+        let content = [
+            "127.0.0.1 localhost",
+            "# >>> linuxdo-accelerator >>>",
+            "127.211.73.84 linux.do",
+            "# <<< linuxdo-accelerator <<<",
+            "1.1.1.1 example.com",
+        ]
+        .join("\n");
+
+        let backup = backup_baseline_content(&content);
+        assert_eq!(backup, "127.0.0.1 localhost\n1.1.1.1 example.com");
+    }
 }

src/hosts_store.rs

@@ -43,7 +43,11 @@ pub(crate) enum BackupState {
     Inconsistent,
 }
 
-pub(crate) fn ensure_hosts_backup(paths: &AppPaths, source_path: &Path) -> Result<()> {
+pub(crate) fn ensure_hosts_backup(
+    paths: &AppPaths,
+    source_path: &Path,
+    backup_content: &str,
+) -> Result<()> {
     match backup_state(paths) {
         BackupState::Ready => {
             validate_hosts_backup(paths, source_path)?;
@@ -59,8 +63,6 @@ pub(crate) fn ensure_hosts_backup(paths: &AppPaths, source_path: &Path) -> Resul
         BackupState::Missing => {}
     }
 
-    let original = fs::read_to_string(source_path)
-        .with_context(|| format!("failed to read hosts file {}", source_path.display()))?;
     let attributes = snapshot_file_attributes(source_path)?;
     let meta = HostsBackupMeta {
         source_path: source_path.display().to_string(),
@@ -75,7 +77,7 @@ pub(crate) fn ensure_hosts_backup(paths: &AppPaths, source_path: &Path) -> Resul
 
     cleanup_temp_file(&backup_temp_path)?;
     cleanup_temp_file(&meta_temp_path)?;
-    write_temp_file(&backup_temp_path, &original)?;
+    write_temp_file(&backup_temp_path, backup_content)?;
     write_temp_file(
         &meta_temp_path,
         std::str::from_utf8(&serialized).context("failed to encode hosts backup metadata")?,
@@ -531,10 +533,10 @@ mod tests {
         let source_path = test_dir.join("hosts");
 
         std::fs::write(&source_path, "first").unwrap();
-        ensure_hosts_backup(&paths, &source_path).unwrap();
+        ensure_hosts_backup(&paths, &source_path, "first").unwrap();
 
         std::fs::write(&source_path, "second").unwrap();
-        ensure_hosts_backup(&paths, &source_path).unwrap();
+        ensure_hosts_backup(&paths, &source_path, "second").unwrap();
 
         assert_eq!(
             std::fs::read_to_string(&paths.hosts_backup_path).unwrap(),
@@ -571,7 +573,7 @@ mod tests {
         std::fs::create_dir_all(&paths.runtime_dir).unwrap();
         let source_path = test_dir.join("hosts");
         std::fs::write(&source_path, "original").unwrap();
-        ensure_hosts_backup(&paths, &source_path).unwrap();
+        ensure_hosts_backup(&paths, &source_path, "original").unwrap();
 
         let mut meta = load_backup_meta(&paths).unwrap();
         meta.source_path = r"C:\different\hosts".to_string();

src/service.rs

@@ -173,9 +173,10 @@ pub fn helper_stop(config_path: Option<PathBuf>) -> Result<()> {
     let result = (|| -> Result<()> {
         let config = AppConfig::load_or_create(&paths.config_path)?;
         ensure_elevated(&config, true)?;
-        terminate_running_service(&paths)?;
-
         let mut issues = Vec::new();
+        if let Some(issue) = terminate_running_service(&paths)? {
+            issues.push(issue);
+        }
         if let Err(error) = remove_hosts(&paths) {
             issues.push(format!("failed to clean managed hosts block: {error:#}"));
         }
@@ -189,7 +190,7 @@ pub fn helper_stop(config_path: Option<PathBuf>) -> Result<()> {
         let status_message = if issues.is_empty() {
             "加速已停止".to_string()
         } else {
-            "加速服务已停止，但存在残留清理问题".to_string()
+            "已执行停止请求，但存在残留清理问题".to_string()
         };
         if let Err(error) = state::mark_stopped(&paths, &status_message) {
             issues.push(format!("failed to update service state: {error:#}"));
@@ -214,9 +215,10 @@ pub fn cleanup(config_path: Option<PathBuf>) -> Result<()> {
     let result = (|| -> Result<()> {
         let config = AppConfig::load_or_create(&paths.config_path)?;
         ensure_elevated(&config, true)?;
-        terminate_running_service(&paths)?;
-
         let mut issues = Vec::new();
+        if let Some(issue) = terminate_running_service(&paths)? {
+            issues.push(issue);
+        }
         let (hosts_message, hosts_warning) = cleanup_hosts_state(&paths);
         if let Some(warning) = hosts_warning {
             issues.push(warning);
@@ -366,14 +368,22 @@ fn ensure_service_stopped_for_hosts_change(paths: &AppPaths, command_name: &str)
     Ok(())
 }
 
-fn terminate_running_service(paths: &AppPaths) -> Result<()> {
+fn terminate_running_service(paths: &AppPaths) -> Result<Option<String>> {
     if let Some(pid) = state::read_pid(paths)? {
         if is_process_running(pid) {
-            terminate_process(pid)?;
-            wait_until_stopped(pid, Duration::from_secs(5))?;
+            if let Err(error) = terminate_process(pid) {
+                return Ok(Some(format!(
+                    "failed to terminate running service pid {pid}: {error:#}"
+                )));
+            }
+            if let Err(error) = wait_until_stopped(pid, Duration::from_secs(5)) {
+                return Ok(Some(format!(
+                    "running service pid {pid} did not stop cleanly: {error:#}"
+                )));
+            }
         }
     }
-    Ok(())
+    Ok(None)
 }
 
 fn cleanup_hosts_state(paths: &AppPaths) -> (String, Option<String>) {