349102 - implement back off code for remote phishing lookups

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Tony Chang (Google)]

Just in case the data provider's servers can't keep up with phishing requests, we should have back off code in the client that temporarily stops making requests.  

Search suggestion uses the following logic:
if there are 3 server errors (500, 502, 503) in 10 minutes, we stop making requests for 10 minutes.  When we restart, if there's another error, we double the wait (20 minutes), and so on.

We should have the same logic, although we may tweak the constants.

Comment 1

[Jesse Ruderman]

What do we do when the anti-phishing server gives us an error, or during back-off?  Allow the load?  Allow the load and put up a subtle warning that the page hasn't been checked for phishiness?  Disallow the load?

Comment 2

[Tony Chang (Google)]

(In reply to comment #1)
> What do we do when the anti-phishing server gives us an error, or during
> back-off?  Allow the load?  Allow the load and put up a subtle warning that the
> page hasn't been checked for phishiness?  Disallow the load?

Not sure, but that seems to be an orthogonal problem because it could currently happen and nothing is shown to the user.  This at least allows the data provider to not melt down.

I would vote for a subtle warning, but maybe that should be a separate bug.

Comment 3

[Tony Chang (Google)]

The suggestion is that we fall back and use local table look ups if the remote lookups fail.  This would require that we also download all tables all the time so we have it as a backup.

Comment 4

[Tony Chang (Google)]

Attachment: v1: remote lookup backoff

- update xml-fetcher to return the http status code
- if http lookup fails, check local lists
- if http lookup fails to many times, back off and use local lists

Comment 5

[Niels Provos]

Comment on attachment 235337 [details] [diff] [review]
v1: remote lookup backoff

looks good.

Comment 6

[Tony Chang (Google)]

Attachment: v2: refactor to remove checkURL_ (not needed)

Functionally same as last patch, just removing the checkURL_ function which isn't needed anymore.

Comment 7

[Tony Chang (Google)]

on trunk

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 235823 [details] [diff] [review]
v2: refactor to remove checkURL_ (not needed)

a=beltzner on behalf of 181drivers

Comment 9

[Tony Chang (Google)]

on branch