search

librespot-org / librespot

Open Source Spotify client library
MIT License
4.52k stars 545 forks source link

Update dependencies #1194

Closed yubiuser closed 8 months ago

yubiuser commented 11 months ago

Updates dependencies

    governor matches 0.5.1; latest is 0.6.0
    num-derive matches 0.3.3;   latest is 0.4.0
    quick-xml matches 0.29.0;   latest is 0.30.0

librespot-playback: /librespot/playback/Cargo.toml
    glib matches 0.17.10;   latest is 0.18.1
yubiuser commented 11 months ago

Needed to upgrade MSRV to 1.67

yubiuser commented 11 months ago

Ok, glib needed MSRV 1.70....

roderickvd commented 11 months ago

I appreciate you keeping this up to date.

MSRV always seems a heated topic. Clearly most active people here want to ditch the approach of tracking some Linux distribution for its Rust version. I'm fine with that also because Rust seems to patch security vulnerabilities in latest versions, and not back port them to earlier ones (i.e. no LTS releases or anything like that).

So, if you could resolve the conflict then we can merge.

yubiuser commented 11 months ago

I appreciate you keeping this up to date.

I appreciate librespot and use it in one of my own projects. This is my way of giving back.

Yeah, MSRV is a heated topic. My opinion: they have a clean installer/uninstaller which easily allows to update (or remove) rust - there is no need to stay on an outdated version distributed by the OS.

Merge conflicts have been resolved.

roderickvd commented 8 months ago

Sorry for taking so long. Merging now!

If I may... there are a couple of security alerts one of which requiring webkpi >= 0.22. Would you be so kind?

yubiuser commented 8 months ago

I created a PR (https://github.com/librespot-org/librespot/pull/1224) to update all dependencies to their latest version. However, webkpi is only an indirect dependency by some of librespot's dependencies, so I added it manually to librespot's dependencies. But some dependecies hard-code a 0.21.4 version (hyper-proxy, hyper-rustls)

yubiuser commented 8 months ago

There is a PR that would update hype-proxy (https://github.com/tafia/hyper-proxy/pull/38), however, the repo looks a bit abandoned....

roderickvd commented 8 months ago

That's a pity. If the repo is stale, with no other active fork, maybe we should bundle a patched version?

This is the vulnerability. I think it's OK to post, since it is not easily exploitable and does not grant privilege escalation:

When this crate is given a pathological certificate chain to validate, it will spend CPU time exponential with the number of candidate certificates at each step of path building.

Both TLS clients and TLS servers that accept client certificate are affected.

This was previously reported in https://github.com/briansmith/webpki/issues/69.

rustls-webpki is a fork of this crate which contains a fix for this issue and is actively maintained.

yubiuser commented 8 months ago

There are some "active" forks (https://techgaun.github.io/active-forks/index.html#https://github.com/tafia/hyper-proxy) but mainly they contain one additional branch with updated dependencies.

If the repo is stale, with no other active fork, maybe we should bundle a patched version?

This could be a way to go - but is outside of my rust knowledge/scope.

yubiuser commented 8 months ago

One of the forks is from @herrernst who already contributed to librespot. Their branch also contained some more modifications then just dependency upgrades. Maybe they can contribute again?

herrernst commented 8 months ago

Yeah, I experimented a little bit with the hyper* dependencies. hyper was just released as 1.0.0: https://github.com/hyperium/hyper/releases/tag/v1.0.0 Maybe I have time in the days to come to fix hyper-proxy. The security issue posted above doesn't seem to be a concern to librespot, but upgrading would nonetheless be good.