diff --git a/chain/src/txhashset/txhashset.rs b/chain/src/txhashset/txhashset.rs index e5efd9e3cd..83eba6967d 100644 --- a/chain/src/txhashset/txhashset.rs +++ b/chain/src/txhashset/txhashset.rs @@ -1522,7 +1522,7 @@ pub fn zip_write( // We expect to see *exactly* the paths listed above. // No attempt is made to be permissive or forgiving with "alternative" paths. // These are the *only* files we will attempt to extract from the zip file. - // If any of these are missing the zip extraction will immediately fail. + // If any of these are missing we will attempt to continue as some are potentially optional. zip::extract_files(txhashset_data, &txhashset_path, files)?; Ok(()) } diff --git a/chain/tests/test_txhashset.rs b/chain/tests/test_txhashset.rs index b45ef737ae..b93a98adf0 100644 --- a/chain/tests/test_txhashset.rs +++ b/chain/tests/test_txhashset.rs @@ -55,94 +55,65 @@ fn test_unexpected_zip() { Path::new(&db_root).join(format!("txhashset_zip_{}", head.hash().to_string())), ); // Then add strange files in the original txhashset folder - write_file(db_root.clone()); + File::create(&Path::new(&db_root).join("txhashset").join("badfile")) + .expect("problem creating a file"); + File::create( + &Path::new(&db_root) + .join("txhashset") + .join("output") + .join("badfile"), + ) + .expect("problem creating a file"); + + let files = file::list_files(&Path::new(&db_root).join("txhashset")); + let expected_files: Vec<_> = vec![ + "badfile", + "kernel/pmmr_data.bin", + "kernel/pmmr_hash.bin", + "kernel/pmmr_size.bin", + "output/badfile", + "output/pmmr_data.bin", + "output/pmmr_hash.bin", + "rangeproof/pmmr_data.bin", + "rangeproof/pmmr_hash.bin", + ]; + assert_eq!( + files, + expected_files + .iter() + .map(|x| PathBuf::from(x)) + .collect::>() + ); + assert!(txhashset::zip_read(db_root.clone(), &head).is_ok()); - // Check that the temp dir dos not contains the strange files let txhashset_zip_path = Path::new(&db_root).join(format!("txhashset_zip_{}", head.hash().to_string())); - assert!(txhashset_contains_expected_files( - format!("txhashset_zip_{}", head.hash().to_string()), - txhashset_zip_path.clone() - )); let _ = fs::remove_dir_all( Path::new(&db_root).join(format!("txhashset_zip_{}", head.hash().to_string())), ); - let zip_file = File::open(zip_path).unwrap(); - assert!(txhashset::zip_write(PathBuf::from(db_root.clone()), zip_file, &head).is_ok()); - // Check that the txhashset dir dos not contains the strange files - let txhashset_path = Path::new(&db_root).join("txhashset"); - assert!(txhashset_contains_expected_files( - "txhashset".to_string(), - txhashset_path.clone() - )); let _ = fs::remove_dir_all(Path::new(&db_root).join("txhashset")); + assert!(txhashset::zip_write(PathBuf::from(db_root.clone()), zip_file, &head).is_ok()); + + // Check that the new txhashset dir contains *only* the expected files + // No "badfiles" and no "size" file. + let files = file::list_files(&Path::new(&db_root).join("txhashset")); + let expected_files: Vec<_> = vec![ + "kernel/pmmr_data.bin", + "kernel/pmmr_hash.bin", + "output/pmmr_data.bin", + "output/pmmr_hash.bin", + "rangeproof/pmmr_data.bin", + "rangeproof/pmmr_hash.bin", + ]; + assert_eq!( + files, + expected_files + .iter() + .map(|x| PathBuf::from(x)) + .collect::>() + ); } // Cleanup chain directory clean_output_dir(&db_root); } - -fn write_file(db_root: String) { - OpenOptions::new() - .create(true) - .write(true) - .open( - Path::new(&db_root) - .join("txhashset") - .join("kernel") - .join("strange0"), - ) - .unwrap(); - OpenOptions::new() - .create(true) - .write(true) - .open(Path::new(&db_root).join("txhashset").join("strange1")) - .unwrap(); - fs::create_dir(Path::new(&db_root).join("txhashset").join("strange_dir")).unwrap(); - OpenOptions::new() - .create(true) - .write(true) - .open( - Path::new(&db_root) - .join("txhashset") - .join("strange_dir") - .join("strange2"), - ) - .unwrap(); - fs::create_dir( - Path::new(&db_root) - .join("txhashset") - .join("strange_dir") - .join("strange_subdir"), - ) - .unwrap(); - OpenOptions::new() - .create(true) - .write(true) - .open( - Path::new(&db_root) - .join("txhashset") - .join("strange_dir") - .join("strange_subdir") - .join("strange3"), - ) - .unwrap(); -} - -fn txhashset_contains_expected_files(dirname: String, path_buf: PathBuf) -> bool { - let list_zip_files = file::list_files(path_buf.into_os_string().into_string().unwrap()); - let zip_files_hashset: HashSet<_> = HashSet::from_iter(list_zip_files.iter().cloned()); - let expected_files = vec![ - dirname, - "output".to_string(), - "rangeproof".to_string(), - "kernel".to_string(), - "pmmr_hash.bin".to_string(), - "pmmr_data.bin".to_string(), - ]; - let expected_files_hashset = HashSet::from_iter(expected_files.iter().cloned()); - let intersection: HashSet<_> = zip_files_hashset - .difference(&expected_files_hashset) - .collect(); - intersection.is_empty() -} diff --git a/util/src/file.rs b/util/src/file.rs index fa331e6799..8ab4647cb3 100644 --- a/util/src/file.rs +++ b/util/src/file.rs @@ -44,18 +44,15 @@ pub fn copy_dir_to(src: &Path, dst: &Path) -> io::Result { } /// List directory -pub fn list_files(path: String) -> Vec { - let mut files_vec: Vec = vec![]; - for entry in WalkDir::new(Path::new(&path)) +pub fn list_files(path: &Path) -> Vec { + WalkDir::new(path) + .sort_by(|a, b| a.path().cmp(b.path())) + .min_depth(1) .into_iter() - .filter_map(|e| e.ok()) - { - match entry.file_name().to_str() { - Some(path_str) => files_vec.push(path_str.to_string()), - None => println!("Could not read optional type"), - } - } - return files_vec; + .filter_map(|x| x.ok()) + .filter(|x| x.file_type().is_file()) + .filter_map(|x| x.path().strip_prefix(path).map(|x| x.to_path_buf()).ok()) + .collect() } fn copy_to(src: &Path, src_type: &fs::FileType, dst: &Path) -> io::Result { diff --git a/util/src/zip.rs b/util/src/zip.rs index cd4fcce520..e7e5502a5b 100644 --- a/util/src/zip.rs +++ b/util/src/zip.rs @@ -34,13 +34,14 @@ pub fn create_zip(dst_file: &File, src_dir: &Path, files: Vec) -> io::R .unix_permissions(0o644); for x in &files { - writer.get_mut().start_file_from_path(x, options)?; let file_path = src_dir.join(x); - info!("compress: {:?} -> {:?}", file_path, x); - let file = File::open(file_path)?; - io::copy(&mut BufReader::new(file), &mut writer)?; - // Flush the BufWriter after each file so we start then next one correctly. - writer.flush()?; + if let Ok(file) = File::open(file_path.clone()) { + info!("compress: {:?} -> {:?}", file_path, x); + writer.get_mut().start_file_from_path(x, options)?; + io::copy(&mut BufReader::new(file), &mut writer)?; + // Flush the BufWriter after each file so we start then next one correctly. + writer.flush()?; + } } writer.get_mut().finish()?; @@ -55,24 +56,23 @@ pub fn extract_files(from_archive: File, dest: &Path, files: Vec) -> io let res = thread::spawn(move || { let mut archive = zip_rs::ZipArchive::new(from_archive).expect("archive file exists"); for x in files { - let file = archive - .by_name(x.to_str().expect("valid path")) - .expect("file exists in archive"); - let path = dest.join(file.sanitized_name()); - let parent_dir = path.parent().expect("valid parent dir"); - fs::create_dir_all(&parent_dir).expect("create parent dir"); - let outfile = fs::File::create(&path).expect("file created"); - io::copy(&mut BufReader::new(file), &mut BufWriter::new(outfile)) - .expect("write to file"); + if let Ok(file) = archive.by_name(x.to_str().expect("valid path")) { + let path = dest.join(file.sanitized_name()); + let parent_dir = path.parent().expect("valid parent dir"); + fs::create_dir_all(&parent_dir).expect("create parent dir"); + let outfile = fs::File::create(&path).expect("file created"); + io::copy(&mut BufReader::new(file), &mut BufWriter::new(outfile)) + .expect("write to file"); - info!("extract_files: {:?} -> {:?}", x, path); + info!("extract_files: {:?} -> {:?}", x, path); - // Set file permissions to "644" (Unix only). - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - let mode = PermissionsExt::from_mode(0o644); - fs::set_permissions(&path, mode).expect("set file permissions"); + // Set file permissions to "644" (Unix only). + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = PermissionsExt::from_mode(0o644); + fs::set_permissions(&path, mode).expect("set file permissions"); + } } } }) diff --git a/util/tests/file.rs b/util/tests/file.rs index bb3d7185bf..456cec379c 100644 --- a/util/tests/file.rs +++ b/util/tests/file.rs @@ -28,11 +28,9 @@ fn copy_dir() { let original_path = Path::new("./target/tmp2/original"); let copy_path = Path::new("./target/tmp2/copy"); file::copy_dir_to(original_path, copy_path).unwrap(); - let original_files = file::list_files("./target/tmp2/original".to_string()); - let copied_files = file::list_files("./target/tmp2/copy".to_string()); - for i in 1..5 { - assert_eq!(copied_files[i], original_files[i]); - } + let original_files = file::list_files(&Path::new("./target/tmp2/original")); + let copied_files = file::list_files(&Path::new("./target/tmp2/copy")); + assert_eq!(original_files, copied_files); fs::remove_dir_all(root).unwrap(); }