From 42e14954004efc0bb0afc9ad18f09a27aa025ea5 Mon Sep 17 00:00:00 2001 From: Akrm Al-Hakimi Date: Wed, 1 Jul 2026 08:54:18 -0400 Subject: [PATCH 1/5] 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 71c6415832..d4d1c186f5 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -71,19 +71,6 @@ pub enum InvalidName { PlusPrefix(String), } -/// 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 { @@ -463,6 +450,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 7a40cf1a94a6603d63487f544bb642161316dd87 Mon Sep 17 00:00:00 2001 From: Akrm Al-Hakimi Date: Wed, 1 Jul 2026 08:54:44 -0400 Subject: [PATCH 2/5] 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 d4d1c186f5..b0adafb47a 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -90,7 +90,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)); } @@ -172,7 +172,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) @@ -211,7 +211,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)); } @@ -277,9 +277,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())) } } @@ -360,12 +364,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())) @@ -450,6 +451,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('+') { @@ -480,13 +494,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 { @@ -496,9 +518,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 } } @@ -531,11 +552,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::from_str(&name).unwrap(); } #[test] fn test_parse_resolvable_name(name in arb_resolvable_name()) { + prop_assume!(name != "none"); + prop_assume!(name != "."); + prop_assume!(name != ".."); ResolvableToolchainName::from_str(&name).unwrap(); } @@ -567,10 +595,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::from_str(s).unwrap()) @@ -591,11 +619,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::from_str(s).unwrap()) @@ -605,4 +633,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::from_str(name).unwrap_err(); + ResolvableToolchainName::from_str(name).unwrap_err(); + ToolchainName::from_str(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 ee69fdb81d..1f412e7b14 100644 --- a/tests/suite/cli_rustup.rs +++ b/tests/suite/cli_rustup.rs @@ -3790,7 +3790,7 @@ async fn non_utf8_toolchain() { .await .with_stderr(snapbox::str![[r#" ... -error: toolchain '�(' is not installed +error: invalid toolchain name '�(' ... "#]]); } @@ -3817,7 +3817,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 c4a0efd495..ebbb0b89ca 100644 --- a/tests/suite/cli_v2.rs +++ b/tests/suite/cli_v2.rs @@ -1662,7 +1662,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 cd50e82f2ba0fe6a9fc79cb30c3696429153d161 Mon Sep 17 00:00:00 2001 From: Akrm Al-Hakimi Date: Tue, 30 Jun 2026 08:50:06 -0400 Subject: [PATCH 3/5] 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: From f6b0be93a3ee11ea2d530ace4d25ba8175496d48 Mon Sep 17 00:00:00 2001 From: Akrm Al-Hakimi Date: Mon, 3 Aug 2026 09:56:43 -0400 Subject: [PATCH 4/5] feat(toolchain/names): accept unicode identifiers in toolchain names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validate named toolchains with the UTS #39 general security profile via `unicode_security::GeneralSecurityProfile::identifier_allowed()` instead of an ASCII allowlist, following the approach sketched in #4059. Letters and digits in any script are now legal, so `合法的` works as a custom toolchain name. ASCII letters, digits, `.`, `_` and `-` remain allowed, so every official toolchain name still parses. Whitespace, most punctuation, emoji, and invisible or direction-altering characters such as U+202E RIGHT-TO-LEFT OVERRIDE are still rejected. Two characters the profile permits are excluded anyway: `:`, because a named toolchain becomes a directory under `.rustup/toolchains` and `name:stream` denotes an NTFS alternate data stream on Windows, and `'`, which needs quoting in too many shells to be worth allowing. `.` and `..` are rejected explicitly, since the profile permits both. Confusables remain unresolved: `μ` is accepted while `µ` is not, and precomposed and decomposed `é` are distinct names. That matches the existing status quo. --- Cargo.lock | 41 ++++++++++++++++++ Cargo.toml | 1 + doc/user-guide/src/concepts/toolchains.md | 9 +++- src/toolchain/names.rs | 51 ++++++++++++++++++++++- 4 files changed, 98 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e936caa9d4..cb3c1b7017 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2107,6 +2107,7 @@ dependencies = [ "tracing-log", "tracing-opentelemetry", "tracing-subscriber", + "unicode-security", "url", "wait-timeout", "walkdir", @@ -2546,6 +2547,21 @@ dependencies = [ "zerovec", ] +[[package]] +name = "tinyvec" +version = "1.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bb4ebadaa0af04fab11ae01eb5f9fdb5f9c5b875506e210e71c07873528baa7f" +dependencies = [ + "tinyvec_macros", +] + +[[package]] +name = "tinyvec_macros" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" + [[package]] name = "tokio" version = "1.53.1" @@ -2876,6 +2892,31 @@ version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" +[[package]] +name = "unicode-normalization" +version = "0.1.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5fd4f6878c9cb28d874b009da9e8d183b5abc80117c40bbd187a1fde336be6e8" +dependencies = [ + "tinyvec", +] + +[[package]] +name = "unicode-script" +version = "0.5.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "383ad40bb927465ec0ce7720e033cb4ca06912855fc35db31b5755d0de75b1ee" + +[[package]] +name = "unicode-security" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e4ddba1535dd35ed8b61c52166b7155d7f4e4b8847cec6f48e71dc66d8b5e50" +dependencies = [ + "unicode-normalization", + "unicode-script", +] + [[package]] name = "unicode-width" version = "0.2.2" diff --git a/Cargo.toml b/Cargo.toml index eeb5769af8..0ff745fb1e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -101,6 +101,7 @@ tracing = "0.1" tracing-log = "0.2" tracing-opentelemetry = { version = "0.33", optional = true } tracing-subscriber = { version = "0.3.23", features = ["env-filter"] } +unicode-security = "0.1" url = "2.4" wait-timeout = "0.2" xz2 = "0.1.3" diff --git a/doc/user-guide/src/concepts/toolchains.md b/doc/user-guide/src/concepts/toolchains.md index bff57b218d..b80cb8aa2a 100644 --- a/doc/user-guide/src/concepts/toolchains.md +++ b/doc/user-guide/src/concepts/toolchains.md @@ -57,8 +57,13 @@ 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 -`-`. +Custom toolchain names may contain any character that [UTS #39] allows in an +identifier, except for `:` and `'`. In practice that means letters and digits +in any script, along with `.`, `_`, and `-`; whitespace, most punctuation, +emoji, and invisible or direction-altering characters are rejected. The names +`.` and `..` are not accepted either. + +[UTS #39]: https://www.unicode.org/reports/tr39/#Identifier_Status_and_Type For example, on Ubuntu you might clone `rust-lang/rust` into `~/rust`, build it, and then run: diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index b0adafb47a..b47fb026ae 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -49,6 +49,7 @@ use std::{ }; use thiserror::Error; +use unicode_security::GeneralSecurityProfile; use crate::dist::{PartialToolchainDesc, TargetTuple, ToolchainDesc}; @@ -451,12 +452,22 @@ impl Deref for PathBasedToolchainName { } } +/// Common validate rules for toolchain names that aren't paths. +/// +/// Beyond the shared [`validate`] rules, every character must be allowed in an +/// identifier by the Unicode general security profile (UTS #39). That admits +/// ASCII letters, digits, `.`, `_` and `-` -- so every official toolchain name +/// still parses -- along with unicode identifiers such as `合法的`, while +/// rejecting whitespace, most punctuation, emoji, and invisible or +/// direction-altering characters. fn validate_named_toolchain(candidate: &str) -> Result<&str, InvalidName> { let candidate = validate(candidate)?; + // `.` and `..` pass the profile but would escape or alias the toolchains + // directory, so they need rejecting on their own. if !matches!(candidate, "." | "..") && candidate .chars() - .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-')) + .all(|c| c.identifier_allowed() && !EXCLUDED.contains(&c)) { Ok(candidate) } else { @@ -477,6 +488,12 @@ fn validate(candidate: &str) -> Result<&str, InvalidName> { } } +/// Characters that UTS #39 permits in identifiers but that we reject anyway, +/// because a named toolchain also has to work as a directory name under +/// `.rustup/toolchains`: `:` would open an NTFS alternate data stream, and `'` +/// needs quoting in enough shells to be a nuisance. +const EXCLUDED: &[char] = &[':', '\'']; + #[cfg(test)] mod tests { use std::str::FromStr; @@ -641,13 +658,43 @@ mod tests { "foo#bar", "the cake is a lie", "this.is.not-a+semver", - "quote'toolchain", ".", "..", + // permitted by UTS #39, excluded by us + "quote'toolchain", + "stream:name", + // rejected by UTS #39 + "µ", // MICRO SIGN, restricted in favour of U+03BC + "🦀", // emoji + "tilde~home", // not identifier punctuation + "abc\u{202E}def", // RIGHT-TO-LEFT OVERRIDE + "zero\u{200D}width", // ZERO WIDTH JOINER + "no\u{00A0}break", // NO-BREAK SPACE ] { CustomToolchainName::from_str(name).unwrap_err(); ResolvableToolchainName::from_str(name).unwrap_err(); ToolchainName::from_str(name).unwrap_err(); } } + + #[test] + fn custom_names_accept_unicode_identifiers() { + for name in ["合法的", "μ", "café", "Ελληνικά", "тулчейн"] { + CustomToolchainName::from_str(name).unwrap(); + ResolvableToolchainName::from_str(name).unwrap(); + ToolchainName::from_str(name).unwrap(); + } + } + + #[test] + fn official_names_survive_the_identifier_profile() { + for name in [ + "stable-x86_64-unknown-linux-gnu", + "nightly-2015-01-01-aarch64-apple-darwin", + "1.86.0-x86_64-pc-windows-msvc", + "beta-i686-pc-windows-gnullvm", + ] { + ToolchainName::from_str(name).unwrap(); + } + } } From 028f723a35969d4424d7d2fd2771a08270c091bd Mon Sep 17 00:00:00 2001 From: Akrm Al-Hakimi Date: Mon, 3 Aug 2026 10:23:52 -0400 Subject: [PATCH 5/5] refactor(toolchain/names): inline `validate()` aliases into `FromStr` `try_from_str!()` fed `TryFrom`, `TryFrom<&str>` and `FromStr` off a single inherent `validate()`, which is why each name type had one. With the macro gone every such method had exactly one caller, its own `from_str()`, so move the bodies there and drop the methods. Rename the free `validate()` to `normalize_name()`, which is precisely what it does: strips a `+` prefix, trims trailing slashes, and rejects empty names. --- src/toolchain/names.rs | 128 ++++++++++++++++------------------------- 1 file changed, 50 insertions(+), 78 deletions(-) diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index b47fb026ae..4f7709b3d8 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -87,11 +87,15 @@ impl ResolvableToolchainName { Self::Official(desc) => ToolchainName::Official(desc.resolve(host)?), }) } +} - // If candidate could be resolved, return a ready to resolve version of it. +impl FromStr for ResolvableToolchainName { + type Err = InvalidName; + + // If value could be resolved, return a ready to resolve version of it. // Otherwise error. - fn validate(candidate: &str) -> Result { - let candidate = validate_named_toolchain(candidate)?; + fn from_str(value: &str) -> Result { + let candidate = validate_named_toolchain(value)?; if let Ok(desc) = PartialToolchainDesc::from_str(candidate) { return Ok(Self::Official(desc)); } @@ -103,14 +107,6 @@ impl ResolvableToolchainName { } } -impl FromStr for ResolvableToolchainName { - type Err = InvalidName; - - fn from_str(value: &str) -> Result { - Self::validate(value) - } -} - impl From<&PartialToolchainDesc> for ResolvableToolchainName { fn from(value: &PartialToolchainDesc) -> Self { Self::Official(value.to_owned()) @@ -135,22 +131,16 @@ pub(crate) enum MaybeResolvableToolchainName { None, } -impl MaybeResolvableToolchainName { - // If candidate could be resolved, return a ready to resolve version of it. - // Otherwise error. - fn validate(candidate: &str) -> Result { - Ok(match validate(candidate)? { - "none" => Self::None, - candidate => Self::Some(ResolvableToolchainName::validate(candidate)?), - }) - } -} - impl FromStr for MaybeResolvableToolchainName { type Err = InvalidName; + // If value could be resolved, return a ready to resolve version of it. + // Otherwise error. fn from_str(value: &str) -> Result { - Self::validate(value) + Ok(match normalize_name(value)? { + "none" => Self::None, + candidate => Self::Some(ResolvableToolchainName::from_str(candidate)?), + }) } } @@ -171,9 +161,11 @@ pub(crate) enum MaybeOfficialToolchainName { Some(PartialToolchainDesc), } -impl MaybeOfficialToolchainName { - fn validate(candidate: &str) -> Result { - Ok(match validate_named_toolchain(candidate)? { +impl FromStr for MaybeOfficialToolchainName { + type Err = InvalidName; + + fn from_str(value: &str) -> Result { + Ok(match validate_named_toolchain(value)? { "none" => Self::None, candidate => Self::Some( PartialToolchainDesc::from_str(candidate) @@ -183,14 +175,6 @@ impl MaybeOfficialToolchainName { } } -impl FromStr for MaybeOfficialToolchainName { - type Err = InvalidName; - - fn from_str(value: &str) -> Result { - Self::validate(value) - } -} - impl Display for MaybeOfficialToolchainName { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -209,21 +193,6 @@ pub enum ToolchainName { Custom(CustomToolchainName), } -impl ToolchainName { - /// If the string is already resolved, allow direct conversion - fn validate(candidate: &str) -> Result { - let candidate = validate_named_toolchain(candidate)?; - if let Ok(desc) = ToolchainDesc::from_str(candidate) { - return Ok(Self::Official(desc)); - } - - match CustomToolchainName::from_str(candidate) { - Ok(custom) => Ok(Self::Custom(custom)), - Err(_) => Err(InvalidName::ToolchainName(candidate.into())), - } - } -} - impl From for ToolchainName { fn from(value: ToolchainDesc) -> Self { Self::Official(value) @@ -239,8 +208,17 @@ impl From for ToolchainName { impl FromStr for ToolchainName { type Err = InvalidName; + /// If the string is already resolved, allow direct conversion fn from_str(value: &str) -> Result { - Self::validate(value) + let candidate = validate_named_toolchain(value)?; + if let Ok(desc) = ToolchainDesc::from_str(candidate) { + return Ok(Self::Official(desc)); + } + + match CustomToolchainName::from_str(candidate) { + Ok(custom) => Ok(Self::Custom(custom)), + Err(_) => Err(InvalidName::ToolchainName(candidate.into())), + } } } @@ -270,10 +248,14 @@ impl ResolvableLocalToolchainName { Self::Path(t) => Ok(LocalToolchainName::Path(t)), } } +} + +impl FromStr for ResolvableLocalToolchainName { + type Err = InvalidName; - /// Validates if the string is a resolvable toolchain, or a path based toolchain. - fn validate(candidate: &str) -> Result { - let candidate = validate(candidate)?; + /// Parses a resolvable toolchain, or a path based toolchain. + fn from_str(value: &str) -> Result { + let candidate = normalize_name(value)?; if let Ok(name) = ResolvableToolchainName::from_str(candidate) { return Ok(Self::Named(name)); } @@ -288,14 +270,6 @@ impl ResolvableLocalToolchainName { } } -impl FromStr for ResolvableLocalToolchainName { - type Err = InvalidName; - - fn from_str(value: &str) -> Result { - Self::validate(value) - } -} - impl Display for ResolvableLocalToolchainName { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -363,18 +337,6 @@ impl Display for LocalToolchainName { #[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] pub struct CustomToolchainName(String); -impl CustomToolchainName { - fn validate(candidate: &str) -> Result { - 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())) - } - } -} - impl Deref for CustomToolchainName { type Target = str; @@ -387,7 +349,13 @@ impl FromStr for CustomToolchainName { type Err = InvalidName; fn from_str(value: &str) -> Result { - Self::validate(value) + let candidate = + validate_named_toolchain(value).map_err(|_| InvalidName::CustomName(value.into()))?; + if candidate.parse::().is_ok() || candidate == "none" { + Err(InvalidName::CustomName(candidate.into())) + } else { + Ok(Self(candidate.into())) + } } } @@ -454,14 +422,14 @@ impl Deref for PathBasedToolchainName { /// Common validate rules for toolchain names that aren't paths. /// -/// Beyond the shared [`validate`] rules, every character must be allowed in an +/// Beyond the shared [`normalize_name`] rules, every character must be allowed in an /// identifier by the Unicode general security profile (UTS #39). That admits /// ASCII letters, digits, `.`, `_` and `-` -- so every official toolchain name /// still parses -- along with unicode identifiers such as `合法的`, while /// rejecting whitespace, most punctuation, emoji, and invisible or /// direction-altering characters. fn validate_named_toolchain(candidate: &str) -> Result<&str, InvalidName> { - let candidate = validate(candidate)?; + let candidate = normalize_name(candidate)?; // `.` and `..` pass the profile but would escape or alias the toolchains // directory, so they need rejecting on their own. if !matches!(candidate, "." | "..") @@ -475,8 +443,12 @@ fn validate_named_toolchain(candidate: &str) -> Result<&str, InvalidName> { } } -/// Common validate rules for all sorts of toolchain names -fn validate(candidate: &str) -> Result<&str, InvalidName> { +/// Normalization shared by all sorts of toolchain names. +/// +/// Strips the trailing slashes a shell may have completed onto a toolchain +/// directory, and rejects the `+toolchain` argument form along with names that +/// are empty once normalized. +fn normalize_name(candidate: &str) -> Result<&str, InvalidName> { if let Some(without_plus) = candidate.strip_prefix('+') { return Err(InvalidName::PlusPrefix(without_plus.to_string())); }