Fix b180514: Some exception entries cannot be deleted from a setting panel.

chrome/common/content_settings_pattern_parser.cc

Index: chrome/common/content_settings_pattern_parser.cc
diff --git a/chrome/common/content_settings_pattern_parser.cc b/chrome/common/content_settings_pattern_parser.cc
index 2f823cc61ce928266f879fb5bf19d8e1d8b788f6..f5542b5c9b2cd428037ea8193261821c856952eb 100644
--- a/chrome/common/content_settings_pattern_parser.cc
+++ b/chrome/common/content_settings_pattern_parser.cc
@@ -139,6 +139,11 @@ void PatternParser::Parse(const std::string& pattern_spec,
   if (host_component.IsNonEmpty()) {
     std::string host = pattern_spec.substr(host_component.start,
                                            host_component.len);
+    if (host == "." || host.find("..") != std::string::npos) {

[jochen (gone - plz use gerrit), 2013/04/03 09:12:36]

sorry for being late to the party

I would expect a check like this in ContentSettingsPattern::Builder::Validate()

At that point, we could do a more general check (because the pattern is already
canonicalized), and check that canonicalizing the pattern again doesn't change
it.

[markusheintz_, 2013/04/03 09:48:40]

Do you have any particular checks do you have in mind?

The two consecutive dots are an illegal substring. So while parsing the pattern
string seems the right place to do this check. I don't want to split the parsing
in two files. The Parser is the right place to do all the string parsing.

GURL("a..a") and GURL("A..") are illegal as well. Maybe we should add checks for
that to 
content_settings_pattern_unittest.cc. There is a test call GURL in this file.

On 2013/04/03 09:12:36, jochen wrote:

[yhirano, 2013/04/03 09:59:10]

Let me clarify.
As you say, Builder::Validate() is processed after the canonicalization. So, the
pattern will be changed as:
 "www.example.com.." => "www.example.com."
 "www.example.com." => "www.example.com"
 "www..example.com" => "www..example.com"
 "." => "."
 ".." => "."
.

Hence, if we want to validate as same as this patch, we should check
(1) The (modified) host doesn't have consecutive dots, and
(2) The host doesn't end with a dot

Do you think it is better than this patch?

  check that canonicalizing the pattern again doesn't change it.
Note that there are some points which should be considered:
(a) According to markusheintz_@'s suggestion, the current patch does not permit
patterns with a host having consecutive dots, such as "www..example.com". 
(b) net::TrimEndingDot does nothing for a host consisting of a single dot.
Hence, ".." will be canonicalized to ".", which will be canonicalized to itself.

For these two cases, idempotence check does not work, for at least, current
canonicalization.

+      builder->Invalid();
+      return;
+    }
+
     if (host == kHostWildcard) {
       builder->WithDomainWildcard();
     } else if (StartsWithASCII(host, kDomainWildcard, true)) {