Comments (25)
What's the rationale behind fixing soundness issue = major release? That if anything just makes it harder for people to upgrade.
fern
re-exports types directly from colored
when the colored
feature flag is enabled, so removing it as a dependency is a breaking change.
We can do a minor release adding an alternative, but since we currently directly re-export types from colored
, switching away will necessarily require downstream users to change their Cargo.toml and color-using code.
from fern.
I think owo-colors
will work, as long as we can combine it with enable-ansi-support
for colors on Windows 10+ terminals, and is-terminal
or similar to allow toggling colors.
It'll require a redesign of fern's color module, though, so I've not yet written it. Some of what we do now will become unnecessary, and other parts just won't work well with owo-colors
color types. We could just drop the module entirely and let users use owo-colors
themselves, but it would be nice to provide a one-stop solution for log coloring. Or, at the very least, a good example of how to do the "right thing".
In the meantime, I've released an update of fern with a warning for this security issue in the README.md
and lib.rs
documentation, and a temporary workaround. It should show up in https://crates.io/crates/fern and https://docs.rs/fern/
from fern.
Hello everyone, colored will release a new version soon.
We are investigating how to remove the dependency to atty
and its eventual impact downstream. I believe fixing that will help you, right?
Fern has a MSRV of rust 1.35, which makes you our dependant with the largest rust compatibility so I think you should a say in this.
We think we have 3 options:
- Drop atty, use
std::io::isTerminal
, and bump the MSRV to rust 1.70 (June 2023) - Drop atty and provide a dumb default with rustversion, keeping the MSRV around rust 1.31 (December 2018)
- Drop atty and use is-terminal which MSRV is rust 1.62 (August 2022)
What would you prefer ?
Side-question: would you think that we should have a major release to reflect this change ? It's not clear to me if this should warrant a major bump or not.
from fern.
Hey, new contributor of colored here! I've been hopping on to help maintain some stuff, and I've been in charge of some things like new releases and getting this MSRV stuff figured out.
Will the public API of
colored
change?
It won't, this will just affect internals of how colored
detect TTY usage.
It's been debated to no end whether or not MSRV changes are breaking or not, but my observation is that the community moves towards treating it as not being breaking. Not because it's perfect, but because treating it as breaking is way worse.
That's pretty much been my views on everything too. You never know when a dependency increases their MSRV and how that might affect how your own crate changes their's, so it's been my opinion to just not make it breaking.
IMHO option 1 is the nicest.
I agree that option 1 is the best too, and if combined with colored-rs/colored#133 (comment) that'd make every dependency (except one when targeting Windows) disappear. Just because 1.70 is so recent though I was thinking it should probably hold off until 1.70 gets a bit older. The main option I was looking at then was number 3, as then terminal detection can still be done without any problem.
It does not look like
fern
is strict about keeping its MSRV at all
It definitely looks like @daboross isn't wanting to be too strict, but I'll probably hold off until he can get a response in here. It doesn't look like his GitHub is too active though, so I'll probably wait a week or so and then get a decision going if he hasn't responded.
from fern.
Thanks for noting this!
I think my preferred way forward here would be to add an alternative color library under a different feature flag, and then drop colored
in the next major revision. I believe there's been some discussion of other color libraries in fern
's other issues, but I'll have to dig it up.
Given the security issue, maybe it'd be best to release a major revision for this specifically... In any case, I'd still prefer to have a minor version upgrade path.
from fern.
Looks like owo-colors (mentioned in #67) is probably still the best choice here - only question I have is how well it works on Windows, which can be tested.
from fern.
@mackwic, @hwittenborn Thanks for letting me chime in here!
@faern is correct - I'm mostly concerned with unnecessary version bumps in fern's internals. I'd definitely qualify this as necessary, or at least highly useful, and I wouldn't want to constrain dependencies of this library in any case, just the code itself. Maybe I can add some documentation in the CI file to specify that explicitly.
In any case, all three approaches sound reasonable to me! I'd leave it up to you which is best.
If you go for just upping the MSRV of colored to 1.62 or 1.70, I'd agree with still just doing a minor release. That may break builds, but that's why Cargo.lock
exists, and it'll always be a direct compile-time failure of colored
itself, not silently changing behavior nor breaking any downstream usage of the library. Also probably worth noting if you set package.rust-version
, you even get nice error messages for this.
from fern.
That sounds great @daboross! There's plans for a v3
of colored
in the near future, so what'll probably happen is is-terminal
will be used on v2
so the MSRV isn't super new, and then std::io::IsTerminal
will be used on v3
, and by the time a release is ready for v3
the MSRV should hopefully have a few versions in front of it.
I'll let @kurtlawrence chime in too because he's helping maintain stuff too - do you think that'd a good approach, or would you prefer something else?
from fern.
Once we get that resolved a release should be good to come out real soon as well, I'm thinking of getting that done tomorrow. This vulnerability stuff is definitely something I want to get rid of ASAP, so as soon as it's good to it's definitely going to happen.
from fern.
I submitted a PR on getting this updated and fixed. #128
from fern.
I realize no change to fern
is required. Your new release of colored
is just compatible. I can just do cargo update -p colored
and get rid of the last instance of atty
in my dependency tree 🥳 Thank you very much.
from fern.
Cool, I'm glad to hear @faern! I didn't realize bumping v1 would make it automatic like that either, I'm definitely glad that all got done.
Y'all can handle the major/minor version bump however y'all would like, it won't bother me one way or the other. It's good to see everything's pretty much in the clear now though :).
from fern.
Thank you again for the patch! Now fern does not need to rush any colored upgrade any longer
from fern.
Thanks for the quick reply and interest in solving this!
What's the rationale behind fixing soundness issue = major release? That if anything just makes it harder for people to upgrade. Better if it's a patch release so as many as possible get it without updating their Cargo.toml
files? If it can be done without an API/behavior change of course. If the API changes it's by definition a breaking change.
from fern.
So happy to hear from a colored
developer and that the project is not dead! ❤️
Will the public API of colored
change? If not, it's not a breaking change. It's been debated to no end whether or not MSRV changes are breaking or not, but my observation is that the community moves towards treating it as not being breaking. Not because it's perfect, but because treating it as breaking is way worse.
I am not a fern
developer. I'm just a happy library consumer who wants to get rid of atty. IMHO option 1 is the nicest. It's finally in stable, fewer dependencies etc.
It does not look like fern
is strict about keeping its MSRV at all:
from fern.
is
is-terminal
will be used onv2
so the MSRV isn't super new, and thenstd::io::IsTerminal
will be used onv3
, and by the time a release is ready forv3
the MSRV should hopefully have a few versions in front of it.
Yep, that sounds good to me.
from fern.
Alright this should be good to go in the latest release of colored
@daboross! Let me know if there's any issues you face :)
from fern.
I looked at is-terminal
and it looks like their MSRV is 1.63
instead of 1.62
though, so that's what's going to be in place for v2
of colored
. I'm assuming that's fine with you since you were going to be fine with 1.70
@daboross, but let me know if any changes need to be made.
from fern.
colored 2.0.2
swapping out atty
for is-terminal
is finally released! 🥳 Now all fern
has to do is to upgrade from colored 1
to colored 2
from fern.
Sadly this is a breaking change due to colored
types being public in the API of fern
(fern::colors::Color
is a colored
type). So this will need to be a bump to fern 0.7.0
.
from fern.
Yep - that'll be fine for me. I had forgotten colored had done a 2.0 release, but for a security fix like this I'll do a major release and up it.
If you feel like backporting to colored 1.0 and doing a 1.9.4 release from a branch (branched off of the 1.9.3 release), that could fix it for more people today, but I don't think it's that big of a deal either way. Most rust crates don't support old major versions with security fixes in any case.
from fern.
The important part IMO is to give people a working version of fern
without atty
as a transitive dependency. And I currently don't have the bandwidth to work on colored
. So just getting fern 0.7
with colored ^2
out there is fine for me.
from fern.
Bump on this. There seems to be a pretty clear and easy way forward here. Anything stopping the PR from being reviewed/merged?
from fern.
Hey! I'm back, I've been a bit busy with some other projects I work on.
If you feel like backporting to colored 1.0 and doing a 1.9.4 release from a branch (branched off of the 1.9.3 release), that could fix it for more people today, but I don't think it's that big of a deal either way.
I went ahead and got that done, that'll probably be the last major update for v1
though. I'd definitely recommend upgrade to v2
if y'all are able to though - were there any major blockers stopping that from happening? (From what I saw the only major thing that created a version bump was the interface of colored::Color
returning Cow<&str>
over &str
.)
v3
is definitely going to be more of a rewrite, so if you'd prefer to bump to any version and keep compatibility I'd make it v2
.
from fern.
The signatures of the methods on colored::Color
does not matter. If we change to a different version of colored that is not semver compatible (2 != 1) then it's going to be breaking even if 100% of the API surface were to be identical. Because colored_v1::Color
and colored_v2::Color
will be 100% different types according to the compiler.
If I in my own crate have these dependencies:
[dependencies]
colored = "1.0"
fern = "0.6"
And then I have the code:
let a_color: colored::Color = fern::colors::Color::Black;
The the compiler will error on these being different types if fern
changes to colored 2
.
But great that you have backported the atty
-removal! ❤️ I can look at making an as-small-as-possible bump here in fern
and hope we can release a non-breaking 0.6 version of fern
.
from fern.
Related Issues (20)
- WithFgColor doesn't match colored::Colorize's behavior
- `level_for_module` level-filtering for modules, not targets HOT 3
- Doc type: "equivalent form another crate"
- is there anyway to disable all crates log and only show log of my program? HOT 2
- Connection forcibly closed using `TcpStream` as `Box<dyn Write + Send>` HOT 2
- `SharedDispatch` type is not accessible HOT 2
- Missing list of dependancies in example HOT 2
- Syslog only logs in UTC HOT 2
- Reading from Box not possible since fern::Output::writer consumes it? HOT 2
- DateBased struct not accessible
- double free or corruption (out) HOT 1
- Feature Request: Log string configuration
- Implement log::kv Handling
- Colors and Emojis seem to break rending in Bash and Yakuake HOT 2
- switch from chrono to time HOT 2
- Different formats for differen logging targets? HOT 1
- Possible to chain more files after already intializing a logger?
- Top-level documentation suggests using 0.5 instead of 0.6 HOT 1
- Add new Linux CI HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from fern.