switch to using v2.2 safebrowsing servers

RESOLVED FIXED

Status

()

Toolkit
Safe Browsing
RESOLVED FIXED
9 years ago
4 years ago

People

(Reporter: dcamp, Assigned: dcamp)

Tracking

({fixed1.9.1, verified1.9.0.5})

Trunk
fixed1.9.1, verified1.9.0.5
Points:
---
Bug Flags:
blocking-firefox3.5 +
blocking1.9.0.5 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

9 years ago
Created attachment 345025 [details] [diff] [review]
switch prefs

Simple fix to switch to reporting a pver of 2.2 when requesting from the safebrowsing server.  The fixes necessary to support 2.2 are already on trunk.
Flags: blocking-firefox3.1?
Attachment #345025 - Flags: review?(tony)

Comment 1

9 years ago
Comment on attachment 345025 [details] [diff] [review]
switch prefs

+        // XXX: temp
+        result->mConfirmed = PR_TRUE;

Can you explain this to me?  Otherwise, looks good.
Attachment #345025 - Flags: review?(tony) → review+
(Assignee)

Comment 2

9 years ago
err yeah, that snuck in for a completely unrelated test, I'll take that out before landing.
Comment on attachment 345025 [details] [diff] [review]
switch prefs

>--- a/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp       Mon Oct 27 10:55:51 2008 -0700
>+++ b/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp       Mon Oct 27 17:02:38 2008 -0700
>-#define MAX_PATH_COMPONENTS 4
>+#define MAX_PATH_COMPONENTS 5

Could you comment on this change somehow?

BTW: Where is the specification of the new "safebrowsing" protocol (v2.2)? http://code.google.com/p/google-safe-browsing/wiki/Protocolv2Spec says at the moment: "Client specification for the Google Safe Browsing v2.1 protocol".

If it is not publicly available, could you enumerate all differences between version 2.1 and 2.2?
(Assignee)

Comment 4

9 years ago
(In reply to comment #3)
> (From update of attachment 345025 [details] [diff] [review])
> >--- a/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp       Mon Oct 27 10:55:51 2008 -0700
> >+++ b/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp       Mon Oct 27 17:02:38 2008 -0700
> >-#define MAX_PATH_COMPONENTS 4
> >+#define MAX_PATH_COMPONENTS 5
> 
> Could you comment on this change somehow?

Err, yes:  that was part of the unrelated test from comment #2, I'll post a new patch with just the pref changes.

> BTW: Where is the specification of the new "safebrowsing" protocol (v2.2)?
> http://code.google.com/p/google-safe-browsing/wiki/Protocolv2Spec says at the
> moment: "Client specification for the Google Safe Browsing v2.1 protocol".

I'll bug google to update it.

There are actually no significant protocol changes in 2.2.  There has been one behavior change on the server - the server can send an empty add chunk as a way to fill in gaps in the chunk list.  Previously after some expiration, the add chunk list would say something like "1-2,4-10,13-50".  With server version 2.2, they might send an empty chunk for 3, 11, and 12, so that the client will say it has 1-50.  This is just to keep the request size smaller.

There's nothing in the spec that actually needs to change (empty add chunks are perfectly legal in 2.1).  All that they might consider adding is a recommendation that an empty add chunk should be allowed to expire subs that were supposed to apply to that add chunk.  It's this change that I refer to as "the fixes needed to support 2.2 on trunk".
(Assignee)

Comment 5

9 years ago
Created attachment 345753 [details] [diff] [review]
the right patch

just the prefs, none of the crap.
Attachment #345025 - Attachment is obsolete: true
(In reply to comment #4)
> (...)
Yeah, some time ago I noticed similar changes (regarding zero size chunks) in Google Chrome too (http://src.chromium.org/viewvc/chrome?view=rev&sortby=date&revision=3131)...
Thanks for your explanation.
(Assignee)

Comment 7

9 years ago
Landed as http://hg.mozilla.org/mozilla-central/rev/ee04803822c9
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Flags: blocking-firefox3.1? → blocking-firefox3.1+
(Assignee)

Comment 8

9 years ago
Created attachment 347892 [details] [diff] [review]
branch patch

Branch patch includes a low-risk subset of the patch from bug 459281, which has been on trunk for a few weeks now.
Attachment #347892 - Flags: review?(tony)
(Assignee)

Comment 9

9 years ago
Asking for this to block 3.0.5 at google's request.  Switching to the 2.2 servers will significantly reduce bandwidth usage at google's end (and for individual users), due to less fragmented chunk requests.
Flags: blocking1.9.0.5?

Updated

9 years ago
Attachment #347892 - Flags: review?(tony) → review+

Comment 10

9 years ago
Comment on attachment 347892 [details] [diff] [review]
branch patch

LG

Are the tests not portable?
Assignee: nobody → dcamp
Flags: blocking1.9.0.5? → blocking1.9.0.5+
Yes, please, we want some tests for the 1.9.0 branch
Code freeze is on Monday, and although we've marked it blocking we probably wouldn't really hold the release for this so please make sure you don't miss.
(Assignee)

Comment 12

9 years ago
Created attachment 348093 [details] [diff] [review]
branch patch, now with tests

New branch patch backports the relevant test too.
Attachment #347892 - Attachment is obsolete: true
Attachment #348093 - Flags: review?(tony)

Updated

9 years ago
Attachment #348093 - Attachment is patch: true
Attachment #348093 - Attachment mime type: application/octet-stream → text/plain

Updated

9 years ago
Attachment #348093 - Flags: review?(tony) → review+
(Assignee)

Updated

9 years ago
Attachment #348093 - Flags: approval1.9.0.5?
Comment on attachment 348093 [details] [diff] [review]
branch patch, now with tests

Approved for 1.9.0.5, a=dveditz for release-drivers
Attachment #348093 - Flags: approval1.9.0.5? → approval1.9.0.5+
(Assignee)

Comment 14

9 years ago
Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--  firefox.js
new revision: 1.339; previous revision: 1.338
done
Checking in toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp;
/cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp,v  <--  nsUrlClassifierDBService.cpp
new revision: 1.82; previous revision: 1.81
done
Checking in toolkit/components/url-classifier/tests/unit/test_addsub.js;
/cvsroot/mozilla/toolkit/components/url-classifier/tests/unit/test_addsub.js,v  <--  test_addsub.js
new revision: 1.8; previous revision: 1.7
done
Keywords: fixed1.9.0.5
Verified changes with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5pre) Gecko/2008120105 GranParadiso/3.0.5pre.
Keywords: fixed1.9.0.5 → verified1.9.0.5
Keywords: fixed1.9.1
Component: Phishing Protection → Phishing Protection
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.