checksec.rs icon indicating copy to clipboard operation
checksec.rs copied to clipboard

Support color output as a run-time option instead of compile-time configuration?

Open tnballo opened this issue 5 years ago • 4 comments

This issue is for an extremely specific quirk that isn’t really a problem in practice, but bringing it to your attention on the off chance other library API users ran into the same thing.

Currently use this crate as a dependency in a CLI tool, the tool accepts a --no-color flag to turn off colored output. Original rationale was to support UNIX piping, assuming ANSI escape sequences would cause issues. But it turns out the colored crate uses the atty crate to automatically detect when a file descriptor isn’t a terminal and disable coloring. This means checksec has no problem at all with UNIX piping if colored output is enabled at compile-time.

Regardless, I’m slightly annoyed that I can’t get checksec to toggle color at runtime based on a parameter. It’s technically possible if I re-implement all formatting rules for every field member using the “new type pattern” (which I already half do here to get multi-line output), but that’s a bit painful.

Do you think it’s worth supporting coloring as a run-time option instead of compile-time configuration?

tnballo avatar Nov 11 '20 19:11 tnballo

Agreed the current mechanisms for toggling colorized output is non-ideal. I will look at supporting run-time color configuration in in a future release.

etke avatar Nov 20 '20 05:11 etke

Wanted to provide an update on this issue (after a long while!).

Found a suitable workaround for my purposes: using checksec with the color feature enabled and then supporting a --no-color flag by leveraging the strip-ansi-escapes crate to optionally "uncolor" at runtime.

One way to implement runtime color toggling in checksec directly might be:

  • Adding a color: bool field to structs like CheckSecResults and doing match self.color in the Display trait implementation.

  • Enums like ASLR could similarly store the bool in each variant (e.g. DynamicBase { color: bool }, etc) and also do Display matching.

  • Having the existing color feature control a default setting and adding a set_color(enable: bool) where appropriate to allow runtime override of the feature-selected default.

Happy to work on this and create a PR, but afraid that the tradeoff in additional code complexity wouldn't be worth it for the majority of checksec users. So also happy to stick with the workaround and close this issue. Let me know.

tnballo avatar Apr 28 '22 22:04 tnballo

Long over-due but should be resolved with https://github.com/etke/checksec.rs/commit/4033e5b95f27c25e9556dfc378eb653087b7ccc2

etke avatar Jun 26 '22 20:06 etke

Awesome, thank you! 👍

tnballo avatar Jun 26 '22 23:06 tnballo

This issue is resolved isn't it? You might want to close it.

titison avatar Nov 11 '23 20:11 titison