[Extensions] Custom manifest keys for invalid host scheme warning

Currently, the install warning message for a host with an invalid scheme
explicitly mentions the "permissions" key even if the host is specified
inside optional_permissions or host_permissions. This CL un-hardcodes
warning's message so it can specify other manifest keys.

Bug: 1136993
Change-Id: I52eb1e00fd6d357742018c2f25f256237cd25483
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2473584
Commit-Queue: Kelvin Jiang <kelvinjiang@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817744}

chrome/common/extensions/manifest_tests/extension_manifests_chromepermission_unittest.cc

@@ -24,10 +24,11 @@ namespace errors = manifest_errors;
 typedef ChromeManifestTest ChromePermissionManifestTest;
 
 TEST_F(ChromePermissionManifestTest, ChromeURLPermissionInvalid) {
-  LoadAndExpectWarning("permission_chrome_url_invalid.json",
-                       ErrorUtils::FormatErrorMessage(
-                           errors::kInvalidPermissionScheme,
-                           chrome::kChromeUINewTabURL));
+  LoadAndExpectWarning(
+      "permission_chrome_url_invalid.json",
+      ErrorUtils::FormatErrorMessage(errors::kInvalidPermissionScheme,
+                                     manifest_keys::kPermissions,
+                                     chrome::kChromeUINewTabURL));
 }
 
 TEST_F(ChromePermissionManifestTest, ChromeUntrustedURLPermissionInvalid) {
@@ -71,7 +72,7 @@ TEST_F(ChromePermissionManifestTest,
   LoadAndExpectWarning("permission_chrome_resources_url.json",
                        ErrorUtils::FormatErrorMessage(
                            errors::kInvalidPermissionScheme,
-                           "chrome://resources/"));
+                           manifest_keys::kPermissions, "chrome://resources/"));
   std::string error;
   LoadExtension(ManifestData("permission_chrome_resources_url.json"),
                 &error,

chrome/common/extensions/manifest_tests/permissions_parser_unittest.cc

@@ -112,6 +112,16 @@ TEST_F(PermissionsParserTest, OptionalHostPermissionsAllURLs) {
               testing::UnorderedElementsAre("*://*/*"));
 }
 
+TEST_F(PermissionsParserTest, OptionalHostPermissionsInvalidScheme) {
+  std::vector<std::string> expected_warnings;
+  expected_warnings.push_back(ErrorUtils::FormatErrorMessage(
+      manifest_errors::kInvalidPermissionScheme,
+      manifest_keys::kOptionalPermissions, "chrome://extensions/"));
+
+  scoped_refptr<Extension> extension(LoadAndExpectWarnings(
+      "optional_permissions_invalid_scheme.json", expected_warnings));
+}
+
 TEST_F(PermissionsParserTest, HostPermissionsKey) {
   std::vector<std::string> expected_warnings;
   expected_warnings.push_back(ErrorUtils::FormatErrorMessage(
@@ -142,6 +152,18 @@ TEST_F(PermissionsParserTest, HostPermissionsKeyInvalidHosts) {
       "host_permissions_key_invalid_hosts.json", expected_warnings));
 }
 
+TEST_F(PermissionsParserTest, HostPermissionsKeyInvalidScheme) {
+  std::vector<std::string> expected_warnings;
+  expected_warnings.push_back(ErrorUtils::FormatErrorMessage(
+      manifest_errors::kInvalidPermissionScheme,
+      manifest_keys::kHostPermissions, "chrome://extensions/"));
+
+  expected_warnings.push_back(kManifestVersionWarning);
+
+  scoped_refptr<Extension> extension(LoadAndExpectWarnings(
+      "host_permissions_key_invalid_scheme.json", expected_warnings));
+}
+
 // Tests that listing a permissions as optional when that permission cannot be
 // optional produces a warning and doesn't add the permission.
 TEST_F(PermissionsParserTest, UnsupportedOptionalPermissionWarning) {
@@ -188,8 +210,13 @@ TEST_F(PermissionsParserTest, ChromeFavicon) {
   };
 
   auto has_install_warning = [](const Extension& extension) {
+    const char* permissions_key = extension.manifest_version() > 2
+                                      ? manifest_keys::kHostPermissions
+                                      : manifest_keys::kPermissions;
+
     InstallWarning expected_warning(ErrorUtils::FormatErrorMessage(
-        manifest_errors::kInvalidPermissionScheme, kFaviconPattern));
+        manifest_errors::kInvalidPermissionScheme, permissions_key,
+        kFaviconPattern));
     return base::Contains(extension.install_warnings(), expected_warning);
   };
 

chrome/common/extensions/permissions/permissions_data_unittest.cc

@@ -574,7 +574,7 @@ TEST_F(ExtensionScriptAndCaptureVisibleTest, Permissions) {
   EXPECT_FALSE(warnings.empty());
   EXPECT_EQ(ErrorUtils::FormatErrorMessage(
                 manifest_errors::kInvalidPermissionScheme,
-                "chrome://*/"),
+                manifest_keys::kPermissions, "chrome://*/"),
             warnings[0].message);
   EXPECT_EQ(DISALLOWED, GetExtensionAccess(extension.get(), settings_url));
   EXPECT_EQ(DISALLOWED,

chrome/test/data/extensions/manifest_tests/host_permissions_key_invalid_scheme.json

+{
+  "name": "Host permissions with invalid scheme",
+  "version": "0.1",
+  "manifest_version": 3,
+  "description": "Extension with a pattern with an invalid scheme in host_permissions",
+  "host_permissions": [
+    "chrome://extensions/"
+  ]
+}

chrome/test/data/extensions/manifest_tests/optional_permissions_invalid_scheme.json

+{
+  "name": "Optional host permissions with invalid scheme",
+  "version": "0.1",
+  "manifest_version": 2,
+  "description": "Extension with a pattern with an invalid scheme in optional_permissions",
+  "optional_permissions": [
+    "chrome://extensions/"
+  ]
+}

extensions/common/manifest_constants.cc

@@ -611,8 +611,7 @@ const char kInvalidPermission[] =
     "Invalid value for 'permissions[*]'.";
 const char kInvalidPermissions[] =
     "Invalid value for 'permissions'.";
-const char kInvalidPermissionScheme[] =
-    "Invalid scheme for 'permissions[*]'.";
+const char kInvalidPermissionScheme[] = "Invalid scheme for '*[*]'.";
 const char kInvalidReplacementAndroidApp[] =
     "Invalid value for 'replacement_android_app'";
 const char kInvalidReplacementWebApp[] =

extensions/common/manifest_handlers/permissions_parser.cc

@@ -169,7 +169,7 @@ void ParseHostPermissions(Extension* extension,
         // below).
         extension->AddInstallWarning(InstallWarning(
             ErrorUtils::FormatErrorMessage(errors::kInvalidPermissionScheme,
-                                           permission_str),
+                                           key, permission_str),
             key, permission_str));
         continue;
       }