Skip to content

Remove insecure API from uv-fastid #19951

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
woodruffw merged 1 commit into from
Jun 22, 2026
Merged

Remove insecure API from uv-fastid #19951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions crates/uv-errors/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,6 @@ thiserror = { workspace = true }

[lints]
workspace = true

[lib]
doctest = false
Comment on lines +28 to +29

@woodruffw woodruffw Jun 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

cargo shear was complaining about this locally, not sure why it was passing in CI though.

(Not related to the diff otherwise.)

@konstin konstin Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are you using the same version as CI? We may need to update either of the two.

@woodruffw woodruffw Jun 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was on 1.11.2 locally, just upgraded to 1.13.1 and the error disappeared. But it looks like this might have been a true positive, since uv-errors really doesn't contain any doctests.

@konstin konstin Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

iirc we added doctest = false in a bunch of places for test performance, but for cargo shear the 1.13.1 behavior sounds correct, as doctest shouldn't add any new dependencies.

1 change: 0 additions & 1 deletion crates/uv-fastid/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ doctest = false

[dependencies]
rand = { workspace = true }
fastrand = { workspace = true }
serde = { workspace = true, optional = true }

[dev-dependencies]
Expand Down
24 changes: 1 addition & 23 deletions crates/uv-fastid/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ impl Deref for Id {

#[inline]
fn deref(&self) -> &str {
// SAFETY: every `Id` constructor (`secure`, `insecure`, `FromStr`, and
// SAFETY: every `Id` constructor (`secure`, `FromStr`, and
// transitively `serde::Deserialize`) guarantees the bytes are drawn from
// `ALPHABET`, which is entirely ASCII. The bytes are therefore valid UTF-8.
#[allow(unsafe_code)]
Expand Down Expand Up @@ -85,16 +85,6 @@ impl Id {
Self(Self::reduce(&raw))
}

/// Generate an [`Id`] from a fast, non-cryptographically secure RNG.
///
/// The resulting ID is suitable for use in contexts where uniqueness is needed
/// but the risk of an adversarial collision is negligible (such as local cache keys).
pub fn insecure() -> Self {
let mut raw = [0u8; 16];
fastrand::fill(&mut raw);
Self(Self::reduce(&raw))
}

/// Reduce a 16-byte array of random bytes to a 16-byte array of ASCII characters in our ID alphabet.
fn reduce(raw: &[u8; 16]) -> [u8; 16] {
raw.map(|byte| ALPHABET[(byte & MASK) as usize])
Expand Down Expand Up @@ -138,13 +128,6 @@ mod tests {
assert!(id.bytes().all(|byte| ALPHABET.contains(&byte)));
}

#[test]
fn test_insecure() {
let id = Id::insecure();
assert_eq!(id.len(), 16);
assert!(id.bytes().all(|byte| ALPHABET.contains(&byte)));
}

#[test]
fn test_from_str() {
for tc in ["", "short", "0123456789abcdefg", "invalid!"] {
Expand All @@ -167,11 +150,6 @@ mod tests {
let json = serde_json::to_string(&id).unwrap();
let parsed: Id = serde_json::from_str(&json).unwrap();
assert_eq!(id, parsed);

let id = Id::insecure();
let json = serde_json::to_string(&id).unwrap();
let parsed: Id = serde_json::from_str(&json).unwrap();
assert_eq!(id, parsed);
}

#[test]
Expand Down