Cleaning up crate features #672
Labels
No labels
breaking
bug
dependencies
good first issue
help wanted
unstable
No milestone
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
gh-uuid-rs/uuid#672
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This is a meta issue to take a fresh look at the library’s dependency tree. In particular:
fast-rngfeature that addsrandas a dependency becausegetrandomis slow on Windows. Is that still necessary?wasm-bindgenandgetrandomunconditionally when building for the web. Could we use our crate features more cleverly to only pull in features we need?macro-diagnosticsfeature that enables a proc-macro for better error reporting of compile-time UUID literals. Is this actually used? Can we emit more meaningful errors without needing a proc-macro on more recent compilers?We can consider a major 2.x version of
uuidthat is semver-compatible with 1.x (no actual API changes and re-export 2.x in 1.x), but that removes or rejigs our crate features as necessary to keep our dependency tree sane.From my understanding, you only need the
jswhen you are compiling to WASM and need therndfeature. (You might need it for atomics too, but the following still holds with a few additions). So, we can change the definition of therndfeature to be:rnd = ["wasm-bindgen", "getrandom"]. Then, the dependencies imports can be gated by target:This also makes the
jsfeature obsolete.That's interesting 🤔 Would that
rndfeature simply ignore thewasm-bindgenandgetrandomfeatures on targets that don't specify them as dependencies?Ya, that's correct. I don't know how this applies to user-selected features (i.e. features that you specify via cargo's
--featureflag), but, for features that enable other features, cargo doesn't care. That or cargo is aware thatwasm-bindgenis a valid feature sometimes, so if it gets specified and is not used that's ok.For
fast-rngwithrandyou would want to useSmallRngbut we can do better: usefastrandinstead. It's considerably lighter-weight thanrand. I can put in a PR.I don't think this is a good idea.
SmallRngandfastranduse predictable RNGs, which means any UUID generated by them would be predictable. This can result in security issues for some UUID v4 applications.PR was already closed, so not pursuing it anymore.
Couldn't it be an issue when using
fast-rngjust for some local high-throughput location? When usingfast-rngas feature, you might use it everywhere by accident. It might even be injected into another authentication crate you are using that is using uuid-v4 for token generation. And then, this authentication crate would also generate uuid-v4 tokens withfast-rngas theuuidversion might fit and features are just considered optional and non destructive.Wouldn't it be better to have explicit fast versions as separate function, separate module or maybe even separate crate?
I just created an own fast-uuid-v7 crate because I didn't knew
fast-rngexisted. But I probably would also avoid such a feature for security reasons.@marcomq We wouldn't make the source of the
fast-rngfeature any less secure than the default. It's a feature that's mostly only useful on Windows, but may not even be necessary there anymore now thatgetrandomappears to be using a different underlying Windows API.