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 ReCaptcha activity and correctly save obtained cookies #3035

Merged
merged 13 commits into from
Feb 2, 2020

Conversation

Stypox
Copy link
Member

@Stypox Stypox commented Jan 29, 2020

Description

This pr aims to solve the many issues regarding the ReCaptchaActivity. In #2527 I tried to fix it, though I was unable to test the changes since I couldn't reproduce any ReCAPTCHAs anymore, so it kept crashing. After @B0pol in #2979 provided a way to reproduce, I was able to redesign the activity. Before the cookies were extracted from the page on close, but unfortunately this didn't imply that the captha was solved, since there could have been redirections or the page didn't close on resolution. Now the user is provided with a "Done" button to press when he finishes solving the captcha, that sets the cookies to the downloader.
Sometimes services return a recaptcha challenge error code (429) just to indicate Too Many Requests, so in that case the ReCaptchaActivity is opened, but the only thing the user has to do is press "Done" since there are no recaptchas to solve. This is the only scenario I was able to test, and I am confident the Youtube cookie extraction method works since I didn't change it, (it didn't work, but it should be fixed now) though I am not entirely sure that the handleCookies function is always called (someone who knows how WebViewClients work?). (now it is always called on close).

Also renamed some recaptcha-related strings to have them match the naming conventions, leading to many changed strings.xml files.

Follow-ups for next pull requests

Maybe we should keep a hash-table with all cookies and their values and update that table every time we need, instead of overwriting the whole cookie string every time. Since, for now, the only service with recaptchas is YouTube, there is no problem to always overwrite the old Downloader cookies, but for the future we might need to change this.
Anyway in the handleCookies function I left the space for other services (other than Youtube) to try and extract cookies.

Fixes & debug apk

Fixes #3023 fixes #2979 fixes #2924 fixes #2788 fixes #2484
Debug apk: app-debug.zip

@TobiGr TobiGr added the bug Issue is related to a bug label Jan 29, 2020
@B0pol
Copy link
Member

B0pol commented Jan 29, 2020

I car confirm it's fixing fine #2979.
Just a question : could you make background agree with theme?

This is the page I had with peertube.co.uk , my theme was default (grey)

@TobiGr
Copy link
Contributor

TobiGr commented Jan 29, 2020

Just a question : could you make background agree with theme?

This should be quite easy. Maybe we can pass either some js to the webview after loading the site. For converting R.color ids which are used in the themes to hex, you might use the the getHexRGBColor method from here. However, it should be move the to an util class.

@TobiGr TobiGr added this to the 0.18.3 milestone Jan 29, 2020
@Stypox
Copy link
Member Author

Stypox commented Jan 30, 2020

@B0pol I am not sure it is safe to change the background of the webview. It would surely require some (potentially slow) JavaScript code to be run, and it could create graphical problems on complex recaptcha pages (I use an extension to change colors on my Firefox ("Dark Background and Light Text"), but on some websites it doesn't work properly).
I don't think it is really a problem to have an unthemed recaptcha activity, because after all it is shown very rarely. But anyway if we decide to implement it, I think it would be better suited for a separate PR, to make merging this faster ;-).

@juanfra684
Copy link

I'm testing the apk file attached to the first comment but it doesn't fix my problems with YT captchas. I always get a captcha when I try to watch a YT video. I solve the captcha and can watch the video in the webview of NewPipe. I press ✔️ when I solve the captchas but then I get again another captcha with the next video which I watch.

@Stypox
Copy link
Member Author

Stypox commented Feb 1, 2020

@juanfra684 could you test this apk, based on the last commit?
app-debug_1.zip

If even the apk above doesn't work, please use the following one. When you press :heavy_check_mark (after solving) it throws an error on purpose (so that I can see the content of cookies): please paste the bug report content here.
app-debug_2.zip

@Stypox
Copy link
Member Author

Stypox commented Feb 1, 2020

@TobiGr this is how the Activity looks like now. Is it ok? I decided to move the button to the right for better UI, since it's an action.

@juanfra684
Copy link

@juanfra684 could you test this apk, based on the last commit?
app-debug_1.zip

app-debug_1.zip is corrupted. I can extract the apk but I can't install it. app-debug_2.zip installs fine.

@Stypox
Copy link
Member Author

Stypox commented Feb 1, 2020

@juanfra684 Could you try again with this apk (as a replacement for app-debug_1.zip)? Thank you :-)
app-debug.zip

@juanfra684
Copy link

I've the same problems with the new apk. This is the output of app-debug_2 with the captcha solved:

