lavalink-devs / youtube-source

A rewritten YouTube source manager for Lavaplayer.
MIT License
163 stars 28 forks source link

fix: avoid infinite recursion in case playabiliytStatus NON_EMBEDDABLE #53

Closed taraskondratiuk closed 2 months ago

devoxin commented 2 months ago

This doesn't fix anything? Did you even look at the code? It calls itself with a non-null status parameter, which avoids setting some client fields which would be required when embedding videos. Without these embed fields we shouldn't ever get back a NON_EMBEDDABLE state. Additionally, the getPlayabilityStatus call directly after would throw if a video is still unplayable without the embed fields, which is what the true is for.

taraskondratiuk commented 2 months ago

Did you even look at the code?

yep, and even manually tested initial variant (which is infinite recursion) and my fix

It calls itself

yep, that's the issue, it calls itself without any break condition

the getPlayabilityStatus call directly after

it's never reached cause of recursion

devoxin commented 2 months ago

yep, and even manually tested initial variant (which is infinite recursion) and my fix

Your fix is not proper. There is a reason for that recurse and removing it is treating a symptom and not the cause.

yep, that's the issue, it calls itself without any break condition

There are break conditions, it was just not being triggered because of the way embedded clients work.

it's never reached cause of recursion

See above. Calling getPlayabilityStatus(json, true) will cause any non-OK playability statuses to throw. Calling it with false will return the status rather than throwing. Throwing will cause the code to halt.