423790 - Phishing Backoff not working with 400s

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect, P1)

Description

[Garrett Casto]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12
Build Identifier: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12

If the phishing server returns 400's, Firefox will make the request every minute, instead of just making 3 requests, and then backing off for an hour.

Reproducible: Always

Steps to Reproduce:
1. Have server return 400.

Comment 1

[Brian Ryner (not reading)]

Bumping up the priority -- we really need to get this fixed for beta 5, so that Firefox won't hammer the phishing servers if it receives 400's.

Comment 2

[Dave Camp (:dcamp)]

The backoff object doesn't consider 400 to be an error.  This should be an easy
fix, I'll attach a patch soon.

Comment 3

[Dave Camp (:dcamp)]

Attachment: trigger backoff for 400s

Comment 4

[Dave Camp (:dcamp)]

Attachment: slightly better fix

This version avoids scheduling the one-minute reping if the backoff object doesn't consider the server response worth erroring out on.

Comment 5

[Tony Chang (Google)]

Comment on attachment 310410 [details] [diff] [review]
slightly better fix

This is fine, but it's also a dupe of bug 378132.  This fix seems to be slightly better since we're now considering 4xx an error (that's not how we spec'ed it originally).

Comment 7

[Dave Camp (:dcamp)]

Checking in content/listmanager.js;
/cvsroot/mozilla/toolkit/components/url-classifier/content/listmanager.js,v  <--  listmanager.js
new revision: 1.28; previous revision: 1.27
done
Checking in content/request-backoff.js;
/cvsroot/mozilla/toolkit/components/url-classifier/content/request-backoff.js,v  <--  request-backoff.js
new revision: 1.2; previous revision: 1.1
done

Comment 8

[Dave Camp (:dcamp)]

The patch in bug 378132 might be a safer basis for a branch patch, as it changes fewer interfaces (a bit).  You'd want to update it to treat 4xx as errors.

Comment 9

[Daniel Veditz [:dveditz]]

Clearing branch blocking request because we took the patch in bug 378132. If we still need more please re-nominate and explain.

Comment 10

[Carsten Book [:Tomcat]]

Dave/Garret, can you verify this bug is fixed, i think its difficult for QA to generate this Error 400 :-)