search

rustsec / advisory-db

Security advisory database for Rust crates published through crates.io
https://rustsec.org
Other
892 stars 342 forks source link

yaml-rust appears unmaintained... #1921

Closed qtfkwk closed 4 months ago

qtfkwk commented 4 months ago

See also https://github.com/chyh1990/yaml-rust/issues/197

qtfkwk commented 4 months ago

Stumbled onto https://crates.io/crates/yaml-rust2 / https://github.com/Ethiraric/yaml-rust2 yesterday... it forks from the latest commit on the original repository https://github.com/chyh1990/yaml-rust on 2021-07-12, and adds commits starting on 2023-08-14 and latest on 2024-02-08.

I've done a quick review and appears to be functional and in good faith, but should have more people review and confirm.

Also, strangely... @Ethiraric doesn't seem to have any means of contact posted in Github, and has issues disabled on the yaml-rust2 repository (?). Edit: resolved... see https://github.com/rustsec/advisory-db/issues/1921#issuecomment-1994488282

Ethiraric commented 4 months ago

Heya! Having issues disabled was (I think) an unintended side-effect of the repository being a fork in a first place and was not at all intentional.

It forks from one of the pull requests from the original yaml-rust where the author added support for the yaml test suite, a set of YAML files and their expected output. yaml-rust did not pass the whole test suite. Although most of the tests were corner cases that were unlikely to be encountered in real-world YAML, I have fixed all of them so that yaml-rust2 now correctly passes the whole suite.

I've also added a bunch of comments to the code, refactored some constructs and optimized so that the library should be faster than yaml-rust.

I have waited to produce real benchmark data before doing an announced release. I am fairly new to the art of benchmarking, and my last attempts at optimizing have failed, which is why there hasn't been any meaningful commit for a month now.

As-is, I see no reason the code would be less functional than yaml-rust, but I am human and make errors, so maybe there's something I missed, in which case a bug report (now that I have issues enabled, thanks to you) would be welcome.

If there's anything I could help with, do feel free to open an issue or send me an e-mail (I'll add that to my profile in a second, but I have a gmail address whose local part is my username here).

smoelius commented 4 months ago

@davvid's https://crates.io/crates/yaml-rust-davvid is another fork that, to the best of my knowledge, is sincere.

davvid commented 4 months ago

Yes, I'm relying on the availability of this functionality long-term so I'd be more than happy to help maintain a fork or swap over to a different fork. I wasn't aware of yaml-rust2, thanks for bringing that to my attention.

Ethiraric commented 4 months ago

I've had a look at the commits on @davvid's repository, and it seems that they are mostly focused on the output API and handling of Yaml elements. If that's the case, our works should be complementary as I almost exclusively focused on the parsing side of the project.

Ethiraric commented 4 months ago

@davvid Our repositories should have a common ancestor in the latest yaml-rust commit.

Would it be okay with you if I tried merging your commit tree into my repository?

davvid commented 4 months ago

@Ethiraric that'd be great. I actually started on the merge before seeing your message and submitted the changes in a clean rebase over at https://github.com/Ethiraric/yaml-rust2/pull/2

Ethiraric commented 4 months ago

Thank you very much for your work on this pull request! I hope we can get it to merge cleanly soon :heart:

tarcieri commented 4 months ago

I just wanted to say that yaml-rust appears to be implicitly unmaintained, however per our unmaintained crates policy we prefer to wait for 90 days after an issue like https://github.com/chyh1990/yaml-rust/issues/197 has been opened to give the author time to respond.

If someone would like to file an unmaintained crate advisory in advance, we can merge it when that time has elapsed.

davvid commented 4 months ago

I'm linking these here so that folks landing here can see the full paper trail going back to 2020. I guess waiting another 3 months isn't really going to hurt (or change much) in that respect since we've already waited ~4 years.

https://github.com/chyh1990/yaml-rust/issues/160

https://github.com/chyh1990/yaml-rust/issues/192

tarcieri commented 4 months ago

Aah, with https://github.com/chyh1990/yaml-rust/issues/160 especially it seems clear this crate is unmaintained as the maintenance status has already been publicly asked about without response.

