Commit a3b26d6
authored
Properly validate crate names in
### What does this PR try to resolve?
`cargo install` did not properly validate package names it received
causing
- out of bounds indexing
- performing erroneous searches
#### Out of bounds indexing
When `cargo install` is passed a string starting with a character with
more than 2 bytes, it would index out of bounds and panic
This often manifests itself through copying from another program that
changes the characters or typing on mobile. For example, the below error
is caused by using an en dash instead of a dash, so it's interpreted as
a crate name since clap does not parse it as a flag. The code that
panics assumes it will only receive ascii string slices, the cargo
install code does validate naming errors, but it swallows errors
regarding invalid characters
```rs
cargo install ––path .
thread 'main' (335494) panicked at src/tools/cargo/crates/cargo-util/src/registry.rs:24:44:
byte index 2 is not a char boundary; it is inside '–' (bytes 0..3) of `––path`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
After my changes, this is the new result
```rs
cargo run -q -- install ––path .
error: invalid character `–` in package name: `––path`, the first character must be a Unicode XID start character (most letters or `_`)
```
#### Erroneous searches
The registry is erroneously searched for other invalid package names
when attempting to install
```rs
cargo install 2382399823
Updating crates.io index
error: could not find `2382399823` in registry `crates-io` with version `*`
cargo install !
Updating crates.io index
error: could not find `!` in registry `crates-io` with version `*`
```
After my changes, it bails before searching
```rs
cargo run -q -- install 2382399823
error: invalid character `2` in package name: `2382399823`, the name cannot start with a digit
cargo run -q -- install !
error: invalid character `!` in package name: `!`, the first character must be a Unicode XID start character (most letters or `_`)
```
#### Reserved names behavior preseved
These changes continue to allow installing reserved names such as `nul`
and `std`
```rs
cargo run -q -- install std
Updating crates.io index
error: could not find `std` in registry `crates-io` with version `*`
```
### How to test and review this PR?
Run `cargo install` with values that do not conform to the [package name
identifier
specifications](https://doc.rust-lang.org/cargo/reference/manifest.html#the-name-field)
### Potential Followup
I'd argue the error message could be improved with a help diagnostic
like the following. Would this be something y'all would be interested
in? If so, would there be a more general place to provide this
diagnostic, i.e. are there places name validations are done in a non-cli
context?
```rs
cargo run -q -- install ––path .
error: invalid character `–` in package name: `––path`, the first character must be a Unicode XID start character (most letters or `_`)
help: did you mean to make this a flag? found dashlike character `–` in package name `––path`. consider inserting `-` like so `--path`
```
### Concerns
EDIT: CI passes, so likely just a setup issue
There were around 90 tests that failed even when running tests from
master. I figure this is an issue with my own setup. All tests in the
area I worked in were passing
```rs
---- cargo_remove::target::case stdout ----
thread 'cargo_remove::target::case' (728642) panicked at /home/jacob/coding/rust/src/tools/cargo/crates/cargo-test-support/src/lib.rs:373:76:
called `Result::unwrap()` on an `Err` value: Error { inner: "Failed to copy /home/jacob/coding/rust/src/tools/cargo/tests/testsuite/cargo_remove/target/in to /home/jacob/coding/rust/src/tools/cargo/target/tmp/cit/t1067/case/: Is a directory (os error 21)", backtrace: None }
---- cargo_remove::target_dev::case stdout ----
thread 'cargo_remove::target_dev::case' (728654) panicked at /home/jacob/coding/rust/src/tools/cargo/crates/cargo-test-support/src/lib.rs:373:76:
called `Result::unwrap()` on an `Err` value: Error { inner: "Failed to copy /home/jacob/coding/rust/src/tools/cargo/tests/testsuite/cargo_remove/target_dev/in to /home/jacob/coding/rust/src/tools/cargo/target/tmp/cit/t1069/case/: Is a directory (os error 21)", backtrace: None }
```cargo install (#16314)2 files changed
+45
-0
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
167 | 167 | | |
168 | 168 | | |
169 | 169 | | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
170 | 176 | | |
171 | 177 | | |
172 | 178 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3068 | 3068 | | |
3069 | 3069 | | |
3070 | 3070 | | |
| 3071 | + | |
| 3072 | + | |
| 3073 | + | |
| 3074 | + | |
| 3075 | + | |
| 3076 | + | |
| 3077 | + | |
| 3078 | + | |
| 3079 | + | |
| 3080 | + | |
| 3081 | + | |
| 3082 | + | |
| 3083 | + | |
| 3084 | + | |
| 3085 | + | |
| 3086 | + | |
| 3087 | + | |
| 3088 | + | |
| 3089 | + | |
| 3090 | + | |
| 3091 | + | |
| 3092 | + | |
| 3093 | + | |
| 3094 | + | |
| 3095 | + | |
| 3096 | + | |
| 3097 | + | |
| 3098 | + | |
| 3099 | + | |
| 3100 | + | |
| 3101 | + | |
| 3102 | + | |
| 3103 | + | |
| 3104 | + | |
| 3105 | + | |
| 3106 | + | |
| 3107 | + | |
| 3108 | + | |
| 3109 | + | |
0 commit comments