Skip to content

Commit 7aa4c97

Browse files
cachebagcachebag
committed
fix(toolchain): restrict named toolchain characters
1 parent 4764169 commit 7aa4c97

3 files changed

Lines changed: 91 additions & 20 deletions

File tree

‎src/toolchain/names.rs‎

Lines changed: 71 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,15 @@ 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+
}
128+
120129
/// Common validate rules for all sorts of toolchain names
121130
fn validate(candidate: &str) -> Result<&str, InvalidName> {
122131
if let Some(without_plus) = candidate.strip_prefix('+') {
@@ -130,6 +139,13 @@ fn validate(candidate: &str) -> Result<&str, InvalidName> {
130139
}
131140
}
132141

142+
fn is_legal_named_toolchain(candidate: &str) -> bool {
143+
!matches!(candidate, "." | "..")
144+
&& candidate
145+
.chars()
146+
.all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-'))
147+
}
148+
133149
/// A toolchain name from user input.
134150
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
135151
pub(crate) enum ResolvableToolchainName {
@@ -152,7 +168,7 @@ impl ResolvableToolchainName {
152168
// If candidate could be resolved, return a ready to resolve version of it.
153169
// Otherwise error.
154170
fn validate(candidate: &str) -> Result<ResolvableToolchainName, InvalidName> {
155-
let candidate = validate(candidate)?;
171+
let candidate = validate_named_toolchain(candidate)?;
156172
candidate
157173
.parse::<PartialToolchainDesc>()
158174
.map(ResolvableToolchainName::Official)
@@ -226,7 +242,7 @@ pub(crate) enum MaybeOfficialToolchainName {
226242

227243
impl MaybeOfficialToolchainName {
228244
fn validate(candidate: &str) -> Result<MaybeOfficialToolchainName, InvalidName> {
229-
let candidate = validate(candidate)?;
245+
let candidate = validate_named_toolchain(candidate)?;
230246
if candidate == "none" {
231247
Ok(MaybeOfficialToolchainName::None)
232248
} else {
@@ -262,7 +278,7 @@ pub enum ToolchainName {
262278
impl ToolchainName {
263279
/// If the string is already resolved, allow direct conversion
264280
fn validate(candidate: &str) -> Result<Self, InvalidName> {
265-
let candidate = validate(candidate)?;
281+
let candidate = validate_named_toolchain(candidate)?;
266282
candidate
267283
.parse::<ToolchainDesc>()
268284
.map(ToolchainName::Official)
@@ -311,8 +327,13 @@ impl ResolvableLocalToolchainName {
311327
ResolvableToolchainName::try_from(candidate)
312328
.map(ResolvableLocalToolchainName::Named)
313329
.or_else(|_| {
314-
PathBasedToolchainName::try_from(&PathBuf::from(candidate) as &Path)
315-
.map(ResolvableLocalToolchainName::Path)
330+
let path = PathBuf::from(candidate);
331+
if candidate.contains('/') || candidate.contains('\\') {
332+
PathBasedToolchainName::try_from(&path as &Path)
333+
.map(ResolvableLocalToolchainName::Path)
334+
} else {
335+
Err(InvalidName::ToolchainName(candidate.into()))
336+
}
316337
})
317338
}
318339
}
@@ -394,6 +415,7 @@ impl CustomToolchainName {
394415
|| candidate == "none"
395416
|| candidate.contains('/')
396417
|| candidate.contains('\\')
418+
|| !is_legal_named_toolchain(candidate)
397419
{
398420
Err(InvalidName::CustomName(candidate.into()))
399421
} else {
@@ -490,13 +512,21 @@ mod tests {
490512
fn partial_toolchain_desc_regex() -> String {
491513
let tuple_regex = format!(
492514
r"(-({}))?(?:-({}))?(?:-({}))?",
493-
LIST_ARCHS.join("|"),
494-
LIST_OSES.join("|"),
495-
LIST_ENVS.join("|")
515+
regex_alternates(LIST_ARCHS),
516+
regex_alternates(LIST_OSES),
517+
regex_alternates(LIST_ENVS)
496518
);
497519
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
498520
}
499521

522+
fn regex_alternates(values: &[&str]) -> String {
523+
values
524+
.iter()
525+
.map(|value| regex::escape(value))
526+
.collect::<Vec<_>>()
527+
.join("|")
528+
}
529+
500530
prop_compose! {
501531
fn arb_partial_toolchain_desc()
502532
(s in string_regex(&partial_toolchain_desc_regex()).unwrap()) -> String {
@@ -506,9 +536,8 @@ mod tests {
506536

507537
prop_compose! {
508538
fn arb_custom_name()
509-
(s in r"[^\\/+][^\\/]*") -> String {
539+
(s in r"[A-Za-z0-9._-]+") -> String {
510540
// perhaps need to filter 'none' and partial toolchains - but they won't typically be generated anyway.
511-
// Also filter '+' prefix as that's reserved for +toolchain syntax.
512541
s
513542
}
514543
}
@@ -541,11 +570,18 @@ mod tests {
541570

542571
#[test]
543572
fn test_parse_custom(name in arb_custom_name()) {
573+
prop_assume!(name != "none");
574+
prop_assume!(name != ".");
575+
prop_assume!(name != "..");
576+
prop_assume!(PartialToolchainDesc::from_str(&name).is_err());
544577
CustomToolchainName::try_from(name).unwrap();
545578
}
546579

547580
#[test]
548581
fn test_parse_resolvable_name(name in arb_resolvable_name()) {
582+
prop_assume!(name != "none");
583+
prop_assume!(name != ".");
584+
prop_assume!(name != "..");
549585
ResolvableToolchainName::try_from(name).unwrap();
550586
}
551587

@@ -577,10 +613,10 @@ mod tests {
577613
"1.8.0-x86_64-apple-darwin",
578614
"1.8.0-x86_64-unknown-linux-gnu",
579615
"1.10.0-x86_64-unknown-linux-gnu",
580-
"bar(baz)",
581-
"foo#bar",
582-
"the cake is a lie",
583-
"this.is.not-a+semver",
616+
"bar.baz",
617+
"foo_bar",
618+
"stage1-local",
619+
"this.is.not-a_semver",
584620
]
585621
.into_iter()
586622
.map(|s| ToolchainName::try_from(s).unwrap())
@@ -601,11 +637,11 @@ mod tests {
601637
"1.8.0-beta-x86_64-apple-darwin",
602638
"1.8.0-beta.2-x86_64-apple-darwin",
603639
// https://github.com/rust-lang/rustup/issues/3517
604-
"foo#bar",
605-
"bar(baz)",
606-
"this.is.not-a+semver",
640+
"foo_bar",
641+
"bar.baz",
642+
"this.is.not-a_semver",
607643
// https://github.com/rust-lang/rustup/issues/3168
608-
"the cake is a lie",
644+
"stage1-local",
609645
]
610646
.into_iter()
611647
.map(|s| ToolchainName::try_from(s).unwrap())
@@ -615,4 +651,21 @@ mod tests {
615651

616652
assert_eq!(expected, v);
617653
}
654+
655+
#[test]
656+
fn custom_names_reject_special_characters() {
657+
for name in [
658+
"bar(baz)",
659+
"foo#bar",
660+
"the cake is a lie",
661+
"this.is.not-a+semver",
662+
"quote'toolchain",
663+
".",
664+
"..",
665+
] {
666+
CustomToolchainName::try_from(name).unwrap_err();
667+
ResolvableToolchainName::try_from(name).unwrap_err();
668+
ToolchainName::try_from(name).unwrap_err();
669+
}
670+
}
618671
}

‎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)