diff --git a/crates/puffin-cli/tests/pip_compile.rs b/crates/puffin-cli/tests/pip_compile.rs index bc5e0a899..545130f32 100644 --- a/crates/puffin-cli/tests/pip_compile.rs +++ b/crates/puffin-cli/tests/pip_compile.rs @@ -444,6 +444,59 @@ optional-dependencies.foo = [ Ok(()) } +/// Request an extra with a name that does not conform to the specification. +#[test] +fn invalid_extra_name() -> Result<()> { + let temp_dir = assert_fs::TempDir::new()?; + let cache_dir = assert_fs::TempDir::new()?; + let venv = temp_dir.child(".venv"); + + Command::new(get_cargo_bin(BIN_NAME)) + .arg("venv") + .arg(venv.as_os_str()) + .arg("--cache-dir") + .arg(cache_dir.path()) + .current_dir(&temp_dir) + .assert() + .success(); + venv.assert(predicates::path::is_dir()); + + let pyproject_toml = temp_dir.child("pyproject.toml"); + pyproject_toml.touch()?; + pyproject_toml.write_str( + r#"[build-system] +requires = ["setuptools", "wheel"] + +[project] +name = "project" +dependencies = [] +optional-dependencies.foo = [ + "django==5.0b1", +] +"#, + )?; + + insta::with_settings!({ + filters => vec![ + (r"\d+(ms|s)", "[TIME]"), + (r"# .* pip-compile", "# [BIN_PATH] pip-compile"), + (r"--cache-dir .*", "--cache-dir [CACHE_DIR]"), + ] + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip-compile") + .arg("pyproject.toml") + .arg("--extra") + .arg("invalid name!") + .arg("--cache-dir") + .arg(cache_dir.path()) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir)); + }); + + Ok(()) +} + /// Resolve a specific Flask wheel via a URL dependency. #[test] fn compile_wheel_url_dependency() -> Result<()> { diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__invalid_extra_name.snap b/crates/puffin-cli/tests/snapshots/pip_compile__invalid_extra_name.snap new file mode 100644 index 000000000..8546596bb --- /dev/null +++ b/crates/puffin-cli/tests/snapshots/pip_compile__invalid_extra_name.snap @@ -0,0 +1,23 @@ +--- +source: crates/puffin-cli/tests/pip_compile.rs +info: + program: puffin + args: + - pip-compile + - pyproject.toml + - "--extra" + - invalid name! + - "--cache-dir" + - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpXIuamZ + env: + VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp0LGEEX/.venv +--- +success: false +exit_code: 2 +----- stdout ----- + +----- stderr ----- +error: invalid value 'invalid name!' for '--extra ': Extra names must start and end with a letter or digit and may only contain -, _, ., and alphanumeric characters + +For more information, try '--help'. + diff --git a/crates/puffin-package/Cargo.toml b/crates/puffin-package/Cargo.toml index a2c7db8e3..4b796bb59 100644 --- a/crates/puffin-package/Cargo.toml +++ b/crates/puffin-package/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" pep440_rs = { path = "../pep440-rs", features = ["serde"] } pep508_rs = { path = "../pep508-rs", features = ["serde"] } +anyhow = { workspace = true } fs-err = { workspace = true } mailparse = { workspace = true } memchr = { workspace = true } @@ -19,7 +20,6 @@ tracing.workspace = true unscanny = { workspace = true } [dev-dependencies] -anyhow = { version = "1.0.75" } indoc = { version = "2.0.4" } insta = { version = "1.33.0" } serde_json = { version = "1.0.107" } diff --git a/crates/puffin-package/src/extra_name.rs b/crates/puffin-package/src/extra_name.rs index 9d128f476..18ba89f6d 100644 --- a/crates/puffin-package/src/extra_name.rs +++ b/crates/puffin-package/src/extra_name.rs @@ -1,6 +1,8 @@ use std::fmt; use std::fmt::{Display, Formatter}; +use std::str::FromStr; +use anyhow::{anyhow, Error, Result}; use once_cell::sync::Lazy; use regex::Regex; @@ -14,19 +16,36 @@ impl Display for ExtraName { } static NAME_NORMALIZE: Lazy = Lazy::new(|| Regex::new(r"[-_.]+").unwrap()); +static NAME_VALIDATE: Lazy = + Lazy::new(|| Regex::new(r"(?i)^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$").unwrap()); +/// An extra dependency group name. +/// +/// See: +/// - +/// - impl ExtraName { - /// Collapses any run of the characters `-`, `_` and `.` down to a single `-`. - /// Ex) "---", ".", and "__" all get converted to just "." + /// Create a normalized extra name without validation. /// - /// See: - /// + /// Converts the name to lowercase and collapses any run of the characters `-`, `_` and `.` + /// down to a single `-`, e.g., `---`, `.`, and `__` all get converted to just `-`. pub fn normalize(name: impl AsRef) -> Self { // TODO(charlie): Avoid allocating in the common case (when no normalization is required). let mut normalized = NAME_NORMALIZE.replace_all(name.as_ref(), "-").to_string(); normalized.make_ascii_lowercase(); Self(normalized) } + + /// Create a validated, normalized extra name. + pub fn validate(name: impl AsRef) -> Result { + if NAME_VALIDATE.is_match(name.as_ref()) { + Ok(Self::normalize(name)) + } else { + Err(anyhow!( + "Extra names must start and end with a letter or digit and may only contain -, _, ., and alphanumeric characters" + )) + } + } } impl AsRef for ExtraName { @@ -35,9 +54,11 @@ impl AsRef for ExtraName { } } -impl From<&str> for ExtraName { - fn from(name: &str) -> Self { - Self::normalize(name) +impl FromStr for ExtraName { + type Err = Error; + + fn from_str(name: &str) -> Result { + Self::validate(name) } } @@ -76,4 +97,27 @@ mod tests { "friendly-bard" ); } + + #[test] + fn validate() { + // Unchanged + assert_eq!( + ExtraName::validate("friendly-bard").unwrap().as_ref(), + "friendly-bard" + ); + assert_eq!(ExtraName::validate("1okay").unwrap().as_ref(), "1okay"); + assert_eq!(ExtraName::validate("okay2").unwrap().as_ref(), "okay2"); + // Normalizes + assert_eq!( + ExtraName::validate("Friendly-Bard").unwrap().as_ref(), + "friendly-bard" + ); + // Failures... + assert!(ExtraName::validate(" starts-with-space").is_err()); + assert!(ExtraName::validate("-starts-with-dash").is_err()); + assert!(ExtraName::validate("ends-with-dash-").is_err()); + assert!(ExtraName::validate("ends-with-space ").is_err()); + assert!(ExtraName::validate("includes!invalid-char").is_err()); + assert!(ExtraName::validate("space in middle").is_err()); + } }