110 lines
4.7 KiB
Plaintext
110 lines
4.7 KiB
Plaintext
----- BEGIN CREV PROOF -----
|
|
kind: package review
|
|
version: -1
|
|
date: 2022-09-19T17:10:02.580323647+02:00
|
|
from:
|
|
id-type: crev
|
|
id: YdnEoYtqvbBGv0hhENLDUYmc3tNfm5V5NIG5hCovHyM
|
|
url: https://git.jcg.re/jcgruenhage/crev-proofs.git
|
|
package:
|
|
source: https://crates.io
|
|
name: atty
|
|
version: 0.2.14
|
|
revision: 7b5df17888997d57c2c1c8f91da1db5691f49953
|
|
digest: WJaqS5xu6ORo2px2Rku1ynYYN4TuCdxeS8LR68ngyw4
|
|
review:
|
|
thoroughness: high
|
|
understanding: high
|
|
rating: positive
|
|
flags:
|
|
unmaintained: true
|
|
alternatives:
|
|
- source: https://crates.io
|
|
name: is-terminal
|
|
comment: |-
|
|
One of the more foundational crates found in the dependency tree of a lot of
|
|
rust programs, because both clap and env_logger pull this in.
|
|
|
|
In my review I've fully read the source code and can confirm that I fully
|
|
understand what's happening in here. The unix and hermit targets are
|
|
extremely straight-forward. As for the windows target, that's a bit more
|
|
complicated, but still manageable in the end. Windows doesn't have a clear
|
|
API for determining whether something is a (pseudo) TTY, so the heuristics
|
|
provided by this crate are as good as it's going to get.
|
|
|
|
This crate has quite a few unsafe code sections, but that's sometimes
|
|
required for providing a safe interface. In this case, we need it because the
|
|
underlying functions for unix (libc) are unsafe, and the same applies to a
|
|
bunch of winapi functions used in the heuristics for windows.
|
|
|
|
The bits of unsafe code that's not just wrapping an unsafe function provided
|
|
by another library are all in the windows heuristics, and involved
|
|
provisioning buffers that winapi calls can write info back into and some
|
|
pointer magic. While I couldn't spot an issue with this, a look into the issue
|
|
tracker revealed that other's have. The buffer creation on the heap is not
|
|
necessarily aligned properly, meaning that there's a possible soundness issue
|
|
on windows targets here.
|
|
|
|
Last but not least, we have to talk about maintenance and and alternatives:
|
|
The soundness issue mentioned above has been known for over a year, with a fix
|
|
first pushed to a PR shortly after. Even though there's been reviews from
|
|
well-known rustaceans, the author hasn't merged this PR yet, and in general
|
|
there hasn't been any relevant activity for a while.
|
|
|
|
An alternative implementation that has been derived from `atty` is
|
|
`is-terminal`, which has taken this into consideration and has taken over the
|
|
rest of the implementation from here, with a slightly different API. They're
|
|
also switching the underlying implementations around, reducing the amount of
|
|
unsafe.
|
|
|
|
As for why I'm still rating this as positive: With the exception of the
|
|
potential unsoundness bug on windows, this crate is still okay-ish to be used.
|
|
Also: Getting rid of it soon is not realistic, because it's in the dependency
|
|
tree of quite a few widely-used crates.
|
|
|
|
To summarize: Widely used, foundational crate. Except for on windows, this is
|
|
perfectly fine, on windows there's an unsoundness bug, and there has not been
|
|
an update or other activity from upstream. `is-terminal` is a good
|
|
alternative, which was derived from this crate, but includes the fix for the
|
|
unsoundess bug.
|
|
----- SIGN CREV PROOF -----
|
|
mY2ODgsozEL4os5aFk0hxL8jH5SHu7awErNdlLHY9UhDL-vLdbDjEIYiD0wFPaTTglTxirK41Jwtokpm4rWyDg
|
|
----- END CREV PROOF -----
|
|
|
|
----- BEGIN CREV PROOF -----
|
|
kind: package review
|
|
version: -1
|
|
date: 2022-09-19T17:10:17.861223964+02:00
|
|
from:
|
|
id-type: crev
|
|
id: YdnEoYtqvbBGv0hhENLDUYmc3tNfm5V5NIG5hCovHyM
|
|
url: https://git.jcg.re/jcgruenhage/crev-proofs.git
|
|
package:
|
|
source: https://crates.io
|
|
name: is-terminal
|
|
version: 0.3.0
|
|
revision: 275136008c33722a43724be5a9711af94afa8476
|
|
digest: VtFAGa-8B5YOh1SOZHNPgb9KCSb2fcmOUOhnzRqyHKI
|
|
review:
|
|
thoroughness: high
|
|
understanding: high
|
|
rating: positive
|
|
comment: |-
|
|
Following up my review of atty, this is the alternative that I'm recommending.
|
|
The crate is well-maintained and has the soundness problem from atty fixed.
|
|
The dependencies have been changed around a bit, with rustix replacing libc,
|
|
and windows-sys replacing win-api. This means that there's a bit less unsafe
|
|
code in the library here as well.
|
|
|
|
The API surface changed a bit as well, but I'd claim that the new API is more
|
|
sensible and allows usage on any file descriptor instead of just
|
|
stdin/stdout/stderr.
|
|
|
|
Finally, this has one additional dependency, `io-lifetimes`, by the same
|
|
author. I have not reviewed the implementation, but it's been merged into Rust
|
|
itself and stabilized with 1.63, so I am not worrying about it too much.
|
|
----- SIGN CREV PROOF -----
|
|
qermifHP4gqqELsV3vdWnIq0b42d0bAPKqVdQQXzc8n2DTpgIZYBnEfjuDFM88O7sWbpAFqHeJ9v2Z2NUnf7BQ
|
|
----- END CREV PROOF -----
|
|
|