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.

Sign up for GitHub

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

Jump to bottom

[YouTube] Fix extraction of more complex nsig functions #891

Merged
merged 2 commits into from
Aug 12, 2022
Merged

[YouTube] Fix extraction of more complex nsig functions #891

merged 2 commits into from
Aug 12, 2022

Conversation

Theta-Dev
Copy link
Contributor

@Theta-Dev Theta-Dev commented Aug 12, 2022 •
edited by AudricV
Loading

I fixed the "Could not get any stream" error that occurred after a YouTube update on 12.08.2022.

The issue was that the more complex nsig decryption function now includes curly braces inside strings, which the findNextParenthesis function could not handle. This resulted in the extraction of broken JavaScript and an interpreter error.

Fixes TeamNewPipe/NewPipe#8760.

This PR also catches really this time throttling errors, to really prevent extraction issues when the throttling parameter could not be decrypted.

Test APK: app-debug.zip

@rrrrrrmb
Copy link

So fast \o/ Thanks for the fix :)

AudricV
AudricV reviewed Aug 12, 2022
View reviewed changes
Copy link
Member

@AudricV AudricV left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much!

A few code style issues are present, but if you tested yourself and it works, I think that's fine :)

Could you also apply TeamNewPipe/NewPipe#8760 (comment) if possible, so #786 is finally doing its job? Thank you in advance!

Edit: I will do all the changes myself

extractor/src/main/java/org/schabi/newpipe/extractor/utils/StringUtils.java Outdated Show resolved Hide resolved
extractor/src/test/java/org/schabi/newpipe/extractor/utils/StringUtilsTest.java Outdated Show resolved Hide resolved
@AudricV AudricV added bug Issue is related to a bug ASAP Issue needs to be fixed as soon as possible youtube service, https://www.youtube.com/ labels Aug 12, 2022
@Sk44rt
Copy link

Sk44rt commented Aug 12, 2022

So we'll just wait for merging and making new release

@AudricV
Copy link
Member

AudricV commented Aug 12, 2022

Yes, but I'd like to fix TeamNewPipe/NewPipe#8713 on the fly, by merging #890 once I updated mocks, as it would be great if we don't have to make a second hotfix release :)

@AudricV AudricV changed the title Fix extraction of more complex nsig functions [YouTube] Fix extraction of more complex nsig functions Aug 12, 2022
@gamer191 gamer191 mentioned this pull request Aug 12, 2022
Closed
9 tasks
@Wiskina

This comment was marked as resolved.

Sign in to view
@Theta-Dev @AudricV
Handle curly braces inside strings in StringUtils.matchToClosingParen…
52ded6e 
…thesis

This is required to extract fully more complex YouTube nsig functions.
@AudricV AudricV force-pushed the fix-throttling-decrypter branch from b672d37 to aaab05c Compare August 12, 2022 14:32
@Theta-Dev

This comment was marked as resolved.

Sign in to view
@Wiskina

This comment was marked as resolved.

Sign in to view
@AudricV

This comment was marked as resolved.

Sign in to view
@Theta-Dev
Copy link
Contributor Author

Here is a test build of NewPipe if you want to try the fix:
app-debug.zip

@TDavLinguist TDavLinguist mentioned this pull request Aug 12, 2022
Closed
6 tasks
@AudricV
[YouTube] Catch any exception in YoutubeThrottlingDecrypter.apply and…
5b54834 
… improve docs

This will prevent any future extractor break due to decryption failure, like it was excepted to be the case before.

Some documentation about the throttling decryption has been also improved.
@AudricV AudricV force-pushed the fix-throttling-decrypter branch from aaab05c to 5b54834 Compare August 12, 2022 15:50
FireMasterK
FireMasterK approved these changes Aug 12, 2022
View reviewed changes
Copy link
Member

@FireMasterK FireMasterK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

extractor/src/main/java/org/schabi/newpipe/extractor/utils/StringUtils.java
}
break;
case '\'':
case '"':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The grave character (`) should also probably be included since they can be used to create multi-line strings.

@AudricV
Copy link
Member

AudricV commented Aug 12, 2022

Thank you again for your contribution, @Theta-Dev!

@AudricV AudricV merged commit 6a885ef into TeamNewPipe:dev Aug 12, 2022
@ph00lt0

This comment was marked as off-topic.

Sign in to view
@opusforlife2

This comment was marked as off-topic.

Sign in to view
@jirafey
Copy link

jirafey commented Aug 12, 2022

I want to thank you from the bottom of my heart 🥇

@Univers4craft

This comment was marked as spam.

Sign in to view
@savagekw

This comment was marked as spam.

Sign in to view
@figo2001

This comment was marked as spam.

Sign in to view
@DominikNovosel

This comment was marked as resolved.

Sign in to view
@PavelNL

This comment was marked as off-topic.

Sign in to view
@Ni101

This comment was marked as off-topic.

Sign in to view
@DominikNovosel

This comment was marked as off-topic.

Sign in to view
@KapriQ

This comment was marked as off-topic.

Sign in to view
@AudricV

This comment was marked as off-topic.

Sign in to view
This was referenced Aug 13, 2022
Open
Closed
@RandomModderJDK RandomModderJDK mentioned this pull request Aug 13, 2022
Closed
@Gitoffthelawn
Copy link

In TeamNewPipe/NewPipe#8760 (comment), @AudricV indicates:

What seem to happen is that the function is not extracted fully, because of new line breaks.

We have a code to prevent this issue, but it doesn't catch any exception, but only ParsingExceptions. If we make YoutubeThrottlingDecrypter.apply(String, String) throwing any Exception instead of a ParsingException and that we catch this exception, the anti-break code works.

If I'm reading this correctly, it sounds like the exception handler wasn't working because it was only detecting (and thus handling) ParsingExceptions instead of all exceptions. It appears the above recommendation may a good one because it will handle possible situations in the future that are similar to this issue.

In addition to the recommendation made in the above quote, it seems logical to also handle the new upstream string format so that no exception is generated (which will therefore handle throttling as intended within NewPipe). Has this already been done, or is that a separate task that needs to be completed?

@AudricV
Copy link
Member

AudricV commented Aug 14, 2022

I already included this in 5b54834 😉

@Gitoffthelawn
Copy link

@AudricV wrote:

I already included this in 5b54834 wink

Was your response intended for me? If so, thank you! 👍🏾

@AudricV AudricV mentioned this pull request Aug 18, 2022
Merged
2 tasks
@Wiskina

This comment was marked as outdated.

Sign in to view
@Wiskina

This comment was marked as off-topic.

Sign in to view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AudricV AudricV AudricV left review comments

@Belove0 Belove0 Belove0 left review comments

@FireMasterK FireMasterK FireMasterK approved these changes

Assignees
No one assigned
Labels
ASAP Issue needs to be fixed as soon as possible bug Issue is related to a bug youtube service, https://www.youtube.com/
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FIXED IN 0.23.2] [YouTube] Something went wrong/Could not get any stream