Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(OAuth): client identity matching #421

Merged
merged 1 commit into from
Jun 28, 2023
Merged

Conversation

patrickkfkan
Copy link
Contributor

YouTube seems to have changed their Auth script which breaks OAuth#getClientIdentity(). The Client Identity regex used to match against the body of the script is bound to fail. The result is blocking of the entire process while the regex tries futilely to find a match.

This PR fixes the Client Identity matching part. From what I see, the request payload for device code also seems to have changed. Instead of model_name, it's now device_model:

image

So I have included this change in the PR too.

@patrickkfkan
Copy link
Contributor Author

I've been thinking about the situation that occurred before the changes in this PR, where the entire node process locks up because regex keeps trying to find a match. Just wondering if anything can be done about it... worker threads + aborting on timeout comes to mind, but maintaining compatibility across browser, node.js ...is something I am not familiar with.

@absidue
Copy link
Collaborator

absidue commented Jun 28, 2023

If it really is the regex itself that was causing the process to lock up, then moving it to worker threads is not solving the actual core problem, it's just working around the issue.
You need to rewrite the regex so that it doesn't suffer from catastrophic backtracking.
Moving it to worker threads is like building elaborate traps in your house to stop burglars, instead of just closing the front door that is unlocked and wide open.

@patrickkfkan
Copy link
Contributor Author

I was just thinking of guarding against unforeseeable cases where catastrophic backtracking may occur, but you are right that the regex itself should be worked on instead.

So would you mind reviewing the modified regex to see if it is foolproof enough to handle future YT changes, as in not locking up the process when no match is possible? My knowledge of this is rather limited so would appreciate some help here..

@maciekr1234
Copy link

I can confirm indeed regex and or model_name was causing my app to stop responding.

@LuanRT
Copy link
Owner

LuanRT commented Jun 28, 2023

I'll merge this PR right now and publish a new version as this is quite urgent. I plan on doing a small refactor in this portion of the code soon, so I can look into fixing the unsafe regular exp, but anyone feel free to contribute if you can.

Thanks Patrick!

@LuanRT LuanRT merged commit 07c1b3e into LuanRT:main Jun 28, 2023
@patrickkfkan patrickkfkan deleted the oauth branch October 22, 2024 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants