Cache --whitelisted-extension-id in SimpleFeature.

extensions/common/features/simple_feature.cc

 // Copyright 2014 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/features/simple_feature.h"
 
 #include <algorithm>
 #include <map>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/debug/alias.h"
+#include "base/macros.h"
 #include "base/sha1.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "extensions/common/extension_api.h"
 #include "extensions/common/features/feature_provider.h"
 #include "extensions/common/switches.h"
 
 namespace extensions {
 
 namespace {
 
+// A singleton copy of the --whitelisted-extension-id so that we don't need to
+// copy it from the CommandLine each time.
+std::string* whitelisted_extension_id = NULL;

[not at google - send to devlin, 2015/04/08 17:59:22]

should be g_whitelisted_extension_id;

[jackhou1, 2015/04/09 07:23:28]

Done.

+
 Feature::Availability IsAvailableToManifestForBind(
     const std::string& extension_id,
     Manifest::Type type,
     Manifest::Location location,
     int manifest_version,
     Feature::Platform platform,
     const Feature* feature) {
   return feature->IsAvailableToManifest(
       extension_id, type, location, manifest_version, platform);
 }
@@ skipping 178 matching lines @@
 
 bool IsCommandLineSwitchEnabled(const std::string& switch_name) {
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(switch_name + "=1"))
     return true;
   if (command_line->HasSwitch(std::string("enable-") + switch_name))
     return true;
   return false;
 }
 
+bool IsWhitelistedByCommandLine(const std::string& extension_id) {
+  if (!whitelisted_extension_id)
+    SimpleFeature::SetWhitelistedExtensionId(
+        base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+            switches::kWhitelistedExtensionID));
+  return !whitelisted_extension_id->empty() &&
+         *whitelisted_extension_id == extension_id;
+}
+
 }  // namespace
 
 struct SimpleFeature::Mappings {
   Mappings() {
     extension_types["extension"] = Manifest::TYPE_EXTENSION;
     extension_types["theme"] = Manifest::TYPE_THEME;
     extension_types["legacy_packaged_app"] = Manifest::TYPE_LEGACY_PACKAGED_APP;
     extension_types["hosted_app"] = Manifest::TYPE_HOSTED_APP;
     extension_types["platform_app"] = Manifest::TYPE_PLATFORM_APP;
     extension_types["shared_module"] = Manifest::TYPE_SHARED_MODULE;
@@ skipping 121 matching lines @@
   if (IsIdInBlacklist(extension_id))
     return CreateAvailability(FOUND_IN_BLACKLIST, type);
 
   // TODO(benwells): don't grant all component extensions.
   // See http://crbug.com/370375 for more details.
   // Component extensions can access any feature.
   // NOTE: Deliberately does not match EXTERNAL_COMPONENT.
   if (component_extensions_auto_granted_ && location == Manifest::COMPONENT)
     return CreateAvailability(IS_AVAILABLE, type);
 
-  if (!whitelist_.empty()) {
-    if (!IsIdInWhitelist(extension_id)) {
-      // TODO(aa): This is gross. There should be a better way to test the
-      // whitelist.
-      base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-      if (!command_line->HasSwitch(switches::kWhitelistedExtensionID))
-        return CreateAvailability(NOT_FOUND_IN_WHITELIST, type);
-
-      std::string whitelist_switch_value =
-          base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-              switches::kWhitelistedExtensionID);
-      if (extension_id != whitelist_switch_value)
-        return CreateAvailability(NOT_FOUND_IN_WHITELIST, type);
-    }
+  if (!whitelist_.empty() && !IsIdInWhitelist(extension_id) &&
+      !IsWhitelistedByCommandLine(extension_id)) {

[not at google - send to devlin, 2015/04/08 17:59:22]

"IsWhitelistedByCommandLine" isn't accurate anymore, more
"IsWhitelistedGlobally" perhaps. Or, in the hope that these really are only used
for testing, "IsWhitelistedForTest"?

[jackhou1, 2015/04/09 07:23:28]

Done.

+    return CreateAvailability(NOT_FOUND_IN_WHITELIST, type);
   }
 
   if (!MatchesManifestLocation(location))
     return CreateAvailability(INVALID_LOCATION, type);
 
   if (!platforms_.empty() && !ContainsValue(platforms_, platform))
     return CreateAvailability(INVALID_PLATFORM, type);
 
   if (min_manifest_version_ != 0 && manifest_version < min_manifest_version_)
     return CreateAvailability(INVALID_MIN_MANIFEST_VERSION, type);
@@ skipping 175 matching lines @@
     return false;
 
   const char* const* start = array;
   const char* const* end = array + array_length;
 
   return ((std::find(start, end, extension_id) != end) ||
           (std::find(start, end, HashExtensionId(extension_id)) != end));
 }
 
 // static
+void SimpleFeature::SetWhitelistedExtensionId(const std::string& id) {
+  if (whitelisted_extension_id)
+    delete whitelisted_extension_id;
+  whitelisted_extension_id = new std::string(id);

[not at google - send to devlin, 2015/04/08 17:59:22]

This will leak in the final unit test that calls SetWhitelistedExtensionId, but,
see the comment about Scoped.

[jackhou1, 2015/04/09 07:23:28]

Done.

+}
+
+// static
 bool SimpleFeature::IsIdInList(const std::string& extension_id,
                                const std::vector<std::string>& list) {
   if (!IsValidExtensionId(extension_id))
     return false;
 
   return (ContainsValue(list, extension_id) ||
           ContainsValue(list, HashExtensionId(extension_id)));
 }
 
 bool SimpleFeature::MatchesManifestLocation(
@@ skipping 31 matching lines @@
 bool SimpleFeature::IsValidExtensionId(const std::string& extension_id) {
   // Belt-and-suspenders philosophy here. We should be pretty confident by this
   // point that we've validated the extension ID format, but in case something
   // slips through, we avoid a class of attack where creative ID manipulation
   // leads to hash collisions.
   // 128 bits / 4 = 32 mpdecimal characters
   return (extension_id.length() == 32);
 }
 
 }  // namespace extensions