search

librespot-org / librespot

Open Source Spotify client library
MIT License
4.79k stars 596 forks source link

fix: change spotify version needed in clientoken to use semantic format #1267

Closed acolombier closed 6 months ago

acolombier commented 6 months ago

Fixes #1261

kingosticks commented 6 months ago

Which platforms are tested with this? Is it the string formatting that's important here or just the newer version number? It's a bit odd we still have the old version number in use elsewhere.

acolombier commented 6 months ago

Good question - I'm not sure. I stumble across this as I was comparing the protobuf message between the Spotify Desktop Client and Librespot and noticed that the version appeared to be corrupted - it didn't include a string for a float32 instead, which I think explain the Bad Request server-side. Now, since spotify_version() returns a String, it is very odd how this ends up being packed as a float, instead of string - perhaps a bug in the proto crate? Anyway, using a str as source seems to have fixed the problem, and I can now see identical proto messages between both desktop client and librespot. I didn't spend the time checking whether other values semantic version compatible would work, but I would expect so, if the theory of incorrect message format is the correct one.

My guess is that protocol version and client version are two separate things, but I'm not sure.

Edit: tested on Linux amd64

wisp3rwind commented 6 months ago

FWIW, this also silences the warnings from #1261 for me (also Linux amd64).

roderickvd commented 6 months ago

I'll merge it as it's clearly an important "hot fix". Thanks.

Ideally though all the number versions should be consistent with the protocol version at that time, meaning that the protobufs match / were extracted from that client version. If someone could do that...

acolombier commented 6 months ago

I'd be happy to give it a go, but not sure how to do that. Is there wiki or doc? Or is that a matter of digging in Spotify files to find where they have been packed?

hrkfdn commented 5 months ago

Is this something that's worth backporting to the master branch? It seems to be a problem with the current release.

kingosticks commented 5 months ago

Are you sure? The current release is 0.4.2 and doesn't use a client token to get metadata via HTTP, it uses Mercury. It would be impossible to backport this fix to master. I'm not saying there isn't a similar problem in master but it seems unlikely to be exactly this.

hrkfdn commented 5 months ago

Librespot doesn't as a binary. ncspot however uses it as a library and requests a client token via Mercury like so: https://github.com/hrkfdn/ncspot/blob/d1ea7b069efd4df7e311be9d6d24d4fa3edb2991/src/spotify_worker.rs#L66-L87

This token is then used to request HTTP metadata and according to the bug report https://github.com/hrkfdn/ncspot/issues/1421 seems to be suffering from the same issue reported in #1261.