Skip to content

Commit

Permalink
feat: restrict semver requirement specification
Browse files Browse the repository at this point in the history
  • Loading branch information
halajohn committed Feb 10, 2025
1 parent 25e442c commit a716f1d
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 13 deletions.
32 changes: 30 additions & 2 deletions core/src/ten_manager/src/cmd/cmd_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,36 @@ pub async fn execute_cmd(
Some(local_pkg_info.basic_info.type_and_name.pkg_type);
installing_pkg_name =
Some(local_pkg_info.basic_info.type_and_name.name.clone());
let installing_pkg_version_req =
VersionReq::parse(&local_pkg_info.basic_info.version.to_string())?;

// Currently, tman uses the Rust semver crate, while the cloud store
// uses the npm semver package. The semver requirement specifications of
// these two packages are not completely identical. For example:
//
// - The Rust semver crate uses "," to separate different ranges,
// whereas the npm semver package uses a space (" ") to separate
// different requirement ranges.
// - The npm semver package uses "||" to unify different ranges, but the
// Rust semver crate does not support this feature.
//
// Since TEN is a cross-language system, it needs to define its own
// semver requirement specification. This specification could follow
// either the Rust or npm format or other spec, but in either case, tman
// or the cloud store would need to make adaptations.
//
// Therefore, the current approach is to simplify the specification to
// only support a single-range semver requirement, which is the common
// subset of both the npm semver package and the Rust semver crate.
let local_version_str = local_pkg_info.basic_info.version.to_string();
if local_version_str.contains("||")
|| local_version_str.chars().any(|c| c.is_whitespace())
|| local_version_str.contains(",")
{
return Err(anyhow!(
"Invalid version requirement '{}' in local package manifest: contains forbidden characters (||, whitespace, or ,)",
local_version_str
));
}
let installing_pkg_version_req = VersionReq::parse(&local_version_str)?;

dep_relationship_from_cmd_line = Some(DependencyRelationship {
type_and_name: PkgTypeAndName {
Expand Down
30 changes: 29 additions & 1 deletion core/src/ten_manager/src/version_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// Licensed under the Apache License, Version 2.0, with certain conditions.
// Refer to the "LICENSE" file in the root directory for more information.
//
use anyhow::Result;
use anyhow::{anyhow, Result};
use semver::VersionReq;

/// Used to parse a pattern like `[email protected]` and return `aaa` and `3.0.0`.
Expand All @@ -13,6 +13,34 @@ pub fn parse_pkg_name_version_req(
) -> Result<(String, VersionReq)> {
let parts: Vec<&str> = pkg_name_version.split('@').collect();
if parts.len() == 2 {
// Currently, tman uses the Rust semver crate, while the cloud store
// uses the npm semver package. The semver requirement specifications of
// these two packages are not completely identical. For example:
//
// - The Rust semver crate uses "," to separate different ranges,
// whereas the npm semver package uses a space (" ") to separate
// different requirement ranges.
// - The npm semver package uses "||" to unify different ranges, but the
// Rust semver crate does not support this feature.
//
// Since TEN is a cross-language system, it needs to define its own
// semver requirement specification. This specification could follow
// either the Rust or npm format or other spec, but in either case, tman
// or the cloud store would need to make adaptations.
//
// Therefore, the current approach is to simplify the specification to
// only support a single-range semver requirement, which is the common
// subset of both the npm semver package and the Rust semver crate.
if parts[1].contains("||")
|| parts[1].chars().any(|c| c.is_whitespace())
|| parts[1].contains(",")
{
return Err(anyhow!(
"Invalid version requirement '{}' in package name version string: contains forbidden characters (||, whitespace, or ,)",
parts[1]
));
}

Ok((parts[0].to_string(), VersionReq::parse(parts[1])?))
} else {
Ok((pkg_name_version.to_string(), VersionReq::STAR))
Expand Down
20 changes: 19 additions & 1 deletion core/src/ten_rust/src/json_schema/data/manifest.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,25 @@
},
"versionRequirement": {
"type": "string",
"pattern": "^(\\^|~|>|<|>=|<=|=)?\\d+(\\.\\d+)?(\\.\\d+)?(-[0-9A-Za-z-]+(\\.[0-9A-Za-z-]+)*)?(\\+[0-9A-Za-z-]+(\\.[0-9A-Za-z-]+)*)?([\\s\\,]*\\|\\|[\\s\\,]*(\\^|~|>|<|>=|<=)?\\d+(\\.\\d+)?(\\.\\d+)?(-[0-9A-Za-z-]+(\\.[0-9A-Za-z-]+)*)?(\\+[0-9A-Za-z-]+(\\.[0-9A-Za-z-]+)*)?)*$"
// Currently, tman uses the Rust semver crate, while the cloud store uses
// the npm semver package. The semver requirement specifications of these
// two packages are not completely identical. For example:
//
// - The Rust semver crate uses "," to separate different ranges, whereas
// the npm semver package uses a space (" ") to separate different
// requirement ranges.
// - The npm semver package uses "||" to unify different ranges, but the
// Rust semver crate does not support this feature.
//
// Since TEN is a cross-language system, it needs to define its own semver
// requirement specification. This specification could follow either the
// Rust or npm format or other spec, but in either case, tman or the cloud
// store would need to make adaptations.
//
// Therefore, the current approach is to simplify the specification to
// only support a single-range semver requirement, which is the common
// subset of both the npm semver package and the Rust semver crate.
"pattern": "^(?:\\^|~|>|<|>=|<=|=)?\\d+(?:\\.\\d+)?(?:\\.\\d+)?(?:-[0-9A-Za-z-]+(?:\\.[0-9A-Za-z-]+)*)?(?:\\+[0-9A-Za-z-]+(?:\\.[0-9A-Za-z-]+)*)?$"
},
"httpsUri": {
"type": "string",
Expand Down
82 changes: 73 additions & 9 deletions core/src/ten_rust/src/pkg_info/dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,48 @@ impl TryFrom<&ManifestDependency> for PkgDependency {
pkg_type,
name,
version,
} => Ok(PkgDependency {
type_and_name: PkgTypeAndName {
pkg_type: PkgType::from_str(pkg_type)?,
name: name.clone(),
},
version_req: VersionReq::parse(version)?,
path: None,
base_dir: None,
}),
} => {
// Currently, tman uses the Rust semver crate, while the cloud
// store uses the npm semver package. The semver requirement
// specifications of these two packages are not completely
// identical. For example:
//
// - The Rust semver crate uses "," to separate different
// ranges, whereas the npm semver package uses a space (" ")
// to separate different requirement ranges.
// - The npm semver package uses "||" to unify different ranges,
// but the Rust semver crate does not support this feature.
//
// Since TEN is a cross-language system, it needs to define its
// own semver requirement specification. This specification
// could follow either the Rust or npm format or other spec, but
// in either case, tman or the cloud store would need to make
// adaptations.
//
// Therefore, the current approach is to simplify the
// specification to only support a single-range semver
// requirement, which is the common subset of both the npm
// semver package and the Rust semver crate.
if version.contains("||")
|| version.chars().any(|c| c.is_whitespace())
|| version.contains(",")
{
return Err(anyhow!(
"Invalid version requirement '{}' in manifest: contains forbidden characters (||, whitespace, or ,)",
version
));
}

Ok(PkgDependency {
type_and_name: PkgTypeAndName {
pkg_type: PkgType::from_str(pkg_type)?,
name: name.clone(),
},
version_req: VersionReq::parse(version)?,
path: None,
base_dir: None,
})
}

ManifestDependency::LocalDependency { path, base_dir } => {
// Check if there is a manifest.json file under the path.
Expand Down Expand Up @@ -97,6 +130,37 @@ impl TryFrom<&ManifestDependency> for PkgDependency {
&dep_manifest_path,
)?;

// Currently, tman uses the Rust semver crate, while the cloud
// store uses the npm semver package. The semver requirement
// specifications of these two packages are not completely
// identical. For example:
//
// - The Rust semver crate uses "," to separate different
// ranges, whereas the npm semver package uses a space (" ")
// to separate different requirement ranges.
// - The npm semver package uses "||" to unify different ranges,
// but the Rust semver crate does not support this feature.
//
// Since TEN is a cross-language system, it needs to define its
// own semver requirement specification. This specification
// could follow either the Rust or npm format or other spec, but
// in either case, tman or the cloud store would need to make
// adaptations.
//
// Therefore, the current approach is to simplify the
// specification to only support a single-range semver
// requirement, which is the common subset of both the npm
// semver package and the Rust semver crate.
if local_manifest.version.contains("||")
|| local_manifest.version.chars().any(|c| c.is_whitespace())
|| local_manifest.version.contains(",")
{
return Err(anyhow!(
"Invalid version requirement '{}' in local manifest: contains forbidden characters (||, whitespace, or ,)",
local_manifest.version
));
}

Ok(PkgDependency {
type_and_name: local_manifest.type_and_name.clone(),
version_req: semver::VersionReq::parse(
Expand Down

0 comments on commit a716f1d

Please sign in to comment.