[YouTube] Fixes for n param deobfuscation function #1253
Conversation
…ixup function to prevent early return
ShareASmile
added
bug
| // Pattern.compile(SINGLE_CHAR_VARIABLE_REGEX + "=\"nn\"\\[\\+" + MULTIPLE_CHARS_REGEX | ||
| // + "\\." + MULTIPLE_CHARS_REGEX + "]," + MULTIPLE_CHARS_REGEX + "\\(" | ||
| // + MULTIPLE_CHARS_REGEX + "\\)," + MULTIPLE_CHARS_REGEX + "=" | ||
| // + MULTIPLE_CHARS_REGEX + "\\." + MULTIPLE_CHARS_REGEX + "\\[" | ||
| // + MULTIPLE_CHARS_REGEX + "]\\|\\|null\\).+\\|\\|(" + MULTIPLE_CHARS_REGEX | ||
| // + ")\\(\"\"\\)"), |
There was a problem hiding this comment.
Why did you remove the previous regex? It's better if you keep it, so in case YouTube reverts something it's already there and it works.
There was a problem hiding this comment.
1.
This pattern is prone to find wrong functions.
Imagine this:
- NewPipeExtractor searches for the n deobfuscation function
- the first regex doesn't work so it uses the second one (the one I commented out)
- the (commented out) regex finds the wrong function
- NewPipeExtractor runs the wrong function and gets the wrong n parameter
- User gets 403
- No exception is thrown in NewPipeExtractor so it is harder to find the correct place which needs fixing
2.
The regex which comes after the one commented out already finds a very similar pattern, only different groups and more specific.
In this example:
a.D&&(b="nn"[+a.D],WL(a),c=a.j[b]||null)&&(c=SDa[0](c),a.set(b,c),SDa.length||Wma("")
Looking at different versions of the player code it seems more often correct to use the function which is in SDa[0] but using Wma in this case would find the wrong function in newer versions.
Going for SDa[0] (and searching for what is in the array afterwards) seems more robust across multiple versions. The next regex already catches that.
From what I have seen I think if the regex which I commented out is added back in it should be swapped with the next one to hopefully prevent false positives more often.
(Also it might be a good idea to add some additional checks to validate it found the correct function but I consider this out of scope for this PR.)
But I have to say I'm very new to this YT/NewPipeExtractor stuff and only looked at some but not all player version so I might lack some experience with YTs past which means I might be wrong. I can only talk about what I have seen so far so if you know better feel free to correct me.
My suggestion for now:
I will add it back in but swap it with the next one.
What do you think?
| @@ -214,4 +237,17 @@ private static String validateFunction(@Nonnull final String function) { | |||
| JavaScript.compileOrThrow(function); | |||
| return function; | |||
| } | |||
|
|
|||
| @Nonnull | |||
| private static String fixupFunction(@Nonnull final String function) | |||
There was a problem hiding this comment.
Mmmh, this function is specific to the new player code, right? Maybe it would be best to return the original function if the regexes don't find anything.
Also, could you add a comment explaining what fixupFunction does with an example?
There was a problem hiding this comment.
I think the current code is fine because:
- The first regex just finds the functions parameters. A function in JS always follows the same pattern so this regex should never fail.
- The second regex is used with
replaceFirstof the Matcher.replaceFirstalready checks if the pattern was found and just returns the input if it wasn't found so this already does what you described.
|
Does this PR fix the http 403 error? |
Fixes #1252
Changes:
new regex for extracting the name of the n param deobfuscation function
added fixup for the n param deobfuscation functions code to prevent an early return
I carefully read the contribution guidelines and agree to them.
I have tested the API against NewPipe.
I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API.