Feel free to file an advisory then. It can be merged immediately.

jayvdb commented 4 months ago

Using the latest https://crates.io/crates/yaml-rust2 results in https://osv.dev/RUSTSEC-2021-0153

tarcieri commented 4 months ago

@jayvdb that would be good to report on the yaml-rust2 repo: https://github.com/Ethiraric/yaml-rust2/issues

Ethiraric commented 4 months ago

Sorry this is a bit out of topic but I'll reply here.

If I'm not mistaken, this refers to the encoding crate being unmaintained. A fix is already on master (though it will unfortunately require a breaking change for people using it).

rocallahan commented 4 months ago

Why can't yaml-rust2 take over the yaml-rust crate name? Updating the world to yaml-rust2 is going to be pretty painful. Lots of third-party crates will have to be patched, and there's the possibility of type mismatches while some crates are updated and others aren't. Also, dtolnay responded by marking serde_yaml as unmaintained :-(.

tarcieri commented 4 months ago

@rocallahan the author is completely non-communicative per:

rocallahan commented 4 months ago

@rocallahan the author is completely non-communicative per:

Right. Is there no procedure at all to take over the crate name of a core crate that has been completely abandoned? That seems bad.

tarcieri commented 4 months ago

No, there is not. The only way to deal with a non-communicative author is a fork.

rocallahan commented 4 months ago

No, there is not. The only way to deal with a non-communicative author is a fork.

That is a very bad policy. Bad to the point of ridiculous in a situation like this.

jayvdb commented 4 months ago

@rocallahan , it is a known problem being discussed elsewhere; it is the way it is because of security, so will require a very careful process to override the current security. https://github.com/rust-lang/crates.io/issues/7202 is one semi-related discussion. Maybe some useful discussions at https://users.rust-lang.org/search?q=unmaintained ? e.g. https://users.rust-lang.org/t/pre-rfc-we-need-a-process-for-giving-crates-to-new-maintainers/8033 which leads to https://github.com/rust-lang/rfcs/pull/1794

jayvdb commented 4 months ago

Using the latest https://crates.io/crates/yaml-rust2 results in https://osv.dev/RUSTSEC-2021-0153

https://github.com/Ethiraric/yaml-rust2/issues/16 will resolve this.

rocallahan commented 4 months ago

@rocallahan , it is a known problem being discussed elsewhere

Thanks for the pointers but I don't see any active discussion that covers this specific situation.

rocallahan commented 4 months ago

I see on crates.io yaml-rust is listed as having two owners, and one of them is dtolnay-contrib/yaml-rust. So could dtolnay reassign the yaml-rust crate name to the yaml-rust2 contributors?

tarcieri commented 4 months ago

@rocallahan you would have to ask @dtolnay

Turbo87 commented 4 months ago

That is a very bad policy. Bad to the point of ridiculous in a situation like this.

FWIW the Python ecosystem has https://peps.python.org/pep-0541/ but when we updated the crates.io policies last year there was a lot of opposition against including something like this.

Ethiraric commented 4 months ago

I see on crates.io yaml-rust is listed as having two owners, and one of them is dtolnay-contrib/yaml-rust. So could dtolnay reassign the yaml-rust crate name to the yaml-rust2 contributors?

This has already been asked. Crates.io has already been informed of the issue.

Prior to starting yaml-rust2, I have already asked around for any other way this could be resolved. A lot of options have been explored and I do not think any more discussion about possible ways to take over yaml-rust here will come to fruition.

qtfkwk commented 4 months ago

A "takeover" causes more problems than it's worth. And that's even if it's 100% compatible.

You have competing issues... on one hand you might be thinking: we gotta force everyone who depends on an old and unmaintained crate to upgrade for security reasons. Well (a) you can't because they might have chosen to depend on a specific old version, (b) if they didn't you might be introducing breakage, and (c) it's their problem if they depend on old and unmaintained crates... they still need to exist so the whole thing doesn't fail.

Further, afaik cargo-audit is an aid to help inform authors and users about security issues... not enforce anyone's particular requirements.

IMHO, the best way is as it is: Make a new crate, fork, whatever. Let crate authors depend on whichever crates/versions they want. Done.