1323953 - Always send a 4-byte prefix when doing V4 completions

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect, P2)

Description

[Henry Chang [:hchang]]

Prior to v4, we already had 4 and 32 -byte prefix. However, while doing hash completion, we always send the first 4 bytes for the completion. [1] I tried
to change this behavior by using the actual matched length. For example, if
the prefix is 4 bytes, we send 4 bytes for completion; if the prefix is 7 bytes,
we send 7 bytes for completion.

I though the new behavior will not break anything but I got an xpcshell
test error (test_partial.js). In addition to this test error, we may also
have privacy issue when we send 32-byte prefix for completion. (Bug 1322532).
Chance are we have to send something shorter than the prefix. This bug is
to figure out how much of the prefix we should send for completion in
each case.


[1] https://dxr.mozilla.org/mozilla-central/rev/6dbc6e9f62a705d5f523cc750811bd01c8275ec6/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp#969

Comment 1

[François Marier [:francois]]

I will ask Google about this.

Comment 2

[François Marier [:francois]]

Google says that we should be able to send a 4-byte prefix regardless of the length of the prefix that got matched, as long as we use the full prefix to decide whether or not we need to do the gethash call.

Henry, if I understand you correctly, we currently only send 4-byte prefixes when doing completions in V4, right? This means we don't have to do anything and can just resolve this bug?

Comment 3

[Henry Chang [:hchang]]

(In reply to François Marier [:francois] from comment #2)
> Google says that we should be able to send a 4-byte prefix regardless of the
> length of the prefix that got matched, as long as we use the full prefix to
> decide whether or not we need to do the gethash call.
> 
> Henry, if I understand you correctly, we currently only send 4-byte prefixes
> when doing completions in V4, right? This means we don't have to do anything
> and can just resolve this bug?

No. We send as long as we match the prefix [1]. If we decide to always send 4 bytes
prefix to google server for privacy issue, I can fix it in this bug and get rid of 
the annoying if-else in [1] :). 

[1] https://dxr.mozilla.org/mozilla-central/rev/3cedab21a7e65e6a1c4c2294ecfb5502575a46e3/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp#992

Comment 4

[François Marier [:francois]]

(In reply to Henry Chang [:henry][:hchang] from comment #3)
> No. We send as long as we match the prefix [1]. If we decide to always send
> 4 bytes
> prefix to google server for privacy issue, I can fix it in this bug and get
> rid of 
> the annoying if-else in [1] :). 

Ok, sounds good, let's make it always 4-bytes.

Comment 5

[Henry Chang [:hchang]]

Attachment: Bug 1323953 - Send 4-byte prefix for both v2/v4 tables.

Review commit: https://reviewboard.mozilla.org/r/110246/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/110246/

Comment 6

[François Marier [:francois]]

Comment on attachment 8834236 [details]
Bug 1323953 - Send 4-byte prefix for both v2/v4 tables.

https://reviewboard.mozilla.org/r/110246/#review111362

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4f30316c8432
Send 4-byte prefix for both v2/v4 tables. r=francois

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4f30316c8432