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

Update check is run too often #5467

Closed
4 tasks done
TheAssassin opened this issue Jan 20, 2021 · 2 comments · Fixed by #5474
Closed
4 tasks done

Update check is run too often #5467

TheAssassin opened this issue Jan 20, 2021 · 2 comments · Fixed by #5474
Assignees
Labels
ASAP Issue needs to be fixed as soon as possible bug Issue is related to a bug privacy & data protection Issues regarding either NewPipe, Team NewPipe services or external services NewPipe uses

Comments

@TheAssassin
Copy link
Member

Checklist

This affects all NewPipe versions since the update check was implemented, basically. But yes, it applies to the latest version, too.

Steps to reproduce the bug

See following section.

Actual behaviour

The update check is run every time the app is opened freshly. To be precise, it occurs in App.onCreate. @B0pol thinks this method also runs when the device is rotated, which would be quite annoying.

Since many people have switched to our own builds in the last couple of weeks (basically, since F-Droid has failed to provide up-to-date binaries), this starts to create some significant load on the server. It's not hitting any limits, but enough to increase the load count and trigger monitoring.

There is also a privacy aspect. I was looking at the logs a bit. They're of course anonymized, but I can see clusters of same-range requests, close nearby. This probably could be abused to guesstimate how long and when people use NewPipe.
As we don't want to gather any form of usage statistics (we also don't collect the current versions), we should do something against it.

Expected behavior

There needs to be some client-side rate limiting. It makes no sense to check for updates faster than, say, 6-12 hours. We don't release that often. A push solution would be nice, but we don't have anything like that in place, so we should keep polling. Just reduce the frequency.

I suggest to let the server control these timeouts. This allows us to react on load peaks and similar things better. HTTP provides the Expires header, which seems to be a perfect match.

I would combine this with some hardcoded limits to have some protection against configuration flaws on the server side. I could imagine having a lower limit of 6 hours, as well as a top limit of 48 or even 72 hours. The lower limit shall ensure that update checks aren't run too frequently even if the server doesn't implement the Expires header properly. The upper limit makes sure that update checks are run at least every couple days to protect against configuration mistakes.

Screenshots/Screen recordings

-/-

Logs

-/- (the only logs I have are server logs, and you know I won't share them with anyone)

Device info

Doesn't matter. Probably many different ones.

@TheAssassin TheAssassin added bug Issue is related to a bug ASAP Issue needs to be fixed as soon as possible privacy & data protection Issues regarding either NewPipe, Team NewPipe services or external services NewPipe uses labels Jan 20, 2021
@TheAssassin TheAssassin mentioned this issue Jan 20, 2021
14 tasks
@nadiration
Copy link
Contributor

Also an easily visible manual update button to check for updates is essential (for some users)

@XiangRongLin
Copy link
Collaborator

Also an easily visible manual update button to check for updates is essential (for some users)

@nadiration you can open a seperate issue for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASAP Issue needs to be fixed as soon as possible bug Issue is related to a bug privacy & data protection Issues regarding either NewPipe, Team NewPipe services or external services NewPipe uses
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants