Only whitelist messages.json files in _locales for content verification

extensions/browser/content_verifier.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/browser/content_verifier.h"
 
 #include <algorithm>
 #include <utility>
 #include <vector>
 
@@ skipping 95 matching lines @@
     const std::string& extension_id,
     const base::FilePath& extension_root,
     const base::FilePath& relative_path) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
   const ContentVerifierIOData::ExtensionData* data =
       io_data_->GetData(extension_id);
   if (!data)
     return NULL;
 
-  base::FilePath normalized_path = NormalizeRelativePath(relative_path);
+  base::FilePath normalized_unix_path = NormalizeRelativePath(relative_path);
 
-  std::set<base::FilePath> paths;
-  paths.insert(normalized_path);
-  if (!ShouldVerifyAnyPaths(extension_id, extension_root, paths))
+  std::set<base::FilePath> unix_paths;
+  unix_paths.insert(normalized_unix_path);
+  if (!ShouldVerifyAnyPaths(extension_id, extension_root, unix_paths))
     return NULL;
 
   // TODO(asargent) - we can probably get some good performance wins by having
   // a cache of ContentHashReader's that we hold onto past the end of each job.
   return new ContentVerifyJob(
       new ContentHashReader(extension_id, data->version, extension_root,
-                            normalized_path, delegate_->GetPublicKey()),
+                            normalized_unix_path, delegate_->GetPublicKey()),
       base::BindOnce(&ContentVerifier::VerifyFailed, this, extension_id));
 }
 
 void ContentVerifier::VerifyFailed(const std::string& extension_id,
                                    ContentVerifyJob::FailureReason reason) {
   if (!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) {
     content::BrowserThread::PostTask(
         content::BrowserThread::UI,
         FROM_HERE,
         base::Bind(&ContentVerifier::VerifyFailed, this, extension_id, reason));
@@ skipping 73 matching lines @@
     const std::string& extension_id,
     bool should_verify_any_paths_result) {
   if (should_verify_any_paths_result)
     delegate_->VerifyFailed(extension_id, ContentVerifyJob::MISSING_ALL_HASHES);
 }
 
 void ContentVerifier::OnFetchComplete(
     const std::string& extension_id,
     bool success,
     bool was_force_check,
-    const std::set<base::FilePath>& hash_mismatch_paths) {
+    const std::set<base::FilePath>& hash_mismatch_unix_paths) {
   if (g_test_observer)
     g_test_observer->OnFetchComplete(extension_id, success);
 
   if (shutdown_)
     return;
 
   VLOG(1) << "OnFetchComplete " << extension_id << " success:" << success;
 
   ExtensionRegistry* registry = ExtensionRegistry::Get(context_);
   const Extension* extension =
       registry->GetExtensionById(extension_id, ExtensionRegistry::EVERYTHING);
   if (!delegate_ || !extension)
     return;
 
   ContentVerifierDelegate::Mode mode = delegate_->ShouldBeVerified(*extension);
   if (was_force_check && !success &&
       mode == ContentVerifierDelegate::ENFORCE_STRICT) {
     // We weren't able to get verified_contents.json or weren't able to compute
     // hashes.
     delegate_->VerifyFailed(extension_id, ContentVerifyJob::MISSING_ALL_HASHES);
   } else {
     content::BrowserThread::PostTaskAndReplyWithResult(
-        content::BrowserThread::IO,
-        FROM_HERE,
-        base::Bind(&ContentVerifier::ShouldVerifyAnyPaths,
-                   this,
-                   extension_id,
-                   extension->path(),
-                   hash_mismatch_paths),
-        base::Bind(
-            &ContentVerifier::OnFetchCompleteHelper, this, extension_id));
+        content::BrowserThread::IO, FROM_HERE,
+        base::Bind(&ContentVerifier::ShouldVerifyAnyPaths, this, extension_id,
+                   extension->path(), hash_mismatch_unix_paths),
+        base::Bind(&ContentVerifier::OnFetchCompleteHelper, this,
+                   extension_id));
   }
 }
 
 bool ContentVerifier::ShouldVerifyAnyPaths(
     const std::string& extension_id,
     const base::FilePath& extension_root,
-    const std::set<base::FilePath>& relative_paths) {
+    const std::set<base::FilePath>& relative_unix_paths) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   const ContentVerifierIOData::ExtensionData* data =
       io_data_->GetData(extension_id);
   if (!data)
     return false;
 
   const std::set<base::FilePath>& browser_images = *(data->browser_image_paths);
 
   base::FilePath locales_dir = extension_root.Append(kLocaleFolder);
   std::unique_ptr<std::set<std::string>> all_locales;
 
-  for (std::set<base::FilePath>::const_iterator i = relative_paths.begin();
-       i != relative_paths.end();
-       ++i) {
-    const base::FilePath& relative_path = *i;
-
-    if (relative_path == base::FilePath(kManifestFilename))
+  for (const base::FilePath& relative_unix_path : relative_unix_paths) {
+    if (relative_unix_path == base::FilePath(kManifestFilename))

[Devlin, 2017/04/24 15:06:56]

drive-by: this reparses kManifestFilename as a filepath on each iteration; can
we instead do:
const base::FilePath manifest_file(kManifestFilename);
for (...) {
  if (relative_unix_path == manifest_file)
?

[lazyboy, 2017/04/24 18:29:59]

Thanks, Done.

       continue;
 
-    if (base::ContainsKey(browser_images, relative_path))
+    if (base::ContainsKey(browser_images, relative_unix_path))
       continue;
 
-    base::FilePath full_path = extension_root.Append(relative_path);
+    base::FilePath full_path =

[lazyboy, 2017/04/22 00:49:42]

This is the change from the original CL that makes sure we create the right path
in windows.

Should fix the test failure.

+        extension_root.Append(relative_unix_path.NormalizePathSeparators());
     if (locales_dir.IsParent(full_path)) {
       if (!all_locales) {
         // TODO(asargent) - see if we can cache this list longer to avoid
         // having to fetch it more than once for a given run of the
         // browser. Maybe it can never change at runtime? (Or if it can, maybe
         // there is an event we can listen for to know to drop our cache).
         all_locales.reset(new std::set<std::string>);
         extension_l10n_util::GetAllLocales(all_locales.get());
       }
 
       // Since message catalogs get transcoded during installation, we want
-      // to skip those paths.
-      if (full_path.DirName().DirName() == locales_dir &&
-          !extension_l10n_util::ShouldSkipValidation(
-              locales_dir, full_path.DirName(), *all_locales))
+      // to skip those paths. See if this path looks like
+      // _locales/<some locale>/messages.json - if so then skip it.
+      if (full_path.BaseName() == base::FilePath(kMessagesFilename) &&

[lazyboy, 2017/04/22 00:49:42]

Another change: compare the filename first with "messages.json" first, instead
of constructing and checking the entire path. Not necessary to fix the tests.

[Devlin, 2017/04/24 15:06:56]

ditto re the use of a const FilePath.

+          full_path.DirName().DirName() == locales_dir &&
+          base::ContainsKey(*all_locales,
+                            full_path.DirName().BaseName().MaybeAsASCII())) {
         continue;
+      }
     }
     return true;
   }
   return false;
 }
 
 }  // namespace extensions