Disable windows reserved characters and reserved names in manifest resources

From [0], file name on windows platform can't contain the reserved characters
(\ / : * ? " < > |) and can't use the reserved names(CON, PRN, AUX, NUL, COM1
COM2, COM3, COM4, COM5, COM6, COM7, COM8, COM9, LPT1, LPT2, LPT3, LPT4, LPT5
LPT6, LPT7, LPT8, LPT9)

On Linux/Mac, only chararacters / and NULL('\0') are disallowed.

So We disable the \ : * ? " < > | characters and windows reserved names in
extension manifest resources to make extensions work cross platform better.

[0]: http://msdn.microsoft.com/en-us/library/aa365247.aspx

BUG=274768
TEST = unit_tests --gtest_filter=ExtensionTest.GetResource
TEST = load/package unpacked extension which contains the disallow character
* ? | < >) and see results.

Review URL: https://chromiumcodereview.appspot.com/23079008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@222493 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 83 insertions, 0 deletions

diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index ad28efd..b5e91e5 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -35,6 +35,7 @@
 #include "extensions/common/url_pattern_set.h"
 #include "grit/chromium_strings.h"
 #include "grit/theme_resources.h"
+#include "net/base/net_util.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "url/url_util.h"
 
@@ -97,6 +98,17 @@ class ExtensionConfig {
   Extension::ScriptingWhitelist scripting_whitelist_;
 };
 
+bool ContainsReservedCharacters(const base::FilePath& path) {
+  // We should disallow backslash '\\' as file path separator even on Windows,
+  // because the backslash is not regarded as file path separator on Linux/Mac.
+  // Extensions are cross-platform.
+  // Since FilePath uses backslash '\\' as file path separator on Windows, so we
+  // need to check manually.
+  if (path.value().find('\\') != path.value().npos)
+    return true;
+  return !net::IsSafePortableRelativePath(path);
+}
+
 }  // namespace
 
 #if defined(OS_WIN)
