You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
90% of the reason one needs a temporary directory, is for it not to collide when multiple instances of the same process are running. As it is right now, the name collides, because the suffix is not chosen at random, because it is seeded. So what's the point of calculating the suffix in the first place?
On top of it, when reviewing the code I've noticed that the "random" suffix is generated with unsafe? What's the point? I'm creating a directory, so this operation is going to be super slow anyway. Using unsafe in Rust is a no-no, until it's absolutely necessary. In this case, it's just unjustified.
And why is this library using random instead of rand? The randomness is a big part of generating temporary directory. In some use cases it might be very important for the security. Noone should use a custom random number, instead of the standard, well accepted and reviewed one.
I'm sorry if I'm being mistake somewhere, and if I appear unreasonably harsh, but it seems to me, right now this crate is a footgun, and it might put some unaware users into trouble. If there was a way to flag it as "not safe to use" I would totally do it. Generating a tmp directory is not very dificult and I could have done it inline. Instead I used a 3rd party crate, and now I regret it, because I've discovered multiple problems with the code that seems rather simple to do right. :/
The text was updated successfully, but these errors were encountered:
Thanks for the feedback! The crate is primarily used for testing purposes, and it serves well in those scenarios. It is also a part of a family of crates relying on random. I agree that, when security is a concern, it might not be appropriate; I recommend switching to tempdir.
90% of the reason one needs a temporary directory, is for it not to collide when multiple instances of the same process are running. As it is right now, the name collides, because the suffix is not chosen at random, because it is seeded. So what's the point of calculating the suffix in the first place?
On top of it, when reviewing the code I've noticed that the "random" suffix is generated with
unsafe
? What's the point? I'm creating a directory, so this operation is going to be super slow anyway. Usingunsafe
in Rust is a no-no, until it's absolutely necessary. In this case, it's just unjustified.And why is this library using
random
instead ofrand
? The randomness is a big part of generating temporary directory. In some use cases it might be very important for the security. Noone should use a custom random number, instead of the standard, well accepted and reviewed one.I'm sorry if I'm being mistake somewhere, and if I appear unreasonably harsh, but it seems to me, right now this crate is a footgun, and it might put some unaware users into trouble. If there was a way to flag it as "not safe to use" I would totally do it. Generating a tmp directory is not very dificult and I could have done it inline. Instead I used a 3rd party crate, and now I regret it, because I've discovered multiple problems with the code that seems rather simple to do right. :/
The text was updated successfully, but these errors were encountered: