Signal extension API schema corruption to the browser process.

extensions/common/extension_api.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "extensions/common/extension_api.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <string>
@@ skipping 37 matching lines @@
 std::unique_ptr<base::ListValue> LoadSchemaList(
     const std::string& name,
     const base::StringPiece& schema) {
   std::string error_message;
   std::unique_ptr<base::Value> result(base::JSONReader::ReadAndReturnError(
       schema,
       base::JSON_PARSE_RFC | base::JSON_DETACHABLE_CHILDREN,  // options
       NULL,                                                   // error code
       &error_message));
 
-  // Tracking down http://crbug.com/121424
-  char buf[128];
-  base::snprintf(buf, arraysize(buf), "%s: (%d) '%s'",
-      name.c_str(),
-      result.get() ? result->GetType() : -1,
-      error_message.c_str());
+#if DCHECK_IS_ON()
+  // |schema| comes from JSON hard-coded to the Chrome binary, so in principle
+  // failure to parse indicates we generated corrupted JSON, but in practice
+  // corruption of the Chrome binary (e.g. disk errors) can also break it.
+  // See http://crbug.com/121424.
+  if (!result || !result->IsType(base::Value::TYPE_LIST)) {
+    char buf[128];
+    base::snprintf(buf, arraysize(buf), "%s: (%d) '%s'",
+                   name.c_str(),
+                   result.get() ? result->GetType() : -1,
+                   error_message.c_str());
 
-  CHECK(result.get()) << error_message << " for schema " << schema;
-  CHECK(result->IsType(base::Value::TYPE_LIST)) << " for schema " << schema;
+    DCHECK(result);
+    DCHECK(result->IsType(base::Value::TYPE_LIST));
+  }
+#endif // DCHECK_IS_ON()
+
+  // From() returns null if pass a null or non-list value.

[Devlin, 2016/08/19 15:02:32]

nit: "passed"

[Wez, 2016/08/19 18:22:15]

Done.

   return base::ListValue::From(std::move(result));
 }
 
 const base::DictionaryValue* FindListItem(const base::ListValue* list,
                                           const std::string& property_name,
                                           const std::string& property_value) {
   for (size_t i = 0; i < list->GetSize(); ++i) {
     const base::DictionaryValue* item = NULL;
     CHECK(list->GetDictionary(i, &item))
         << property_value << "/" << property_name;
@@ skipping 130 matching lines @@
   g_shared_instance_for_test = testing_api;
 }
 
 ExtensionAPI::OverrideSharedInstanceForTest::~OverrideSharedInstanceForTest() {
   g_shared_instance_for_test = original_api_;
 }
 
 void ExtensionAPI::LoadSchema(const std::string& name,
                               const base::StringPiece& schema) {
   std::unique_ptr<base::ListValue> schema_list(LoadSchemaList(name, schema));
+  if (!schema_list)
+    return;
+
   std::string schema_namespace;
   extensions::ExtensionsClient* extensions_client =
       extensions::ExtensionsClient::Get();
   DCHECK(extensions_client);
   while (!schema_list->empty()) {
     std::unique_ptr<base::DictionaryValue> schema;
     {
       std::unique_ptr<base::Value> val;
       schema_list->Erase(schema_list->begin(), &val);
       schema = base::DictionaryValue::From(std::move(val));
@@ skipping 97 matching lines @@
       LoadSchema(maybe_schema_resource->first,
                  ReadFromResource(maybe_schema_resource->second));
     } else if (default_configuration_initialized_ &&
                extensions_client->IsAPISchemaGenerated(api_name)) {
       LoadSchema(api_name, extensions_client->GetAPISchema(api_name));
     } else {
       return NULL;
     }
 
     maybe_schema = schemas_.find(api_name);
-    CHECK(schemas_.end() != maybe_schema);
+    // If the schema failed to load then return null for it, rather than
+    // crashing, so that the browser can be notified of the problem.
+    // See http://crbug.com/121424.
+    if (schemas_.end() == maybe_schema)
+      return NULL;
+
     result = maybe_schema->second.get();
   }
 
   if (!child_name.empty())
     result = GetSchemaChild(result, child_name);
 
   return result;
 }
 
+void ExtensionAPI::LoadSchemaListOrDie(const std::string& api) {
+  std::string api_name = GetAPINameFromFullName(api, NULL);

[Devlin, 2016/08/19 15:02:32]

nullptr

[Wez, 2016/08/19 18:22:15]

Sticking with NULL for consistency with the rest of this file; we can migrate
NULL->nullptr as a follow-up.

[Devlin, 2016/08/19 18:40:57]

(Not blocking this change - feel free to ignore) I thought the decision was that
we preferred nullptr in all new code (even new code in existing files), but
didn't want to do mass updates (primarily for git-blame reasons)?  In which
case, putting nullptr here would be correct.

+  extensions::ExtensionsClient* extensions_client =
+      extensions::ExtensionsClient::Get();
+  // IsAPISchemaGenerated() will return false for invalid |api| names.
+  if (!extensions_client->IsAPISchemaGenerated(api))

[Devlin, 2016/08/19 15:02:32]

We aren't checking the unloaded schemas here.  That won't be a problem after
https://codereview.chromium.org/2257003002/ (done for orthogonal performance
reasons), but I don't plan to merge that back, so this should probably handle
that case.

[Wez, 2016/08/19 18:22:15]

Ah, OK, so we need to check for un-generated (why do we call them "unloaded,
BTW?) schemas, which are string resources in the binary.

Looking at the code for that, I notice that things are expecting that
InitDefaultConfiguration() has been called - I don't think we need to worry
about that here (it always will have been called, and if it hasn't been called
then things will safely no-op). Am I missing something?

[Devlin, 2016/08/19 18:40:57]

re unloaded, no idea - the whole concept doesn't make sense (which is why I'm
killing them :))
InitDefaultConfiguration is called when we create the ExtensionAPI, so we should
be good.

+    return;
+
+  CHECK(LoadSchemaList(api_name, extensions_client->GetAPISchema(api_name)));
+}
+
 Feature* ExtensionAPI::GetFeatureDependency(const std::string& full_name) {
   std::string feature_type;
   std::string feature_name;
   SplitDependencyName(full_name, &feature_type, &feature_name);
 
   FeatureProviderMap::iterator provider =
       dependency_providers_.find(feature_type);
   if (provider == dependency_providers_.end())
     return NULL;
 
@@ skipping 29 matching lines @@
       return result;
     }
 
     size_t last_dot_index = api_name_candidate.rfind('.');
     if (last_dot_index == std::string::npos)
       break;
 
     api_name_candidate = api_name_candidate.substr(0, last_dot_index);
   }
 
-  *child_name = "";
+  if (child_name)
+    *child_name = "";
+
   return std::string();
 }
 
 }  // namespace extensions