[Extensions] Add an install warning for too-high manifest versions

Add an install warning for extensions added with a higher manifest
version than is fully supported. We use a warning (rather than an
error) so the extension still loads, and runs as best it can.

Bug: 950647
Change-Id: I32ae2b21bb8cc83e2ec0effaab6d58324a93a25d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1559021Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649235}

extensions/common/extension.cc

@@ -78,13 +78,28 @@ bool ContainsReservedCharacters(const base::FilePath& path) {
 }
 
 // Returns true if the given |manifest_version| is supported for the specified
-// |type| of extension.
+// |type| of extension. Optionally populates |warning| if an InstallWarning
+// should be added.
 bool IsManifestSupported(int manifest_version,
                          Manifest::Type type,
-                         int creation_flags) {
+                         int creation_flags,
+                         std::string* warning) {
+  static constexpr int kMaximumSupportedManifestVersion = 2;
+  static_assert(kMaximumSupportedManifestVersion >= kModernManifestVersion,
+                "The modern manifest version must be supported.");
   // Modern is always safe.
-  if (manifest_version >= kModernManifestVersion)
+  if (manifest_version >= kModernManifestVersion &&
+      manifest_version <= kMaximumSupportedManifestVersion) {
     return true;
+  }
+
+  if (manifest_version > kMaximumSupportedManifestVersion) {
+    *warning = ErrorUtils::FormatErrorMessage(
+        manifest_errors::kManifestVersionTooHighWarning,
+        base::NumberToString(kMaximumSupportedManifestVersion),
+        base::NumberToString(manifest_version));
+    return true;
+  }
 
   // Allow an exception for extensions if a special commandline flag is present.
   // Note: This allows the extension to load, but it may effectively be treated
@@ -748,7 +763,9 @@ bool Extension::LoadManifestVersion(base::string16* error) {
   }
 
   manifest_version_ = manifest_->GetManifestVersion();
-  if (!IsManifestSupported(manifest_version_, GetType(), creation_flags_)) {
+  std::string warning;
+  if (!IsManifestSupported(manifest_version_, GetType(), creation_flags_,
+                           &warning)) {
     std::string json;
     base::JSONWriter::Write(*manifest_->value(), &json);
     LOG(WARNING) << "Failed to load extension.  Manifest JSON: " << json;
@@ -759,6 +776,9 @@ bool Extension::LoadManifestVersion(base::string16* error) {
     return false;
   }
 
+  if (!warning.empty())
+    AddInstallWarning(InstallWarning(warning, keys::kManifestVersion));
+
   return true;
 }
 

extensions/common/extension_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/command_line.h"
 #include "base/optional.h"
 #include "base/test/scoped_command_line.h"
+#include "extensions/common/manifest_constants.h"
 #include "extensions/common/switches.h"
 #include "extensions/common/value_builder.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -18,6 +19,7 @@ testing::AssertionResult RunManifestVersionSuccess(
     std::unique_ptr<base::DictionaryValue> manifest,
     Manifest::Type expected_type,
     int expected_manifest_version,
+    bool expect_warning = false,
     Extension::InitFromValueFlags custom_flag = Extension::NO_FLAGS) {
   std::string error;
   scoped_refptr<const Extension> extension = Extension::Create(
@@ -37,6 +39,19 @@ testing::AssertionResult RunManifestVersionSuccess(
            << "Wrong manifest version: " << extension->manifest_version();
   }
 
+  bool has_manifest_version_warning = false;
+  for (const auto& warning : extension->install_warnings()) {
+    if (warning.key == manifest_keys::kManifestVersion) {
+      has_manifest_version_warning = true;
+      break;
+    }
+  }
+
+  if (has_manifest_version_warning != expect_warning)
+    return testing::AssertionFailure()
+           << "Expected warning: " << expect_warning
+           << ", Found Warning: " << has_manifest_version_warning;
+
   return testing::AssertionSuccess();
 }
 
@@ -70,7 +85,8 @@ TEST(ExtensionTest, ExtensionManifestVersions) {
 
   const Manifest::Type kType = Manifest::TYPE_EXTENSION;
   EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(2), kType, 2));
-  EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(3), kType, 3));
+  EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(3), kType, 3,
+                                        true /* expect warning */));
 
   // Manifest v1 is deprecated, and should not load.
   EXPECT_TRUE(RunManifestVersionFailure(get_manifest(1)));
@@ -109,7 +125,8 @@ TEST(ExtensionTest, PlatformAppManifestVersions) {
 
   const Manifest::Type kType = Manifest::TYPE_PLATFORM_APP;
   EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(2), kType, 2));
-  EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(3), kType, 3));
+  EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(3), kType, 3,
+                                        true /* expect warning */));
   // Omitting the key defaults to v2 for platform apps.
   EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(base::nullopt), kType, 2));
 
@@ -146,7 +163,8 @@ TEST(ExtensionTest, HostedAppManifestVersions) {
 
   const Manifest::Type kType = Manifest::TYPE_HOSTED_APP;
   EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(2), kType, 2));
-  EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(3), kType, 3));
+  EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(3), kType, 3,
+                                        true /* expect warning */));
 
   // Manifest v1 is deprecated, but should still load for hosted apps.
   EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(1), kType, 1));
@@ -172,7 +190,8 @@ TEST(ExtensionTest, UserScriptManifestVersions) {
 
   const Manifest::Type kType = Manifest::TYPE_USER_SCRIPT;
   EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(2), kType, 2));
-  EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(3), kType, 3));
+  EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(3), kType, 3,
+                                        true /* expect warning */));
 
   // Manifest v1 is deprecated, but should still load for user scripts.
   EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(1), kType, 1));

extensions/common/manifest_constants.cc

@@ -714,6 +714,9 @@ const char kLocalesTreeMissing[] =
 const char kManifestParseError[] =
     "Manifest is not valid JSON.";
 const char kManifestUnreadable[] = "Manifest file is missing or unreadable";
+const char kManifestVersionTooHighWarning[] =
+    "The maximum currently-supported manifest version is *, but this is *.  "
+    "Certain features may not work as expected.";
 const char kMissingFile[] =
     "At least one js or css file is required for 'content_scripts[*]'.";
 const char kMultipleOverrides[] =

extensions/common/manifest_constants.h

@@ -484,6 +484,7 @@ extern const char kLocalesNoValidLocaleNamesListed[];
 extern const char kLocalesTreeMissing[];
 extern const char kManifestParseError[];
 extern const char kManifestUnreadable[];
+extern const char kManifestVersionTooHighWarning[];
 extern const char kMissingFile[];
 extern const char kMultipleOverrides[];
 extern const char kNoWildCardsInPaths[];