Properly validate crate names in `cargo install` #16314

20jasper · 2025-12-01T00:03:07Z

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

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

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

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

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

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

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?

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

---- 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 }

rustbot · 2025-12-01T00:03:12Z

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

20jasper · 2025-12-01T00:06:19Z

If you see any weird characters, vim seems to have mangled some of the characters when I was making a PR. I think I fixed them all, but just a heads up

For example, – turned into ΓÇô

20jasper · 2025-12-01T00:08:02Z

src/bin/cargo/commands/install.rs

            }
        }
+
+        if crate_name != "."


. is special cased later to warn that cargo install . is deprecated

20jasper · 2025-12-01T00:33:53Z

tests/testsuite/install.rs

+}
+
+#[cargo_test]
+fn feature_name_dep_colon_case() {


Some cases like this already behaved correctly, but I didn't see any test coverage for them

This is covered in

cargo/tests/testsuite/features_namespaced.rs

Lines 525 to 541 in ebd20d2

p.cargo("check --features dep:bar")

.with_status(101)

.with_stderr_data(str![[r#"

[ERROR] feature `dep:bar` is not allowed to use explicit `dep:` syntax

"#]])

.run();

switch_to_resolver_2(&p);

p.cargo("check --features dep:bar")

.with_status(101)

.with_stderr_data(str![[r#"

[ERROR] feature `dep:bar` is not allowed to use explicit `dep:` syntax

"#]])

.run();

}

weihanglo

Thanks for the contribution!

View changes since this review

weihanglo · 2025-12-01T01:06:39Z

tests/testsuite/install.rs

FWIW, for bugfixes, we usually follow a variant of atomic commit pattern:

Commit a test that asserts the current buggy behavior (passes).

In the next commit, fix the bug and update the test/snapshot.

Every commit passes, and the test/snapshot diff shows the behavior change.

Would you mind rearranging your commit history to follow that?

@weihanglo just pushed up the changes

Thanks for the update!

weihanglo · 2025-12-01T01:23:18Z

tests/testsuite/install.rs

+}
+
+#[cargo_test]
+fn feature_name_dep_colon_case() {


This is covered in

cargo/tests/testsuite/features_namespaced.rs

Lines 525 to 541 in ebd20d2

p.cargo("check --features dep:bar")

.with_status(101)

.with_stderr_data(str![[r#"

[ERROR] feature `dep:bar` is not allowed to use explicit `dep:` syntax

"#]])

.run();

switch_to_resolver_2(&p);

p.cargo("check --features dep:bar")

.with_status(101)

.with_stderr_data(str![[r#"

[ERROR] feature `dep:bar` is not allowed to use explicit `dep:` syntax

"#]])

.run();

}

weihanglo · 2025-12-01T14:58:02Z

tests/testsuite/install.rs

Thanks for the update!

Update cargo submodule 14 commits in 2a7c4960677971f88458b0f8b461a866836dff59..bd979347d814dfe03bba124165dbce9554d0b4d8 2025-11-25 19:58:07 +0000 to 2025-12-02 16:03:50 +0000 - fix(completion): Put host-tuple before actual tuples (rust-lang/cargo#16327) - fix(lints): use plural form correctly (rust-lang/cargo#16324) - fix(completions): include `all` in `cargo tree --target` candidates (rust-lang/cargo#16322) - fix(lints): show lint error number (rust-lang/cargo#16320) - chore(deps): update compatible (rust-lang/cargo#16318) - chore(deps): update crate-ci/typos action to v1.40.0 (rust-lang/cargo#16316) - Do not lock the artifact-dir for check builds + fix uplifting (rust-lang/cargo#16307) - Properly validate crate names in `cargo install` (rust-lang/cargo#16314) - Support --filter-platform=host for cargo metadata rust-lang/cargo#9423 (rust-lang/cargo#16312) - Update to mdbook 0.5 (rust-lang/cargo#16292) - refactor(clean): Better divide old / new layout (rust-lang/cargo#16304) - update: silent failure on non-matching package specs with --breaking (rust-lang/cargo#16276) - fix(log): break timing-info message to multiple (rust-lang/cargo#16303) - fix(clean): Clean hosts builds with new layout (rust-lang/cargo#16300)

rustbot assigned weihanglo Dec 1, 2025

rustbot added A-cli Area: Command-line interface, option parsing, etc. Command-install S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 1, 2025

20jasper changed the title ~~Return error for all invalid packages names for cargo install~~ Properly validate crate names in cargo install Dec 1, 2025

20jasper commented Dec 1, 2025

View reviewed changes

weihanglo reviewed Dec 1, 2025

View reviewed changes

Add tests for erroneous searches and out of bounds indexing

4b808eb

20jasper force-pushed the invalid-name-causes-out-of-bounds-indexing branch from cfda2d9 to 805c6fb Compare December 1, 2025 04:19

Fix erroneous searches with cargo install and out of bounds indexing

805c6fb

weihanglo approved these changes Dec 1, 2025

View reviewed changes

tests/testsuite/install.rs

Copy link

Member

weihanglo Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update!

20jasper reacted with heart emoji

weihanglo added this pull request to the merge queue Dec 1, 2025

Merged via the queue into rust-lang:master with commit a3b26d6 Dec 1, 2025
26 checks passed

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 1, 2025

weihanglo mentioned this pull request Dec 2, 2025

Update cargo submodule rust-lang/rust#149555

Merged

rustbot added this to the 1.93.0 milestone Dec 3, 2025

	p.cargo("check --features dep:bar")
	.with_status(101)
	.with_stderr_data(str![[r#"
	[ERROR] feature `dep:bar` is not allowed to use explicit `dep:` syntax

	"#]])
	.run();

	switch_to_resolver_2(&p);
	p.cargo("check --features dep:bar")
	.with_status(101)
	.with_stderr_data(str![[r#"
	[ERROR] feature `dep:bar` is not allowed to use explicit `dep:` syntax

	"#]])
	.run();
	}

Properly validate crate names in cargo install #16314

Properly validate crate names in cargo install #16314

Conversation

20jasper commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR try to resolve?

Out of bounds indexing

Erroneous searches

Reserved names behavior preseved

How to test and review this PR?

Potential Followup

Concerns

Uh oh!

rustbot commented Dec 1, 2025

Uh oh!

20jasper commented Dec 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weihanglo left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Properly validate crate names in `cargo install` #16314

Properly validate crate names in `cargo install` #16314

20jasper commented Dec 1, 2025 •

edited

Loading

weihanglo left a comment •

edited by rustbot

Loading