Repository navigation
Conversation
|
(Adding 39 lines worth of test feels like overkill to me.) |
| .expect(["rustup", "toolchain", "list"]) | ||
| .await | ||
| .with_stderr(snapbox::str
There was a problem hiding this comment.
@djc Didn't this introduce a new attack vector? I mean in a similar fashion to #4059 where you can essentially do:
warn: could not check if there is a new Rust release: could not parse release date '
<many random lines so that the above warning becomes hard to find>
<some random output to confuse the user>
warn: your Rust installation is broken
hint: to attempt a fix, please execute
<ACE payload>'
There was a problem hiding this comment.
Hmm, that might be true. We could constrain the length to, say, 60 characters, and make sure it doesn't contain any newlines?
There was a problem hiding this comment.
Good idea! IMHO 15 characters actually would make more sense since it's mostly yyyy-mm-dd.
cc @reidliu41
| Ok(Some(manifest)) => manifest.date, | ||
| Ok(None) | Err(_) => FALLBACK_RELEASE_DATE.to_owned(), | ||
| }, | ||
| Err(_) => FALLBACK_RELEASE_DATE.to_owned(), |
There was a problem hiding this comment.
When the manifest is missing or unreadable, we have FALLBACK_RELEASE_DATE.
Shouldn't we have something similar when there is a parsing error?
So, besides throwing the error, we would call NaiveDate::parse_from_str(FALLBACK_RELEASE_DATE, "%Y-%m-%d").
There was a problem hiding this comment.
On another note, this FALLBACK_RELEASE_DATE should be updated every time there is a new rustup release; although that sounds cumbersome.
When the installed stable manifest contains a date that cannot be parsed, the release hint warning prints the literal
{release_date_str}placeholder:This hides the invalid value and makes damaged or custom manifests harder to diagnose.