Parse the whitelist expressions from the phishing verdict response if

Parse the whitelist expressions from the phishing verdict response if
there are any and check to see whether the current URL matches any of
these whitelisted expressions.

BUG=
TEST=ClientSideDetectionServiceTest


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98919

[mattm, 2011-08-30 02:23:08]

http://codereview.chromium.org/7792004/diff/1/chrome/browser/safe_browsing/cl...
File chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
(right):

http://codereview.chromium.org/7792004/diff/1/chrome/browser/safe_browsing/cl...
chrome/browser/safe_browsing/client_side_detection_service_unittest.cc:792: url,
response));
The whitelist matching against the URL seems a bit weird, but maybe I'm not
seeing the use case.

[noelutz, 2011-08-30 16:45:11]

http://codereview.chromium.org/7792004/diff/1/chrome/browser/safe_browsing/cl...
File chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
(right):

http://codereview.chromium.org/7792004/diff/1/chrome/browser/safe_browsing/cl...
chrome/browser/safe_browsing/client_side_detection_service_unittest.cc:792: url,
response));
On 2011/08/30 02:23:09, mattm wrote:
> The whitelist matching against the URL seems a bit weird, but maybe I'm not
> seeing the use case.

This whitelist works in two ways.  I.e., if we 'classify' http://www.a.com/b/ as
phishing then we want to be able to whitelist it either by adding a.com/ to the
whitelist or a more specific expression www.a.com/b/index.html.

[mattm, 2011-08-30 23:09:41]

LGTM

[Brian Ryner, 2011-08-31 00:16:33]

http://codereview.chromium.org/7792004/diff/1/chrome/browser/safe_browsing/cl...
File chrome/browser/safe_browsing/client_side_detection_service.cc (right):

http://codereview.chromium.org/7792004/diff/1/chrome/browser/safe_browsing/cl...
chrome/browser/safe_browsing/client_side_detection_service.cc:626: std::string
canonical_url = host + path + query;
Maybe call this canonical_url_as_pattern or something, since it's not really a
full url (no protocol)?

http://codereview.chromium.org/7792004/diff/1/chrome/browser/safe_browsing/cl...
chrome/browser/safe_browsing/client_side_detection_service.cc:632: GURL
whitelisted_url = GURL(std::string("http://") +
Can't you just do:

GURL whitelisted_url(std::string("http://") + ... );

?

http://codereview.chromium.org/7792004/diff/1/chrome/browser/safe_browsing/cl...
chrome/browser/safe_browsing/client_side_detection_service.cc:635: continue;  //
Skip invalid whitelist expressions.
We never expect this to happen, right?  I wonder if we should log a histogram
for it?

[noelutz, 2011-08-31 00:53:50]

Thanks.  Please take another look.

noe.

http://codereview.chromium.org/7792004/diff/1/chrome/browser/safe_browsing/cl...
File chrome/browser/safe_browsing/client_side_detection_service.cc (right):

http://codereview.chromium.org/7792004/diff/1/chrome/browser/safe_browsing/cl...
chrome/browser/safe_browsing/client_side_detection_service.cc:626: std::string
canonical_url = host + path + query;
On 2011/08/31 00:16:33, Brian Ryner wrote:
> Maybe call this canonical_url_as_pattern or something, since it's not really a
> full url (no protocol)?

Done.

http://codereview.chromium.org/7792004/diff/1/chrome/browser/safe_browsing/cl...
chrome/browser/safe_browsing/client_side_detection_service.cc:632: GURL
whitelisted_url = GURL(std::string("http://") +
On 2011/08/31 00:16:33, Brian Ryner wrote:
> Can't you just do:
> 
> GURL whitelisted_url(std::string("http://") + ... );
> 
> ?

Done.

http://codereview.chromium.org/7792004/diff/1/chrome/browser/safe_browsing/cl...
chrome/browser/safe_browsing/client_side_detection_service.cc:635: continue;  //
Skip invalid whitelist expressions.
On 2011/08/31 00:16:33, Brian Ryner wrote:
> We never expect this to happen, right?  I wonder if we should log a histogram
> for it?

Done.

[Brian Ryner, 2011-08-31 00:56:55]

LGTM

[commit-bot: I haz the power, 2011-08-31 02:59:49]

Change committed as 98919