Fix a crash in ContentSettingsTypeFromGroupName()

The base::StringPiece::operator== causes a conversion of nullptr to
a base::StringPiece, which CHECK()s. Make it fall through
to NOTREACHED() instead.

Also fix braces around if(){} and a range loop as per newer code
style guidelines.

Bug: 1382370
Change-Id: I60df24e522c5270f6d6cd93d9e3e7235b04887d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4236624
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1110294}

chrome/browser/ui/webui/settings/site_settings_handler.cc

@@ -320,12 +320,9 @@ std::string ConvertEtldToOrigin(const std::string etld_plus1, bool secure) {
 }
 
 bool IsPatternValidForType(const std::string& pattern_string,
-                           const std::string& type,
+                           ContentSettingsType content_type,
                            Profile* profile,
                            std::string* out_error) {
-  ContentSettingsType content_type =
-      site_settings::ContentSettingsTypeFromGroupName(type);
-
   ContentSettingsPattern pattern =
       ContentSettingsPattern::FromString(pattern_string);
 
@@ -1472,6 +1469,8 @@ void SiteSettingsHandler::HandleGetOriginPermissions(
       type = *maybe_type;
     ContentSettingsType content_type =
         site_settings::ContentSettingsTypeFromGroupName(type);
+    CHECK(content_type != ContentSettingsType::DEFAULT)
+        << type << " is not expected to have a UI representation.";
     HostContentSettingsMap* map =
         HostContentSettingsMapFactory::GetForProfile(profile_);
 
@@ -1595,8 +1594,11 @@ void SiteSettingsHandler::HandleSetOriginPermissions(
   CHECK(content_settings::ContentSettingFromString(value, &setting));
   std::vector<ContentSettingsType> types;
   if (type_string) {
-    types.push_back(
-        site_settings::ContentSettingsTypeFromGroupName(*type_string));
+    ContentSettingsType content_type =
+        site_settings::ContentSettingsTypeFromGroupName(*type_string);
+    CHECK(content_type != ContentSettingsType::DEFAULT)
+        << *type_string << " is not expected to have a UI representation.";
+    types.push_back(content_type);
   } else {
     // Clear device chooser data permission exceptions.
     if (setting == CONTENT_SETTING_DEFAULT) {
@@ -1691,6 +1693,8 @@ void SiteSettingsHandler::HandleResetCategoryPermissionForPattern(
 
   ContentSettingsType content_type =
       site_settings::ContentSettingsTypeFromGroupName(type);
+  CHECK(content_type != ContentSettingsType::DEFAULT)
+      << type << " is not expected to have a UI representation.";
 
   Profile* profile = nullptr;
   if (incognito) {
@@ -1753,6 +1757,8 @@ void SiteSettingsHandler::HandleSetCategoryPermissionForPattern(
 
   ContentSettingsType content_type =
       site_settings::ContentSettingsTypeFromGroupName(type);
+  CHECK(content_type != ContentSettingsType::DEFAULT)
+      << type << " is not expected to have a UI representation.";
   ContentSetting setting;
   CHECK(content_settings::ContentSettingFromString(value, &setting));
 
@@ -1937,11 +1943,16 @@ void SiteSettingsHandler::HandleIsPatternValidForType(
   CHECK_EQ(3U, args.size());
   const base::Value& callback_id = args[0];
   const std::string& pattern_string = args[1].GetString();
-  const std::string& type = args[2].GetString();
+  const std::string& type_string = args[2].GetString();
+
+  ContentSettingsType content_type =
+      site_settings::ContentSettingsTypeFromGroupName(type_string);
+  CHECK(content_type != ContentSettingsType::DEFAULT)
+      << type_string << " is not expected to have a UI representation.";
 
   std::string reason;
   bool is_valid =
-      IsPatternValidForType(pattern_string, type, profile_, &reason);
+      IsPatternValidForType(pattern_string, content_type, profile_, &reason);
 
   base::Value::Dict return_value;
   return_value.Set(kIsValidKey, base::Value(is_valid));

chrome/browser/ui/webui/settings/site_settings_helper.cc

@@ -372,12 +372,16 @@ bool HasRegisteredGroupName(ContentSettingsType type) {
 }
 
 ContentSettingsType ContentSettingsTypeFromGroupName(base::StringPiece name) {
-  for (size_t i = 0; i < std::size(kContentSettingsTypeGroupNames); ++i) {
-    if (name == kContentSettingsTypeGroupNames[i].name)
-      return kContentSettingsTypeGroupNames[i].type;
+  for (const auto& entry : kContentSettingsTypeGroupNames) {
+    // Content setting types that aren't represented in the settings UI
+    // will have `nullptr` as their `name`. However, converting `nullptr`
+    // to a StringPiece will crash, so we have to handle it explicitly
+    // before comparing.
+    if (entry.name != nullptr && entry.name == name) {
+      return entry.type;
+    }
   }
 
-  NOTREACHED() << name << " is not a recognized content settings type.";
   return ContentSettingsType::DEFAULT;
 }
 
@@ -974,6 +978,7 @@ base::Value::List GetChooserExceptionListFromProfile(
   base::Value::List exceptions;
   ContentSettingsType content_type =
       ContentSettingsTypeFromGroupName(std::string(chooser_type.name));
+  DCHECK(content_type != ContentSettingsType::DEFAULT);
 
   // The BluetoothChooserContext is only available when the
   // WebBluetoothNewPermissionsBackend flag is enabled.

chrome/browser/ui/webui/settings/site_settings_permissions_handler.cc

@@ -195,9 +195,12 @@ SiteSettingsPermissionsHandler::GetUnusedSitePermissionsFromDict(
   std::set<ContentSettingsType> permission_types;
   for (const auto& permission : *permissions) {
     CHECK(permission.is_string());
-    const std::string& type = permission.GetString();
-    permission_types.insert(
-        site_settings::ContentSettingsTypeFromGroupName(type));
+    const std::string& type_string = permission.GetString();
+    ContentSettingsType type =
+        site_settings::ContentSettingsTypeFromGroupName(type_string);
+    CHECK(type != ContentSettingsType::DEFAULT)
+        << type_string << " is not expected to have a UI representation.";
+    permission_types.insert(type);
   }
 
   const base::Value* js_expiration =