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* g_whitelisted_extension_id = NULL;
+
 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 IsWhitelistedForTest(const std::string& extension_id) {
+  if (!g_whitelisted_extension_id) {

[not at google - send to devlin, 2015/04/09 14:36:52]

TODO(jackhou): Delete the commandline whitelisting mechanism.

[jackhou1, 2015/04/10 01:45:31]

Done.

+    g_whitelisted_extension_id = new std::string(
+        base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+            switches::kWhitelistedExtensionID));
+  }
+  return !g_whitelisted_extension_id->empty() &&

[not at google - send to devlin, 2015/04/09 14:36:52]

I don't think the .empty() check here is necessary. Hopefully it can never be
true, and even if it is, the .empty() check isn't any more efficient than the
first iteration of an equality comparison, I would assume.

[tapted, 2015/04/09 14:56:46]

yeah.. I suggested this nugget of paranoia :)

IsIdInWhitelist -> IsIdInList has a check, `if
!IsValidExtensionId(extension_id)`. IsWhitelistedForTest doesn't have this
check.

The worry is that if something managed to get here with an extension id of "",
it would get whitelisted by default (since it's indistinguishable from the would
match the no-command-line case).

[not at google - send to devlin, 2015/04/09 15:01:28]

ok :-) seems like a CHECK (or early-ignore) situation somewhere, but this test
is a nice bit of paranoia either way.

+         *g_whitelisted_extension_id == extension_id;
+}
+
 }  // namespace
 
+SimpleFeature::ScopedWhitelistForTest::ScopedWhitelistForTest(
+    const std::string& id)
+    : previous_id_(g_whitelisted_extension_id) {
+  g_whitelisted_extension_id = new std::string(id);
+}
+
+SimpleFeature::ScopedWhitelistForTest::~ScopedWhitelistForTest() {
+  delete g_whitelisted_extension_id;
+  g_whitelisted_extension_id = previous_id_;
+}
+
 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;
 
     contexts["blessed_extension"] = Feature::BLESSED_EXTENSION_CONTEXT;
@@ skipping 119 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) &&
+      !IsWhitelistedForTest(extension_id)) {
+    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 226 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