From 0c2eed01a291872425a9f9f6fea5f487105187e3 Mon Sep 17 00:00:00 2001 From: Christoph Herzog Date: Wed, 7 Jun 2023 12:11:12 +0200 Subject: [PATCH] fix: Fix incorrect archive construction in wasmer publish A recent addition introduced a bug that prevented the manifest from being correctly added to the tar archive used during the publish process. Also adds a test. --- Cargo.lock | 1 + lib/registry/Cargo.toml | 3 + lib/registry/src/package/builder.rs | 193 +++++++++++++++++++++------- 3 files changed, 149 insertions(+), 48 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1788c3cb27f..c8ebc060152 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5967,6 +5967,7 @@ dependencies = [ "log", "lzma-rs", "minisign", + "pretty_assertions", "regex", "reqwest", "rpassword", diff --git a/lib/registry/Cargo.toml b/lib/registry/Cargo.toml index 747cc3b90bf..17462fac435 100644 --- a/lib/registry/Cargo.toml +++ b/lib/registry/Cargo.toml @@ -46,3 +46,6 @@ wasmparser = { version = "0.51.4", optional = true } rpassword = { version = "7.2.0", optional = true } minisign = { version = "0.7.2", optional = true } reqwest = { version = "0.11.12", default-features = false, features = ["blocking", "multipart", "json", "stream"] } + +[dev-dependencies] +pretty_assertions = "1.3.0" diff --git a/lib/registry/src/package/builder.rs b/lib/registry/src/package/builder.rs index 8cea017083e..f6db99b4b55 100644 --- a/lib/registry/src/package/builder.rs +++ b/lib/registry/src/package/builder.rs @@ -59,8 +59,6 @@ enum PackageBuildError { impl Publish { /// Executes `wasmer publish` pub fn execute(&self) -> Result<(), anyhow::Error> { - let mut builder = Builder::new(Vec::new()); - let input_path = match self.package_path.as_ref() { Some(s) => std::env::current_dir()?.join(s), None => std::env::current_dir()?, @@ -118,6 +116,9 @@ impl Publish { manifest.package.version = version.clone(); } + let archive_dir = tempfile::TempDir::new()?; + let archive_meta = construct_tar_gz(archive_dir.path(), &manifest, &manifest_path)?; + let registry = match self.registry.as_deref() { Some(s) => crate::format_graphql(s), None => { @@ -130,30 +131,11 @@ impl Publish { }; if !self.no_validate { - validate::validate_directory(&manifest, ®istry, manifest_dir.clone())?; + validate::validate_directory(&manifest, ®istry, manifest_dir)?; } - builder.append_path_with_name(&manifest_dir, PACKAGE_TOML_FALLBACK_NAME)?; - - let manifest_string = toml::to_string(&manifest)?; - - let (readme, license) = construct_tar_gz(&mut builder, &manifest, &manifest_dir)?; - - builder - .finish() - .map_err(|e| anyhow::anyhow!("failed to finish .tar.gz builder: {e}"))?; - let tar_archive_data = builder.into_inner().expect("tar archive was not finalized"); - let archive_name = "package.tar.gz".to_string(); - let archive_dir = tempfile::TempDir::new()?; - let archive_dir_path: &std::path::Path = archive_dir.as_ref(); - fs::create_dir(archive_dir_path.join("wapm_package"))?; - let archive_path = archive_dir_path.join("wapm_package").join(&archive_name); - let mut compressed_archive = fs::File::create(&archive_path).unwrap(); - let mut gz_enc = GzEncoder::new(&mut compressed_archive, Compression::best()); - - gz_enc.write_all(&tar_archive_data).unwrap(); - let _compressed_archive = gz_enc.finish().unwrap(); - let mut compressed_archive_reader = fs::File::open(&archive_path)?; + let archive_path = &archive_meta.archive_path; + let mut compressed_archive_reader = fs::File::open(archive_path)?; let maybe_signature_data = sign_compressed_archive(&mut compressed_archive_reader)?; let archived_data_size = archive_path.metadata()?.len(); @@ -169,6 +151,9 @@ impl Publish { manifest.package.name, manifest.package.version ); + let path = archive_dir.into_path(); + eprintln!("Archive persisted at: {}", path.display()); + log::info!( "Publish succeeded, but package was not published because it was run in dry-run mode" ); @@ -180,11 +165,17 @@ impl Publish { Some(registry), self.token.clone(), &manifest.package, - &manifest_string, - &license, - &readme, - &archive_name, - &archive_path, + &archive_meta.manifest_toml, + &archive_meta.license, + &archive_meta.readme, + &archive_meta + .archive_path + .file_name() + .unwrap() + .to_str() + .unwrap() + .to_string(), + archive_path, &maybe_signature_data, archived_data_size, self.quiet, @@ -192,45 +183,65 @@ impl Publish { } } +struct ConstructedPackageArchive { + manifest_toml: String, + readme: Option, + license: Option, + archive_path: PathBuf, +} + fn construct_tar_gz( - builder: &mut tar::Builder>, + archive_dir: &Path, manifest: &wasmer_toml::Manifest, - cwd: &Path, -) -> Result<(Option, Option), anyhow::Error> { + manifest_path: &Path, +) -> Result { + // This is an assert instead of returned error because this is a programmer error. + debug_assert!(manifest_path.is_file(), "manifest path is not a file"); + + let manifest_dir = manifest_path + .parent() + .context("manifest path has no parent directory")?; + + let mut builder = Builder::new(Vec::new()); + builder.append_path_with_name(manifest_path, PACKAGE_TOML_FALLBACK_NAME)?; + + let manifest_string = toml::to_string(&manifest)?; + let package = &manifest.package; let modules = manifest.module.as_deref().unwrap_or_default(); let readme = match package.readme.as_ref() { None => None, Some(s) => { - let path = append_path_to_tar_gz(builder, &manifest.base_directory_path, s).map_err( - |(p, e)| PackageBuildError::ErrorBuildingPackage(format!("{}", p.display()), e), - )?; - fs::read_to_string(path).ok() + let path = append_path_to_tar_gz(&mut builder, &manifest.base_directory_path, s) + .map_err(|(p, e)| { + PackageBuildError::ErrorBuildingPackage(format!("{}", p.display()), e) + })?; + Some(std::fs::read_to_string(path)?) } }; - let license_file = match package.license_file.as_ref() { + let license = match package.license_file.as_ref() { None => None, Some(s) => { - let path = append_path_to_tar_gz(builder, &manifest.base_directory_path, s).map_err( - |(p, e)| PackageBuildError::ErrorBuildingPackage(format!("{}", p.display()), e), - )?; - fs::read_to_string(path).ok() + let path = append_path_to_tar_gz(&mut builder, &manifest.base_directory_path, s) + .map_err(|(p, e)| { + PackageBuildError::ErrorBuildingPackage(format!("{}", p.display()), e) + })?; + Some(std::fs::read_to_string(path)?) } }; for module in modules { - append_path_to_tar_gz(builder, &manifest.base_directory_path, &module.source).map_err( - |(normalized_path, _)| PackageBuildError::SourceMustBeFile { + append_path_to_tar_gz(&mut builder, &manifest.base_directory_path, &module.source) + .map_err(|(normalized_path, _)| PackageBuildError::SourceMustBeFile { module: module.name.clone(), path: normalized_path, - }, - )?; + })?; if let Some(bindings) = &module.bindings { for path in bindings.referenced_files(&manifest.base_directory_path)? { - append_path_to_tar_gz(builder, &manifest.base_directory_path, &path).map_err( + append_path_to_tar_gz(&mut builder, &manifest.base_directory_path, &path).map_err( |(normalized_path, _)| PackageBuildError::MissingBindings { module: module.name.clone(), path: normalized_path, @@ -243,7 +254,7 @@ fn construct_tar_gz( // bundle the package filesystem let default = indexmap::IndexMap::default(); for (_alias, path) in manifest.fs.as_ref().unwrap_or(&default).iter() { - let normalized_path = normalize_path(cwd, path); + let normalized_path = normalize_path(manifest_dir, path); let path_metadata = normalized_path.metadata().map_err(|_| { PackageBuildError::MissingManifestFsPath(normalized_path.to_string_lossy().to_string()) })?; @@ -260,7 +271,25 @@ fn construct_tar_gz( })?; } - Ok((readme, license_file)) + builder + .finish() + .map_err(|e| anyhow::anyhow!("failed to finish .tar.gz builder: {e}"))?; + let tar_archive_data = builder.into_inner().expect("tar archive was not finalized"); + let archive_name = "package.tar.gz".to_string(); + fs::create_dir(archive_dir.join("wapm_package"))?; + let archive_path = archive_dir.join("wapm_package").join(archive_name); + let mut compressed_archive = fs::File::create(&archive_path).unwrap(); + let mut gz_enc = GzEncoder::new(&mut compressed_archive, Compression::best()); + + gz_enc.write_all(&tar_archive_data).unwrap(); + let _compressed_archive = gz_enc.finish().unwrap(); + + Ok(ConstructedPackageArchive { + manifest_toml: manifest_string, + archive_path, + readme, + license, + }) } fn append_path_to_tar_gz( @@ -689,3 +718,71 @@ mod validate { } } } + +#[cfg(test)] +mod tests { + use std::io::Read; + + use super::*; + + #[test] + fn test_construct_package_tar_gz() { + let manifest_str = r#"[package] +name = "wasmer-tests/wcgi-always-panic" +version = "0.6.0" +description = "wasmer-tests/wcgi-always-panic website" + +[[module]] +name = "wcgi-always-panic" +source = "module.wasm" +abi = "wasi" + +[[command]] +name = "wcgi" +module = "wcgi-always-panic" +runner = "https://webc.org/runner/wcgi" +"#; + + let archive_dir = tempfile::tempdir().unwrap(); + + let manifest_dir = tempfile::tempdir().unwrap(); + + let mp = manifest_dir.path(); + let manifest_path = mp.join("wasmer.toml"); + + std::fs::write(&manifest_path, manifest_str).unwrap(); + std::fs::write(mp.join("module.wasm"), "()").unwrap(); + + let mut manifest = wasmer_toml::Manifest::parse(&manifest_str).unwrap(); + manifest.base_directory_path = manifest_dir.path().to_owned(); + + let meta = construct_tar_gz(&archive_dir.path(), &manifest, &manifest_path).unwrap(); + + let mut data = std::io::Cursor::new(std::fs::read(&meta.archive_path).unwrap()); + + let gz = flate2::read::GzDecoder::new(&mut data); + let mut archive = tar::Archive::new(gz); + + let map = archive + .entries() + .unwrap() + .map(|res| { + let mut entry = res.unwrap(); + let name = entry.path().unwrap().to_str().unwrap().to_string(); + let mut contents = String::new(); + entry.read_to_string(&mut contents).unwrap(); + eprintln!("{name}:\n{contents}\n\n"); + (name, contents) + }) + .collect::>(); + + let expected: std::collections::HashMap = [ + ("wapm.toml".to_string(), manifest_str.to_string()), + ("module.wasm".to_string(), "()".to_string()), + ] + .into_iter() + .collect(); + + pretty_assertions::assert_eq!(map, expected); + } +}