From ca589773ded10c7abdd88cae956c0cda3d8630b0 Mon Sep 17 00:00:00 2001 From: Eva Pace Date: Tue, 30 May 2023 13:18:42 -0300 Subject: [PATCH] hostsfile: remove has_content_changed in favor of comparing old and new sections --- hostsfile/src/lib.rs | 109 ++++++++++++++++++------------------------- 1 file changed, 45 insertions(+), 64 deletions(-) diff --git a/hostsfile/src/lib.rs b/hostsfile/src/lib.rs index ed396c1..0cb2323 100644 --- a/hostsfile/src/lib.rs +++ b/hostsfile/src/lib.rs @@ -1,8 +1,8 @@ use std::{ collections::BTreeMap, fmt, - fs::{File, OpenOptions}, - io::{self, BufRead, BufReader, ErrorKind, Read, Write}, + fs::OpenOptions, + io::{self, BufRead, BufReader, ErrorKind, Write}, net::IpAddr, path::{Path, PathBuf}, result, @@ -172,24 +172,6 @@ impl HostsBuilder { Ok(hosts_dir.with_file_name(temp_filename)) } - fn has_content_changed(hosts_path: &Path, new_contents: &[u8]) -> io::Result { - let hosts_file = File::open(hosts_path)?; - - if hosts_file.metadata()?.len() != new_contents.len() as u64 { - return Ok(true); - } - - let hosts_buf = BufReader::new(hosts_file); - - for (b1, b2) in hosts_buf.bytes().zip(new_contents) { - if b1.unwrap() != *b2 { - return Ok(true); - } - } - - Ok(false) - } - /// Inserts a new section to the specified hosts file. If there is a section with the same tag /// name already, it will be replaced with the new list instead. /// @@ -227,9 +209,33 @@ impl HostsBuilder { let begin = lines.iter().position(|line| line.trim() == begin_marker); let end = lines.iter().position(|line| line.trim() == end_marker); + let mut lines_to_insert = vec![]; + if !self.hostname_map.is_empty() { + lines_to_insert.push(begin_marker); + for (ip, hostnames) in &self.hostname_map { + if cfg!(windows) { + // windows only allows one hostname per line + for hostname in hostnames { + lines_to_insert.push(format!("{ip} {hostname}")); + } + } else { + // assume the same format as Unix + lines_to_insert.push(format!("{} {}", ip, hostnames.join(" "))); + } + } + lines_to_insert.push(end_marker); + } + let insert = match (begin, end) { (Some(begin), Some(end)) => { - lines.drain(begin..end + 1); + let old_section: Vec = lines.drain(begin..end + 1).collect(); + + let sections_are_eq = old_section == lines_to_insert; + + if sections_are_eq { + return Ok(false); + } + begin }, (None, None) => { @@ -254,21 +260,12 @@ impl HostsBuilder { for line in &lines[..insert] { writeln!(s, "{line}")?; } - if !self.hostname_map.is_empty() { - writeln!(s, "{begin_marker}")?; - for (ip, hostnames) in &self.hostname_map { - if cfg!(windows) { - // windows only allows one hostname per line - for hostname in hostnames { - writeln!(s, "{ip} {hostname}")?; - } - } else { - // assume the same format as Unix - writeln!(s, "{} {}", ip, hostnames.join(" "))?; - } - } - writeln!(s, "{end_marker}")?; + + // Append hostnames_map section + for line in lines_to_insert { + writeln!(s, "{line}")?; } + for line in &lines[insert..] { writeln!(s, "{line}")?; } @@ -277,26 +274,21 @@ impl HostsBuilder { Err(_) => { Self::write_clobber(hosts_path, &s)?; log::debug!("wrote hosts file with the clobber fallback strategy"); - Ok(true) }, - Ok(has_written) => { - if has_written { - log::debug!("wrote hosts file with the write-and-swap strategy"); - } - Ok(has_written) + _ => { + log::debug!("wrote hosts file with the write-and-swap strategy"); }, - } + }; + + Ok(true) } - fn write_and_swap(temp_path: &Path, hosts_path: &Path, contents: &[u8]) -> io::Result { - if Self::has_content_changed(hosts_path, contents)? { - // Copy the file we plan on modifying so its permissions and metadata are preserved. - std::fs::copy(hosts_path, temp_path)?; - Self::write_clobber(temp_path, contents)?; - std::fs::rename(temp_path, hosts_path)?; - return Ok(true); - } - Ok(false) + fn write_and_swap(temp_path: &Path, hosts_path: &Path, contents: &[u8]) -> io::Result<()> { + // Copy the file we plan on modifying so its permissions and metadata are preserved. + std::fs::copy(hosts_path, temp_path)?; + Self::write_clobber(temp_path, contents)?; + std::fs::rename(temp_path, hosts_path)?; + Ok(()) } fn write_clobber(hosts_path: &Path, contents: &[u8]) -> io::Result<()> { @@ -335,25 +327,14 @@ mod tests { assert!(HostsBuilder::get_temp_path(hosts_path).is_err()); } - #[test] - fn test_has_content_changed() { - let (mut temp_file, temp_path) = tempfile::NamedTempFile::new().unwrap().into_parts(); - temp_file.write_all(b"preexisting\ncontent").unwrap(); - - let new_content = b"preexisting\ncontent\nnew\ncontent"; - assert!(HostsBuilder::has_content_changed(&temp_path, new_content).unwrap()); - - let same_content = b"preexisting\ncontent"; - assert!(!HostsBuilder::has_content_changed(&temp_path, same_content).unwrap()); - } - #[test] fn test_write() { let (mut temp_file, temp_path) = tempfile::NamedTempFile::new().unwrap().into_parts(); temp_file.write_all(b"preexisting\ncontent").unwrap(); let mut builder = HostsBuilder::new("foo"); builder.add_hostname([1, 1, 1, 1].into(), "whatever"); - builder.write_to(&temp_path).unwrap(); + assert!(builder.write_to(&temp_path).unwrap()); + assert!(!builder.write_to(&temp_path).unwrap()); let contents = std::fs::read_to_string(&temp_path).unwrap(); println!("contents: {contents}");