Skip to content

Do not set reCaptcha cookie when there is no cookie stored#5997

Merged
TobiGr merged 1 commit intodevfrom
reChaptcha
May 4, 2021
Merged

Do not set reCaptcha cookie when there is no cookie stored#5997
TobiGr merged 1 commit intodevfrom
reChaptcha

Conversation

@TobiGr
Copy link
Contributor

@TobiGr TobiGr commented Apr 4, 2021

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

choose the correct default value when getting the reChaptcha cookie from the preferences.
In DownloaderImpl#getCookies(String url) the reChaptcha cookie is set if it is not null. For this reason, the cookie was set in every request.

Fixes the following issue(s)

APK testing

On the website the APK can be found by going to the "Checks" tab below the title and then on "artifacts" on the right.

Due diligence

In DownloaderImpl#getCookies(String url) the reChaptcha cookie is set if it is not null. For this reason, the cookie was set in every request.
@TobiGr TobiGr added the privacy & data protection Issues regarding either NewPipe, Team NewPipe services or external services NewPipe uses label Apr 4, 2021
@TobiGr TobiGr requested a review from Stypox April 4, 2021 10:28
@B0pol B0pol changed the title Do not set reChaptache cookie when there is no cookie stored Do not set reCaptacha cookie when there is no cookie stored Apr 4, 2021
@opusforlife2 opusforlife2 changed the title Do not set reCaptacha cookie when there is no cookie stored Do not set reCaptcha cookie when there is no cookie stored Apr 4, 2021
@Redirion
Copy link
Member

Redirion commented Apr 8, 2021

Looks plain simple and easy. Anything we are waiting for? Should be good to merge.

@XiangRongLin
Copy link
Collaborator

@Redirion Can't you then just merge it? You have the rights to approve and merge it

@Redirion
Copy link
Member

but TobiGr requested a review from Stypox. So there could be more into it that requires (regional) testing

@TobiGr
Copy link
Contributor Author

TobiGr commented May 4, 2021

we should merge this. I requested a review from Stypoc, because he has a very good overview of the whole reChaptcha mechanism implemented in NewPipe. I think, this should be good to merge.

@TobiGr TobiGr merged commit 3ca1e55 into dev May 4, 2021
@TobiGr TobiGr deleted the reChaptcha branch May 4, 2021 16:53
This was referenced May 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

privacy & data protection Issues regarding either NewPipe, Team NewPipe services or external services NewPipe uses

Projects

None yet

Development

Successfully merging this pull request may close these issues.

solving captcha won't show video anymore

4 participants