@@ -222,6 +234,8 @@ ExtensionResource Extension::GetResource(
   if (!new_path.empty() && new_path.at(0) == '/')
     new_path.erase(0, 1);
   base::FilePath relative_file_path = base::FilePath::FromUTF8Unsafe(new_path);
+  if (ContainsReservedCharacters(relative_file_path))
+    return ExtensionResource();
   ExtensionResource r(id(), path(), relative_file_path);
   if ((creation_flags() & Extension::FOLLOW_SYMLINKS_ANYWHERE)) {
     r.set_follow_symlinks_anywhere();
@@ -231,6 +245,8 @@ ExtensionResource Extension::GetResource(
 
 ExtensionResource Extension::GetResource(
     const base::FilePath& relative_file_path) const {
+  if (ContainsReservedCharacters(relative_file_path))
+    return ExtensionResource();
   ExtensionResource r(id(), path(), relative_file_path);
   if ((creation_flags() & Extension::FOLLOW_SYMLINKS_ANYWHERE)) {
     r.set_follow_symlinks_anywhere();
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index 7f28990..c2231da 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -27,6 +27,7 @@
 
 using extension_test_util::LoadManifest;
 using extension_test_util::LoadManifestStrict;
+using base::FilePath;
 
 namespace extensions {
 
@@ -99,6 +100,72 @@ TEST(ExtensionTest, GetResourceURLAndPath) {
             extension->GetResourceURL("/test.html").spec());
 }
 
+TEST(ExtensionTest, GetResource) {
+  const FilePath valid_path_test_cases[] = {
+    FilePath(FILE_PATH_LITERAL("manifest.json")),
+    FilePath(FILE_PATH_LITERAL("a/b/c/manifest.json")),
+    FilePath(FILE_PATH_LITERAL("com/manifest.json")),
+    FilePath(FILE_PATH_LITERAL("lpt/manifest.json")),
+  };
+  const FilePath invalid_path_test_cases[] = {
+    // Directory name
+    FilePath(FILE_PATH_LITERAL("src/")),
+    // Contains a drive letter specification.
+    FilePath(FILE_PATH_LITERAL("C:\\manifest.json")),
+    // Use backslash '\\' as separator.
+    FilePath(FILE_PATH_LITERAL("a\\b\\c\\manifest.json")),
+    // Reserved Characters with extension
+    FilePath(FILE_PATH_LITERAL("mani>fest.json")),
+    FilePath(FILE_PATH_LITERAL("mani<fest.json")),
+    FilePath(FILE_PATH_LITERAL("mani*fest.json")),
+    FilePath(FILE_PATH_LITERAL("mani:fest.json")),
+    FilePath(FILE_PATH_LITERAL("mani?fest.json")),
+    FilePath(FILE_PATH_LITERAL("mani|fest.json")),
+    // Reserved Characters without extension
+    FilePath(FILE_PATH_LITERAL("mani>fest")),
+    FilePath(FILE_PATH_LITERAL("mani<fest")),
+    FilePath(FILE_PATH_LITERAL("mani*fest")),
+    FilePath(FILE_PATH_LITERAL("mani:fest")),
+    FilePath(FILE_PATH_LITERAL("mani?fest")),
+    FilePath(FILE_PATH_LITERAL("mani|fest")),
+    // Reserved Names with extension.
+    FilePath(FILE_PATH_LITERAL("com1.json")),
+    FilePath(FILE_PATH_LITERAL("com9.json")),
+    FilePath(FILE_PATH_LITERAL("LPT1.json")),
+    FilePath(FILE_PATH_LITERAL("LPT9.json")),
+    FilePath(FILE_PATH_LITERAL("CON.json")),
+    FilePath(FILE_PATH_LITERAL("PRN.json")),
+    FilePath(FILE_PATH_LITERAL("AUX.json")),
+    FilePath(FILE_PATH_LITERAL("NUL.json")),
+    // Reserved Names without extension.
+    FilePath(FILE_PATH_LITERAL("com1")),
+    FilePath(FILE_PATH_LITERAL("com9")),
+    FilePath(FILE_PATH_LITERAL("LPT1")),
+    FilePath(FILE_PATH_LITERAL("LPT9")),
+    FilePath(FILE_PATH_LITERAL("CON")),
+    FilePath(FILE_PATH_LITERAL("PRN")),
+    FilePath(FILE_PATH_LITERAL("AUX")),
+    FilePath(FILE_PATH_LITERAL("NUL")),
+    // Reserved Names as directory.
+    FilePath(FILE_PATH_LITERAL("com1/manifest.json")),
+    FilePath(FILE_PATH_LITERAL("com9/manifest.json")),
+    FilePath(FILE_PATH_LITERAL("LPT1/manifest.json")),
+    FilePath(FILE_PATH_LITERAL("LPT9/manifest.json")),
+    FilePath(FILE_PATH_LITERAL("CON/manifest.json")),
+    FilePath(FILE_PATH_LITERAL("PRN/manifest.json")),
+    FilePath(FILE_PATH_LITERAL("AUX/manifest.json")),
+    FilePath(FILE_PATH_LITERAL("NUL/manifest.json")),
+  };
+
+  scoped_refptr<Extension> extension = LoadManifestStrict("empty_manifest",
+      "empty.json");
+  EXPECT_TRUE(extension.get());
+  for (size_t i = 0; i < arraysize(valid_path_test_cases); ++i)
+    EXPECT_TRUE(!extension->GetResource(valid_path_test_cases[i]).empty());
+  for (size_t i = 0; i < arraysize(invalid_path_test_cases); ++i)
+    EXPECT_TRUE(extension->GetResource(invalid_path_test_cases[i]).empty());
+}
+
 TEST(ExtensionTest, GetAbsolutePathNoError) {
   scoped_refptr<Extension> extension = LoadManifestStrict("absolute_path",
       "absolute.json");