{
   "user_action": "ui error",
   "request": "App crash, UI failure",
   "content_language": "GB",
   "service": "none",
   "package": "org.schabi.newpipe.debug",
   "version": "0.18.2",
   "os": "Linux Android 7.1.2 - 25",
   "time": "2020-02-01 21:58",
   "exceptions": [
      "java.lang.NullPointerException: Found cookies: YSC=AeJKpj2WGsA; s_gl=e3ea30620a3c9e68880a30237b86c6cfcwIAAABHQg==; VISITOR_INFO1_LIVE=fflNk6rzzmc; GPS=1; CONSENT=WP.283253; YSC=AeJKpj2WGsA; s_gl=e3ea30620a3c9e68880a30237b86c6cfcwIAAABHQg==; VISITOR_INFO1_LIVE=fflNk6rzzmc; GPS=1; CONSENT=WP.283253; YSC=AeJKpj2WGsA; s_gl=e3ea30620a3c9e68880a30237b86c6cfcwIAAABHQg==; VISITOR_INFO1_LIVE=fflNk6rzzmc; GPS=1; CONSENT=WP.283253\n\tat org.schabi.newpipe.ReCaptchaActivity.saveCookiesAndFinish(ReCaptchaActivity.java:134)\n\tat org.schabi.newpipe.ReCaptchaActivity.onOptionsItemSelected(ReCaptchaActivity.java:121)\n\tat android.app.Activity.onMenuItemSelected(Activity.java:3248)\n\tat androidx.fragment.app.FragmentActivity.onMenuItemSelected(FragmentActivity.java:384)\n\tat androidx.appcompat.app.AppCompatActivity.onMenuItemSelected(AppCompatActivity.java:219)\n\tat androidx.appcompat.view.WindowCallbackWrapper.onMenuItemSelected(WindowCallbackWrapper.java:109)\n\tat androidx.appcompat.view.WindowCallbackWrapper.onMenuItemSelected(WindowCallbackWrapper.java:109)\n\tat androidx.appcompat.app.ToolbarActionBar$2.onMenuItemClick(ToolbarActionBar.java:64)\n\tat androidx.appcompat.widget.Toolbar$1.onMenuItemClick(Toolbar.java:207)\n\tat androidx.appcompat.widget.ActionMenuView$MenuBuilderCallback.onMenuItemSelected(ActionMenuView.java:781)\n\tat androidx.appcompat.view.menu.MenuBuilder.dispatchMenuItemSelected(MenuBuilder.java:840)\n\tat androidx.appcompat.view.menu.MenuItemImpl.invoke(MenuItemImpl.java:158)\n\tat androidx.appcompat.view.menu.MenuBuilder.performItemAction(MenuBuilder.java:991)\n\tat androidx.appcompat.view.menu.MenuBuilder.performItemAction(MenuBuilder.java:981)\n\tat androidx.appcompat.widget.ActionMenuView.invokeItem(ActionMenuView.java:625)\n\tat androidx.appcompat.view.menu.ActionMenuItemView.onClick(ActionMenuItemView.java:151)\n\tat android.view.View.performClick(View.java:5637)\n\tat android.view.View$PerformClick.run(View.java:22433)\n\tat android.os.Handler.handleCallback(Handler.java:751)\n\tat android.os.Handler.dispatchMessage(Handler.java:95)\n\tat android.os.Looper.loop(Looper.java:154)\n\tat android.app.ActivityThread.main(ActivityThread.java:6121)\n\tat java.lang.reflect.Method.invoke(Native Method)\n\tat com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:889)\n\tat com.android.internal.os.ZygoteInit.main(ZygoteInit.java:779)\n"
   ],
   "user_comment": ""
}

@Stypox
Copy link
Member Author

Stypox commented Feb 2, 2020

I've the same problems with the new apk.

Do you mean that you're not able to install it, or that solving the recaptcha does nothing? @juanfra684

This is the output of app-debug_2 with the captcha solved:

Thank you :-)

@juanfra684
Copy link

I've the same problems with the new apk.

Do you mean that you're not able to install it, or that solving the recaptcha does nothing? @juanfra684

I can install it but YT always shows a captcha with every video. I can watch any video within the webview after of solve the first captcha, so my IP is not blacklisted. The problem only happens when I go to a video from NewPipe directly.

I've tried a few times solving the captchas and touching ✔️ but YT stills shows captchas with every new video.

@Stypox
Copy link
Member Author

Stypox commented Feb 2, 2020

@juanfra684
Ok, I should have fixed the problem with the latest commit, thanks to the report you provided.
Sorry if I keep asking you to test APKs :-), here is hopefully the last one: app-debug.zip

@juanfra684
Copy link

@juanfra684
Ok, I should have fixed the problem with the latest commit, thanks to the report you provided.
Sorry if I keep asking you to test APKs :-), here is hopefully the last one: app-debug.zip

Don't worry. NewPipe is one of my favorite apps, I've not problem helping to test the fixes.

Looks like your last commit fixed the problem. I only was asked to solve the captcha one time. After that everything worked as usual.

Thanks a lot for fixing this!

@Stypox
Copy link
Member Author

Stypox commented Feb 2, 2020

Thank you very much for testing!
@TobiGr this should be ready :-D

Copy link
Contributor

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me

@TobiGr TobiGr merged commit 471ce4a into TeamNewPipe:dev Feb 2, 2020
@mauriciocolli
Copy link
Contributor

For reference in the future, there's a very easy way to get reCAPTCHAs to test this issue: route your traffic through tor or any popular free proxy. Chances are good that a abuser/spammer already made it blacklisted.

But this appears to have fixed it.

PS: One thing though that was already mentioned, we are setting the cookies for EVERY request, even for those that the service don't use it, what about we don't do that? Maybe something like a cookie jar for services?

I think this could be done before the next release.

@Stypox Stypox deleted the recaptcha branch February 9, 2020 10:48
@Stypox Stypox linked an issue Feb 9, 2020 that may be closed by this pull request
3 tasks
@Stypox
Copy link
Member Author

Stypox commented Feb 27, 2020

For reference in the future, there's a very easy way to get reCAPTCHAs to test this issue: route your traffic through tor or any popular free proxy. Chances are good that a abuser/spammer already made it blacklisted.

How can I route my NewPipe's traffic through tor? @mauriciocolli

@TobiGr
Copy link
Contributor

TobiGr commented Feb 27, 2020

@Stypox Install Orbot. That app is in the GuardianProject repository which is enabled by default in F-Droid.

@Stypox
Copy link
Member Author

Stypox commented Feb 27, 2020

Thanks

@Stypox Stypox mentioned this pull request Mar 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug
Projects
None yet
6 participants