Skip to content

Commit 86f5e64

Browse files
cachebagcachebag
committed
fix(toolchain): restrict named toolchain characters
1 parent 30f9134 commit 86f5e64

3 files changed

Lines changed: 87 additions & 47 deletions

File tree

‎src/toolchain/names.rs‎

Lines changed: 67 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,14 @@ macro_rules! try_from_str {
117117
};
118118
}
119119

120+
fn validate_named_toolchain(candidate: &str) -> Result<&str, InvalidName> {
121+
let candidate = validate(candidate)?;
122+
if is_legal_named_toolchain(candidate) {
123+
Ok(candidate)
124+
} else {
125+
Err(InvalidName::ToolchainName(candidate.into()))
126+
}
127+
}
120128
/// Common validate rules for all sorts of toolchain names
121129
fn validate(candidate: &str) -> Result<&str, InvalidName> {
122130
if let Some(without_plus) = candidate.strip_prefix('+') {
@@ -130,6 +138,12 @@ fn validate(candidate: &str) -> Result<&str, InvalidName> {
130138
}
131139
}
132140

141+
fn is_legal_named_toolchain(candidate: &str) -> bool {
142+
!matches!(candidate, "." | "..")
143+
&& candidate
144+
.chars()
145+
.all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-'))
146+
}
133147
/// A toolchain name from user input.
134148
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
135149
pub(crate) enum ResolvableToolchainName {
@@ -260,7 +274,7 @@ pub enum ToolchainName {
260274
impl ToolchainName {
261275
/// If the string is already resolved, allow direct conversion
262276
fn validate(candidate: &str) -> Result<Self, InvalidName> {
263-
let candidate = validate(candidate)?;
277+
let candidate = validate_named_toolchain(candidate)?;
264278
candidate
265279
.parse::<ToolchainDesc>()
266280
.map(ToolchainName::Official)
@@ -307,8 +321,13 @@ impl ResolvableLocalToolchainName {
307321
ResolvableToolchainName::try_from(candidate)
308322
.map(ResolvableLocalToolchainName::Named)
309323
.or_else(|_| {
310-
PathBasedToolchainName::try_from(&PathBuf::from(candidate) as &Path)
311-
.map(ResolvableLocalToolchainName::Path)
324+
let path = PathBuf::from(candidate);
325+
if candidate.contains('/') || candidate.contains('\\') {
326+
PathBasedToolchainName::try_from(&path as &Path)
327+
.map(ResolvableLocalToolchainName::Path)
328+
} else {
329+
Err(InvalidName::ToolchainName(candidate.into()))
330+
}
312331
})
313332
}
314333
}
@@ -390,6 +409,7 @@ impl CustomToolchainName {
390409
|| candidate == "none"
391410
|| candidate.contains('/')
392411
|| candidate.contains('\\')
412+
|| !is_legal_named_toolchain(candidate)
393413
{
394414
Err(InvalidName::CustomName(candidate.into()))
395415
} else {
@@ -469,35 +489,6 @@ impl Deref for PathBasedToolchainName {
469489
}
470490
}
471491

472-
fn validate_named_toolchain(candidate: &str) -> Result<&str, InvalidName> {
473-
let candidate = validate(candidate)?;
474-
if is_legal_named_toolchain(candidate) {
475-
Ok(candidate)
476-
} else {
477-
Err(InvalidName::ToolchainName(candidate.into()))
478-
}
479-
}
480-
481-
/// Common validate rules for all sorts of toolchain names
482-
fn validate(candidate: &str) -> Result<&str, InvalidName> {
483-
if let Some(without_plus) = candidate.strip_prefix('+') {
484-
return Err(InvalidName::PlusPrefix(without_plus.to_string()));
485-
}
486-
let normalized_name = candidate.trim_end_matches('/');
487-
if normalized_name.is_empty() {
488-
Err(InvalidName::ToolchainName(candidate.into()))
489-
} else {
490-
Ok(normalized_name)
491-
}
492-
}
493-
494-
fn is_legal_named_toolchain(candidate: &str) -> bool {
495-
!matches!(candidate, "." | "..")
496-
&& candidate
497-
.chars()
498-
.all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-'))
499-
}
500-
501492
#[cfg(test)]
502493
mod tests {
503494
use std::str::FromStr;
@@ -515,13 +506,21 @@ mod tests {
515506
fn partial_toolchain_desc_regex() -> String {
516507
let tuple_regex = format!(
517508
r"(-({}))?(?:-({}))?(?:-({}))?",
518-
LIST_ARCHS.join("|"),
519-
LIST_OSES.join("|"),
520-
LIST_ENVS.join("|")
509+
regex_alternates(LIST_ARCHS),
510+
regex_alternates(LIST_OSES),
511+
regex_alternates(LIST_ENVS)
521512
);
522513
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
523514
}
524515

516+
fn regex_alternates(values: &[&str]) -> String {
517+
values
518+
.iter()
519+
.map(|value| regex::escape(value))
520+
.collect::<Vec<_>>()
521+
.join("|")
522+
}
523+
525524
prop_compose! {
526525
fn arb_partial_toolchain_desc()
527526
(s in string_regex(&partial_toolchain_desc_regex()).unwrap()) -> String {
@@ -531,9 +530,8 @@ mod tests {
531530

532531
prop_compose! {
533532
fn arb_custom_name()
534-
(s in r"[^\\/+][^\\/]*") -> String {
533+
(s in r"[A-Za-z0-9._-]+") -> String {
535534
// perhaps need to filter 'none' and partial toolchains - but they won't typically be generated anyway.
536-
// Also filter '+' prefix as that's reserved for +toolchain syntax.
537535
s
538536
}
539537
}
@@ -566,11 +564,18 @@ mod tests {
566564

567565
#[test]
568566
fn test_parse_custom(name in arb_custom_name()) {
567+
prop_assume!(name != "none");
568+
prop_assume!(name != ".");
569+
prop_assume!(name != "..");
570+
prop_assume!(PartialToolchainDesc::from_str(&name).is_err());
569571
CustomToolchainName::try_from(name).unwrap();
570572
}
571573

572574
#[test]
573575
fn test_parse_resolvable_name(name in arb_resolvable_name()) {
576+
prop_assume!(name != "none");
577+
prop_assume!(name != ".");
578+
prop_assume!(name != "..");
574579
ResolvableToolchainName::try_from(name).unwrap();
575580
}
576581

@@ -602,10 +607,10 @@ mod tests {
602607
"1.8.0-x86_64-apple-darwin",
603608
"1.8.0-x86_64-unknown-linux-gnu",
604609
"1.10.0-x86_64-unknown-linux-gnu",
605-
"bar(baz)",
606-
"foo#bar",
607-
"the cake is a lie",
608-
"this.is.not-a+semver",
610+
"bar.baz",
611+
"foo_bar",
612+
"stage1-local",
613+
"this.is.not-a_semver",
609614
]
610615
.into_iter()
611616
.map(|s| ToolchainName::try_from(s).unwrap())
@@ -626,11 +631,11 @@ mod tests {
626631
"1.8.0-beta-x86_64-apple-darwin",
627632
"1.8.0-beta.2-x86_64-apple-darwin",
628633
// https://github.com/rust-lang/rustup/issues/3517
629-
"foo#bar",
630-
"bar(baz)",
631-
"this.is.not-a+semver",
634+
"foo_bar",
635+
"bar.baz",
636+
"this.is.not-a_semver",
632637
// https://github.com/rust-lang/rustup/issues/3168
633-
"the cake is a lie",
638+
"stage1-local",
634639
]
635640
.into_iter()
636641
.map(|s| ToolchainName::try_from(s).unwrap())
@@ -640,4 +645,21 @@ mod tests {
640645

641646
assert_eq!(expected, v);
642647
}
648+
649+
#[test]
650+
fn custom_names_reject_special_characters() {
651+
for name in [
652+
"bar(baz)",
653+
"foo#bar",
654+
"the cake is a lie",
655+
"this.is.not-a+semver",
656+
"quote'toolchain",
657+
".",
658+
"..",
659+
] {
660+
CustomToolchainName::try_from(name).unwrap_err();
661+
ResolvableToolchainName::try_from(name).unwrap_err();
662+
ToolchainName::try_from(name).unwrap_err();
663+
}
664+
}
643665
}

‎tests/suite/cli_misc.rs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,24 @@ error:[..] invalid custom toolchain name 'beta'
9898
...
9999
error:[..] invalid custom toolchain name 'stable'
100100
...
101+
"#]])
102+
.is_err();
103+
cx.config
104+
.expect(["rustup", "toolchain", "link", "bad name", "foo"])
105+
.await
106+
.with_stderr(snapbox::str![[r#"
107+
...
108+
error:[..] invalid custom toolchain name 'bad name'
109+
...
110+
"#]])
111+
.is_err();
112+
cx.config
113+
.expect(["rustup", "toolchain", "link", "foo#bar", "foo"])
114+
.await
115+
.with_stderr(snapbox::str![[r#"
116+
...
117+
error:[..] invalid custom toolchain name 'foo#bar'
118+
...
101119
"#]])
102120
.is_err();
103121
}

‎tests/suite/cli_rustup.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3747,7 +3747,7 @@ async fn non_utf8_toolchain() {
37473747
.await
37483748
.with_stderr(snapbox::str![[r#"
37493749
...
3750-
error: toolchain '�(' is not installed
3750+
error: invalid toolchain name '�('
37513751
...
37523752
"#]]);
37533753
}
@@ -3774,7 +3774,7 @@ async fn non_utf8_toolchain() {
37743774
.await
37753775
.with_stderr(snapbox::str![[r#"
37763776
...
3777-
error: toolchain '��' is not installed
3777+
error: invalid toolchain name '��'
37783778
...
37793779
"#]]);
37803780
}

0 commit comments

Comments
 (0)