From 5a40727bc4756c8c91884594274f943a90728dfe Mon Sep 17 00:00:00 2001 From: Akrm Al-Hakimi Date: Wed, 1 Jul 2026 08:54:18 -0400 Subject: [PATCH 1/3] refactor(toolchain): move name validation helper --- src/toolchain/names.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index 37d92b9912..1d5300cf41 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -112,19 +112,6 @@ macro_rules! try_from_str { }; } -/// Common validate rules for all sorts of toolchain names -fn validate(candidate: &str) -> Result<&str, InvalidName> { - if let Some(without_plus) = candidate.strip_prefix('+') { - return Err(InvalidName::PlusPrefix(without_plus.to_string())); - } - let normalized_name = candidate.trim_end_matches('/'); - if normalized_name.is_empty() { - Err(InvalidName::ToolchainName(candidate.into())) - } else { - Ok(normalized_name) - } -} - /// A toolchain name from user input. #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] pub(crate) enum ResolvableToolchainName { @@ -460,6 +447,19 @@ impl Deref for PathBasedToolchainName { } } +/// Common validate rules for all sorts of toolchain names +fn validate(candidate: &str) -> Result<&str, InvalidName> { + if let Some(without_plus) = candidate.strip_prefix('+') { + return Err(InvalidName::PlusPrefix(without_plus.to_string())); + } + let normalized_name = candidate.trim_end_matches('/'); + if normalized_name.is_empty() { + Err(InvalidName::ToolchainName(candidate.into())) + } else { + Ok(normalized_name) + } +} + #[cfg(test)] mod tests { use std::str::FromStr; From abfb1f1912431230ba5c12395cb1792187da59af Mon Sep 17 00:00:00 2001 From: Akrm Al-Hakimi Date: Wed, 1 Jul 2026 08:54:44 -0400 Subject: [PATCH 2/3] fix(toolchain)!: restrict named toolchain characters --- src/toolchain/names.rs | 95 ++++++++++++++++++++++++++++----------- tests/suite/cli_misc.rs | 18 ++++++++ tests/suite/cli_rustup.rs | 4 +- tests/suite/cli_v2.rs | 2 +- 4 files changed, 91 insertions(+), 28 deletions(-) diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index 1d5300cf41..efdbec75ec 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -131,7 +131,7 @@ impl ResolvableToolchainName { // If candidate could be resolved, return a ready to resolve version of it. // Otherwise error. fn validate(candidate: &str) -> Result { - let candidate = validate(candidate)?; + let candidate = validate_named_toolchain(candidate)?; if let Ok(desc) = PartialToolchainDesc::from_str(candidate) { return Ok(Self::Official(desc)); } @@ -201,7 +201,7 @@ pub(crate) enum MaybeOfficialToolchainName { impl MaybeOfficialToolchainName { fn validate(candidate: &str) -> Result { - Ok(match validate(candidate)? { + Ok(match validate_named_toolchain(candidate)? { "none" => Self::None, candidate => Self::Some( PartialToolchainDesc::from_str(candidate) @@ -234,7 +234,7 @@ pub enum ToolchainName { impl ToolchainName { /// If the string is already resolved, allow direct conversion fn validate(candidate: &str) -> Result { - let candidate = validate(candidate)?; + let candidate = validate_named_toolchain(candidate)?; if let Ok(desc) = ToolchainDesc::from_str(candidate) { return Ok(Self::Official(desc)); } @@ -285,9 +285,13 @@ impl ResolvableLocalToolchainName { return Ok(Self::Named(name)); } - Ok(Self::Path(PathBasedToolchainName::try_from( - &PathBuf::from(candidate) as &Path, - )?)) + if candidate.contains('/') || candidate.contains('\\') { + let path = PathBuf::from(candidate); + let path = PathBasedToolchainName::try_from(&path as &Path)?; + return Ok(Self::Path(path)); + } + + Err(InvalidName::ToolchainName(candidate.into())) } } @@ -363,12 +367,9 @@ pub struct CustomToolchainName(String); impl CustomToolchainName { fn validate(candidate: &str) -> Result { - let candidate = validate(candidate)?; - if candidate.parse::().is_ok() - || candidate == "none" - || candidate.contains('/') - || candidate.contains('\\') - { + let candidate = validate_named_toolchain(candidate) + .map_err(|_| InvalidName::CustomName(candidate.into()))?; + if candidate.parse::().is_ok() || candidate == "none" { Err(InvalidName::CustomName(candidate.into())) } else { Ok(Self(candidate.into())) @@ -447,6 +448,19 @@ impl Deref for PathBasedToolchainName { } } +fn validate_named_toolchain(candidate: &str) -> Result<&str, InvalidName> { + let candidate = validate(candidate)?; + if !matches!(candidate, "." | "..") + && candidate + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-')) + { + Ok(candidate) + } else { + Err(InvalidName::ToolchainName(candidate.to_owned())) + } +} + /// Common validate rules for all sorts of toolchain names fn validate(candidate: &str) -> Result<&str, InvalidName> { if let Some(without_plus) = candidate.strip_prefix('+') { @@ -477,13 +491,21 @@ mod tests { fn partial_toolchain_desc_regex() -> String { let tuple_regex = format!( r"(-({}))?(?:-({}))?(?:-({}))?", - LIST_ARCHS.join("|"), - LIST_OSES.join("|"), - LIST_ENVS.join("|") + regex_alternates(LIST_ARCHS), + regex_alternates(LIST_OSES), + regex_alternates(LIST_ENVS) ); r"(nightly|beta|stable|[0-9]{1}(\.(0|[1-9][0-9]{0,2}))(\.(0|[1-9][0-9]{0,1}))?(-beta(\.(0|[1-9][1-9]{0,1}))?)?)(-([0-9]{4}-[0-9]{2}-[0-9]{2}))?".to_owned() + &tuple_regex } + fn regex_alternates(values: &[&str]) -> String { + values + .iter() + .map(|value| regex::escape(value)) + .collect::>() + .join("|") + } + prop_compose! { fn arb_partial_toolchain_desc() (s in string_regex(&partial_toolchain_desc_regex()).unwrap()) -> String { @@ -493,9 +515,8 @@ mod tests { prop_compose! { fn arb_custom_name() - (s in r"[^\\/+][^\\/]*") -> String { + (s in r"[A-Za-z0-9._-]+") -> String { // perhaps need to filter 'none' and partial toolchains - but they won't typically be generated anyway. - // Also filter '+' prefix as that's reserved for +toolchain syntax. s } } @@ -528,11 +549,18 @@ mod tests { #[test] fn test_parse_custom(name in arb_custom_name()) { + prop_assume!(name != "none"); + prop_assume!(name != "."); + prop_assume!(name != ".."); + prop_assume!(PartialToolchainDesc::from_str(&name).is_err()); CustomToolchainName::try_from(name).unwrap(); } #[test] fn test_parse_resolvable_name(name in arb_resolvable_name()) { + prop_assume!(name != "none"); + prop_assume!(name != "."); + prop_assume!(name != ".."); ResolvableToolchainName::try_from(name).unwrap(); } @@ -564,10 +592,10 @@ mod tests { "1.8.0-x86_64-apple-darwin", "1.8.0-x86_64-unknown-linux-gnu", "1.10.0-x86_64-unknown-linux-gnu", - "bar(baz)", - "foo#bar", - "the cake is a lie", - "this.is.not-a+semver", + "bar.baz", + "foo_bar", + "stage1-local", + "this.is.not-a_semver", ] .into_iter() .map(|s| ToolchainName::try_from(s).unwrap()) @@ -588,11 +616,11 @@ mod tests { "1.8.0-beta-x86_64-apple-darwin", "1.8.0-beta.2-x86_64-apple-darwin", // https://github.com/rust-lang/rustup/issues/3517 - "foo#bar", - "bar(baz)", - "this.is.not-a+semver", + "foo_bar", + "bar.baz", + "this.is.not-a_semver", // https://github.com/rust-lang/rustup/issues/3168 - "the cake is a lie", + "stage1-local", ] .into_iter() .map(|s| ToolchainName::try_from(s).unwrap()) @@ -602,4 +630,21 @@ mod tests { assert_eq!(expected, v); } + + #[test] + fn custom_names_reject_special_characters() { + for name in [ + "bar(baz)", + "foo#bar", + "the cake is a lie", + "this.is.not-a+semver", + "quote'toolchain", + ".", + "..", + ] { + CustomToolchainName::try_from(name).unwrap_err(); + ResolvableToolchainName::try_from(name).unwrap_err(); + ToolchainName::try_from(name).unwrap_err(); + } + } } diff --git a/tests/suite/cli_misc.rs b/tests/suite/cli_misc.rs index 1f52643e88..09674cd7c4 100644 --- a/tests/suite/cli_misc.rs +++ b/tests/suite/cli_misc.rs @@ -98,6 +98,24 @@ error:[..] invalid custom toolchain name 'beta' ... error:[..] invalid custom toolchain name 'stable' ... +"#]]) + .is_err(); + cx.config + .expect(["rustup", "toolchain", "link", "bad name", "foo"]) + .await + .with_stderr(snapbox::str![[r#" +... +error:[..] invalid custom toolchain name 'bad name' +... +"#]]) + .is_err(); + cx.config + .expect(["rustup", "toolchain", "link", "foo#bar", "foo"]) + .await + .with_stderr(snapbox::str![[r#" +... +error:[..] invalid custom toolchain name 'foo#bar' +... "#]]) .is_err(); } diff --git a/tests/suite/cli_rustup.rs b/tests/suite/cli_rustup.rs index 7d49246411..4452667fd9 100644 --- a/tests/suite/cli_rustup.rs +++ b/tests/suite/cli_rustup.rs @@ -3747,7 +3747,7 @@ async fn non_utf8_toolchain() { .await .with_stderr(snapbox::str![[r#" ... -error: toolchain '�(' is not installed +error: invalid toolchain name '�(' ... "#]]); } @@ -3774,7 +3774,7 @@ async fn non_utf8_toolchain() { .await .with_stderr(snapbox::str![[r#" ... -error: toolchain '��' is not installed +error: invalid toolchain name '��' ... "#]]); } diff --git a/tests/suite/cli_v2.rs b/tests/suite/cli_v2.rs index 6b6d9aef7c..b101c91939 100644 --- a/tests/suite/cli_v2.rs +++ b/tests/suite/cli_v2.rs @@ -1617,7 +1617,7 @@ async fn cannot_add_empty_named_custom_toolchain() { .await .with_stderr(snapbox::str![[r#" ... -error: invalid value '' for '': invalid toolchain name '' +error: invalid value '' for '': invalid custom toolchain name '' ... "#]]) .is_err(); From 39fbbccc6f525579e1189dcba3577a2614b12295 Mon Sep 17 00:00:00 2001 From: Akrm Al-Hakimi Date: Tue, 30 Jun 2026 08:50:06 -0400 Subject: [PATCH 3/3] doc: document custom toolchain name characters --- doc/user-guide/src/concepts/toolchains.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/user-guide/src/concepts/toolchains.md b/doc/user-guide/src/concepts/toolchains.md index 336d6a5ae8..bff57b218d 100644 --- a/doc/user-guide/src/concepts/toolchains.md +++ b/doc/user-guide/src/concepts/toolchains.md @@ -57,6 +57,9 @@ local builds of the Rust toolchain. To teach `rustup` about your build, run: $ rustup toolchain link my-toolchain path/to/my/toolchain/sysroot ``` +Custom toolchain names may contain ASCII letters, ASCII digits, `.`, `_`, and +`-`. + For example, on Ubuntu you might clone `rust-lang/rust` into `~/rust`, build it, and then run: