Keats / rust-bcrypt

Easily hash and verify passwords using Bcrypt
MIT License
340 stars 49 forks source link

Make alloc optional #79

Closed mkj closed 1 year ago

mkj commented 1 year ago

This puts all uses of alloc behind cfg, allowing to run on no_std platforms without alloc. It also sets default-features=false for getrandom and zeroize dependencies, getrandom's std or js are already set by features.

This patch is a bit cluttered with the #[cfg]. An alternative would be to move most of libs.rs to a new normal.rs module and then toplevel lib.rs could export those by default, or only bcrypt.rs modules for the no-alloc case. It'd be a bigger change, let me know what you'd prefer.

mkj commented 1 year ago

The plain bcrypt() function works OK, I'm using it on a rp2040 microcontroller. Storing the raw hash/salt rather than the formatted version.

https://github.com/mkj/sunset/blob/a5a62f418900e8d220c29303323cbbd67530b103/embassy/demos/common/src/config.rs#L249

Keats commented 1 year ago

Can you add test setup in CI for running it without that feature enabled?

mkj commented 1 year ago

I've added a small bare metal Arm example to test it. That also checks that getrandom crate isn't getting pulled in since that won't work on a bare metal platforms (without extra getrandom customisation, anyway).

Keats commented 1 year ago

Looks like we need to bump the Rust version to 1.60

mkj commented 1 year ago

Ah yeah, looks like it's unrelated to this PR, just log crate itself updated? https://old.reddit.com/r/rust/comments/12jepss/log_is_going_to_bump_msrv_to_160/

(A pity github delete the logs from the last successful build to compare)

tarcieri commented 1 year ago

FWIW, checking in Cargo.lock can prevent these sorts of random, unrelated regressions.

Related: https://github.com/rust-lang/cargo/issues/9930#issuecomment-1515427835

Keats commented 1 year ago

I kind of like knowing about those tbh

tarcieri commented 1 year ago

@Keats if you enable Dependabot you'll receive PRs to bump Cargo.lock which will show you immediately that dependency updates are breaking MSRV, while keeping PRs green and free of unexpected breakages or having to deal with MSRV bumps

Keats commented 1 year ago

I think the current PR is fine, the only thing missing is a little text in the README to explain what's available if alloc is not enabled

mkj commented 1 year ago

That's a good suggestion. I've added a note to the README now.

Keats commented 1 year ago

Thanks, it's available in 0.15.0