Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Hipcheck now works with scoped npm packages. #238

Merged
merged 1 commit into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions hipcheck/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,9 +568,10 @@ impl ToTargetSeed for CheckNpmArgs {
_ => pm::extract_package_version(raw_package)?,
};

// If the package is scoped, replace the leading '@' in the scope with %40 for proper pURL formatting
let purl = Url::parse(&match version.as_str() {
"no version" => format!("pkg:npm/{}", name),
_ => format!("pkg:npm/{}@{}", name, version),
"no version" => format!("pkg:npm/{}", str::replace(&name, '@', "%40")),
_ => format!("pkg:npm/{}@{}", str::replace(&name, '@', "%40"), version),
})
.unwrap();

Expand Down Expand Up @@ -1248,7 +1249,7 @@ mod tests {

#[test]
fn test_deductive_check_npm_purl() {
let package = "[email protected]".to_string();
let package = "@expressjs/[email protected]".to_string();
let cmd =
get_check_cmd_from_cli(vec!["hc", "check", "pkg:npm/%40expressjs/[email protected]"]);
assert!(matches!(cmd, Ok(CheckCommand::Npm(..))));
Expand Down
90 changes: 84 additions & 6 deletions hipcheck/src/session/pm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,22 @@ impl PackageManager {
pub fn extract_package_version(raw_package: &str) -> Result<(String, String)> {
// Get the package and version from package argument in form package@version because it has @ symbol in it
let mut package_and_version = raw_package.split('@');
let package_value = match package_and_version.next() {
Some(package) => Ok(package),
_ => Err(Error::msg("unable to get package from package@version")),

// Check if the package is scoped, in the form "@scope/package". If it is, include the scope and package as the package name
let package_value = match raw_package.starts_with('@') {
true => {
package_and_version.next();
match package_and_version.next() {
Some(package) => Ok(format!("@{}", package)),
_ => Err(Error::msg("unable to get package from package@version")),
}
}
false => match package_and_version.next() {
Some(package) => Ok(package.to_string()),
_ => Err(Error::msg("unable to get package from package@version")),
},
};

Ok((
package_value.unwrap().to_string(), //this wont panic because we check for it above
package_and_version
Expand All @@ -320,7 +332,17 @@ pub fn extract_package_version_from_url(url: Url) -> Result<(String, String)> {
.path_segments()
.ok_or_else(|| hc_error!("Unable to get path"))?;
let package_value = match path_segments.next() {
Some(package) => Ok(package),
Some(first) => {
// Check if the package is scoped, in the form "@scope/package". If it is, include the scope and package as the package name
if first.starts_with('@') {
match path_segments.next() {
Some(package) => Ok(format!("{}/{}", first, package)),
_ => Err(Error::msg("unable to get package from uri")),
}
} else {
Ok(first.to_string())
}
}
_ => Err(Error::msg("unable to get package from uri")),
};
// An empty string or no string at all should both give "no version" as the version
Expand All @@ -330,8 +352,8 @@ pub fn extract_package_version_from_url(url: Url) -> Result<(String, String)> {
None => "no version",
};
Ok((
package_value.unwrap().to_string(), //this will graceful error if empty because of panic checking above
version.to_string(), //we check for this in match so we can format url correctly
package_value.unwrap(), //this will graceful error if empty because of panic checking above
version.to_string(), //we check for this in match so we can format url correctly
))
} else if package_type.contains(PYPI) {
//pypi gets the second and third segments
Expand Down Expand Up @@ -876,6 +898,62 @@ mod tests {
}
}

#[test]
fn test_extract_repo_for_npm_6() {
let npm_package = "@types/[email protected]";
let link2 = "https://github.com/DefinitelyTyped/DefinitelyTyped.git";

let target_seed = CheckNpmArgs {
package: npm_package.to_string(),
}
.to_target_seed()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
assert_eq!(
package,
Package {
purl: Url::parse("pkg:npm/%40types/[email protected]").unwrap(),
name: "@types/ua-parser-js".to_string(),
version: "0.7.36".to_string(),
host: PackageHost::Npm
}
);

let npm_git = Url::parse(link2).unwrap();
assert_eq!(extract_repo_for_npm_package(&package).unwrap(), npm_git);
} else {
panic!()
}
}

#[test]
fn test_extract_repo_for_npm_7() {
let npm_package = "https://registry.npmjs.org/@types/ua-parser-js/0.7.36";
let link2 = "https://github.com/DefinitelyTyped/DefinitelyTyped.git";

let target_seed = CheckNpmArgs {
package: npm_package.to_string(),
}
.to_target_seed()
.unwrap();
if let TargetSeed::Package(package) = target_seed {
assert_eq!(
package,
Package {
purl: Url::parse("pkg:npm/%40types/[email protected]").unwrap(),
name: "@types/ua-parser-js".to_string(),
version: "0.7.36".to_string(),
host: PackageHost::Npm
}
);

let npm_git = Url::parse(link2).unwrap();
assert_eq!(extract_repo_for_npm_package(&package).unwrap(), npm_git);
} else {
panic!()
}
}

#[test]
/// Tests scm:git: prefix removal case.
fn test_extract_repo_for_maven_2() {
Expand Down
9 changes: 8 additions & 1 deletion hipcheck/src/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,15 @@ impl TargetType {
}
"npm" => {
// Construct NPM package w/ optional version from pURL as the updated target string
let mut package = String::new();

// Include scope if provided
if let Some(scope) = purl.namespace() {
package.push_str(scope);
package.push('/');
}
let name = purl.name();
let mut package = name.to_string();
package.push_str(name);
// Include version if provided
if let Some(version) = purl.version() {
package.push('@');
Expand Down