Fix content verification failures on unusual content script paths

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 "base/files/file_path.h"
 #include "base/stl_util.h"
@@ skipping 44 matching lines @@
 void ContentVerifier::Shutdown() {
   shutdown_ = true;
   content::BrowserThread::PostTask(
       content::BrowserThread::IO,
       FROM_HERE,
       base::Bind(&ContentVerifierIOData::Clear, io_data_));
   observer_.RemoveAll();
   fetcher_.reset();
 }
 
+static base::FilePath MakePathRelative(const base::FilePath& path) {

[Ken Rockot(use gerrit already), 2015/09/09 16:56:10]

nit since you're moving this, might as well nix the "static" and move it to an
anonymous namespace instead

[asargent_no_longer_on_chrome, 2015/09/09 17:42:39]

Done. Renamed to "NormalizeRelativePath" also. 

+  if (path.ReferencesParent())
+    return base::FilePath();
+
+  std::vector<base::FilePath::StringType> parts;
+  path.GetComponents(&parts);
+  if (parts.empty())
+    return base::FilePath();
+
+  // Remove the first component if it is '.' or '/' or '//'.
+  const base::FilePath::StringType separators(
+      base::FilePath::kSeparators, base::FilePath::kSeparatorsLength);
+  if (!parts[0].empty() &&
+      (parts[0] == base::FilePath::kCurrentDirectory ||
+       parts[0].find_first_not_of(separators) == std::string::npos))
+    parts.erase(parts.begin());
+
+  // Note that elsewhere we always normalize path separators to '/' so this
+  // should work for all platforms.
+  return base::FilePath(base::JoinString(parts, "/"));
+}
+
 ContentVerifyJob* ContentVerifier::CreateJobFor(
     const std::string& extension_id,
     const base::FilePath& extension_root,
     const base::FilePath& relative_path) {

[Ken Rockot(use gerrit already), 2015/09/09 16:56:10]

nit: Maybe instead of calling this |relative_path| and the real one
|actual_relative_path|, the real one could be changed to |relative_path| and
this could be something like |path_spec|? Since it's not really the relative
path, right?

[asargent_no_longer_on_chrome, 2015/09/09 17:42:39]

I sort of like calling the argument relative_path since that provides a clue
that the data supplied will be interpreted as a relative path, and "path_spec"
makes me think of absolute paths since GURL::spec() gives you the text of the
full URL. 

I renamed "actual_relative_path" to "normalized_path" instead. 

   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
   const ContentVerifierIOData::ExtensionData* data =
       io_data_->GetData(extension_id);
   if (!data)
     return NULL;
 
+  base::FilePath actual_relative_path = MakePathRelative(relative_path);
+
   std::set<base::FilePath> paths;
-  paths.insert(relative_path);
+  paths.insert(actual_relative_path);
   if (!ShouldVerifyAnyPaths(extension_id, extension_root, 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,
-                            relative_path, delegate_->GetPublicKey()),
+                            actual_relative_path, delegate_->GetPublicKey()),
       base::Bind(&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 13 matching lines @@
 
   if (reason == ContentVerifyJob::MISSING_ALL_HASHES) {
     // If we failed because there were no hashes yet for this extension, just
     // request some.
     fetcher_->DoFetch(extension, true /* force */);
   } else {
     delegate_->VerifyFailed(extension_id, reason);
   }
 }
 
-static base::FilePath MakeImagePathRelative(const base::FilePath& path) {
-  if (path.ReferencesParent())
-    return base::FilePath();
-
-  std::vector<base::FilePath::StringType> parts;
-  path.GetComponents(&parts);
-  if (parts.empty())
-    return base::FilePath();
-
-  // Remove the first component if it is '.' or '/' or '//'.
-  const base::FilePath::StringType separators(
-      base::FilePath::kSeparators, base::FilePath::kSeparatorsLength);
-  if (!parts[0].empty() &&
-      (parts[0] == base::FilePath::kCurrentDirectory ||
-       parts[0].find_first_not_of(separators) == std::string::npos))
-    parts.erase(parts.begin());
-
-  // Note that elsewhere we always normalize path separators to '/' so this
-  // should work for all platforms.
-  return base::FilePath(
-      base::JoinString(parts, base::FilePath::StringType(1, '/')));
-}
-
 void ContentVerifier::OnExtensionLoaded(
     content::BrowserContext* browser_context,
     const Extension* extension) {
   if (shutdown_)
     return;
 
   ContentVerifierDelegate::Mode mode = delegate_->ShouldBeVerified(*extension);
   if (mode != ContentVerifierDelegate::NONE) {
     // The browser image paths from the extension may not be relative (eg
     // they might have leading '/' or './'), so we strip those to make
     // comparing to actual relative paths work later on.
     std::set<base::FilePath> original_image_paths =
         delegate_->GetBrowserImagePaths(extension);
 
     scoped_ptr<std::set<base::FilePath>> image_paths(
         new std::set<base::FilePath>);
     for (const auto& path : original_image_paths) {
-      image_paths->insert(MakeImagePathRelative(path));
+      image_paths->insert(MakePathRelative(path));
     }
 
     scoped_ptr<ContentVerifierIOData::ExtensionData> data(
         new ContentVerifierIOData::ExtensionData(
             image_paths.Pass(),
             extension->version() ? *extension->version() : base::Version()));
     content::BrowserThread::PostTask(content::BrowserThread::IO,
                                      FROM_HERE,
                                      base::Bind(&ContentVerifierIOData::AddData,
                                                 io_data_,
@@ skipping 106 matching lines @@
           !extension_l10n_util::ShouldSkipValidation(
               locales_dir, full_path.DirName(), *all_locales))
         continue;
     }
     return true;
   }
   return false;
 }
 
 }  // namespace extensions