From 8f7d2587e27589c1d0bddeb3f6de9c50aad6829e Mon Sep 17 00:00:00 2001 From: mtomasz Date: Tue, 12 May 2015 01:21:25 -0700 Subject: [PATCH] Improve parsing manifest for FSP API. This CL makes the FSP section a prerequisite for the permission. It also adds a warning if a section is used without a permission. TEST=unit_tests: *FileSystemProviderHandlerTest*" BUG=474146 Review URL: https://codereview.chromium.org/1133013002 Cr-Commit-Position: refs/heads/master@{#329374} --- chrome/chrome_tests_unit.gypi | 1 + .../file_system_provider_handler_unittest.cc | 31 ++++++++++++++++++++++ .../file_system_provider_capabilities_handler.cc | 30 +++++++++++++++++++-- .../file_system_provider_capabilities_handler.h | 1 + .../file_system_provider/add_watcher/manifest.json | 5 ++++ .../file_system_provider/big_file/manifest.json | 5 ++++ .../file_system_provider/copy_entry/manifest.json | 5 ++++ .../create_directory/manifest.json | 5 ++++ .../file_system_provider/create_file/manifest.json | 5 ++++ .../delete_entry/manifest.json | 5 ++++ .../file_system_provider/evil/manifest.json | 5 ++++ .../file_system_provider/extension/manifest.json | 5 ++++ .../file_system_provider/get_all/manifest.json | 5 ++++ .../get_metadata/manifest.json | 5 ++++ .../file_system_provider/mime_type/manifest.json | 5 ++++ .../file_system_provider/move_entry/manifest.json | 5 ++++ .../file_system_provider/notify/manifest.json | 5 ++++ .../read_directory/manifest.json | 5 ++++ .../file_system_provider/read_file/manifest.json | 5 ++++ .../remove_watcher/manifest.json | 5 ++++ .../file_system_provider/thumbnail/manifest.json | 5 ++++ .../file_system_provider/truncate/manifest.json | 5 ++++ .../file_system_provider/unmount/manifest.json | 5 ++++ .../file_system_provider/write_file/manifest.json | 5 ++++ .../filesystemprovider_missing_capabilities.json | 7 +++++ .../filesystemprovider_missing_permission.json | 9 +++++++ .../manifest_tests/filesystemprovider_valid.json | 12 +++++++++ extensions/common/manifest_constants.cc | 9 ++++++- extensions/common/manifest_constants.h | 2 ++ 29 files changed, 199 insertions(+), 3 deletions(-) create mode 100644 chrome/common/extensions/api/file_system_provider/file_system_provider_handler_unittest.cc create mode 100644 chrome/test/data/extensions/manifest_tests/filesystemprovider_missing_capabilities.json create mode 100644 chrome/test/data/extensions/manifest_tests/filesystemprovider_missing_permission.json create mode 100644 chrome/test/data/extensions/manifest_tests/filesystemprovider_valid.json diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 058844eed155..5e35b8a77f44 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -1326,6 +1326,7 @@ 'browser/ui/webui/chromeos/login/signin_userlist_unittest.cc', 'browser/ui/webui/options/chromeos/cros_language_options_handler_unittest.cc', 'common/extensions/api/file_browser_handlers/file_browser_handler_manifest_unittest.cc', + 'common/extensions/api/file_system_provider/file_system_provider_handler_unittest.cc', ], 'chrome_unit_tests_desktop_linux_sources': [ 'browser/password_manager/native_backend_kwallet_x_unittest.cc', diff --git a/chrome/common/extensions/api/file_system_provider/file_system_provider_handler_unittest.cc b/chrome/common/extensions/api/file_system_provider/file_system_provider_handler_unittest.cc new file mode 100644 index 000000000000..677f261f8219 --- /dev/null +++ b/chrome/common/extensions/api/file_system_provider/file_system_provider_handler_unittest.cc @@ -0,0 +1,31 @@ +// Copyright 2015 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 "chrome/common/extensions/manifest_tests/chrome_manifest_test.h" +#include "extensions/common/manifest_constants.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace extensions { + +class FileSystemProviderHandlerTest : public ChromeManifestTest {}; + +TEST_F(FileSystemProviderHandlerTest, Valid) { + RunTestcase(Testcase("filesystemprovider_valid.json"), EXPECT_TYPE_SUCCESS); +} + +TEST_F(FileSystemProviderHandlerTest, Invalid_MissingCapabilities) { + RunTestcase( + Testcase("filesystemprovider_missing_capabilities.json", + manifest_errors::kInvalidFileSystemProviderMissingCapabilities), + EXPECT_TYPE_ERROR); +} + +TEST_F(FileSystemProviderHandlerTest, Invalid_MissingPermission) { + RunTestcase( + Testcase("filesystemprovider_missing_permission.json", + manifest_errors::kInvalidFileSystemProviderMissingPermission), + EXPECT_TYPE_WARNING); +} + +} // namespace extensions diff --git a/chrome/common/extensions/api/file_system_provider_capabilities/file_system_provider_capabilities_handler.cc b/chrome/common/extensions/api/file_system_provider_capabilities/file_system_provider_capabilities_handler.cc index e716df08babc..c3dc906cea8e 100644 --- a/chrome/common/extensions/api/file_system_provider_capabilities/file_system_provider_capabilities_handler.cc +++ b/chrome/common/extensions/api/file_system_provider_capabilities/file_system_provider_capabilities_handler.cc @@ -10,6 +10,7 @@ #include "chrome/common/extensions/api/manifest_types.h" #include "extensions/common/error_utils.h" #include "extensions/common/manifest_constants.h" +#include "extensions/common/manifest_handlers/permissions_parser.h" #include "url/gurl.h" namespace extensions { @@ -47,10 +48,30 @@ const FileSystemProviderCapabilities* FileSystemProviderCapabilities::Get( bool FileSystemProviderCapabilitiesHandler::Parse(Extension* extension, base::string16* error) { - const base::DictionaryValue* section = NULL; + const bool has_permission = extensions::PermissionsParser::HasAPIPermission( + extension, extensions::APIPermission::ID::kFileSystemProvider); + const base::DictionaryValue* section = nullptr; extension->manifest()->GetDictionary( manifest_keys::kFileSystemProviderCapabilities, §ion); - DCHECK(section); + + if (has_permission && !section) { + *error = base::ASCIIToUTF16( + manifest_errors::kInvalidFileSystemProviderMissingCapabilities); + return false; + } + + if (!has_permission && section) { + // If the permission is not specified, then the section has simply no + // effect, hence just a warning. + extension->AddInstallWarning(extensions::InstallWarning( + manifest_errors::kInvalidFileSystemProviderMissingPermission)); + return true; + } + + if (!has_permission && !section) { + // No permission and no capabilities, so ignore. + return true; + } api::manifest_types::FileSystemProviderCapabilities idl_capabilities; if (!api::manifest_types::FileSystemProviderCapabilities::Populate( @@ -88,4 +109,9 @@ const std::vector FileSystemProviderCapabilitiesHandler::Keys() return SingleKey(manifest_keys::kFileSystemProviderCapabilities); } +bool FileSystemProviderCapabilitiesHandler::AlwaysParseForType( + Manifest::Type type) const { + return true; +} + } // namespace extensions diff --git a/chrome/common/extensions/api/file_system_provider_capabilities/file_system_provider_capabilities_handler.h b/chrome/common/extensions/api/file_system_provider_capabilities/file_system_provider_capabilities_handler.h index 7e2000f33ac7..dcee62809cf0 100644 --- a/chrome/common/extensions/api/file_system_provider_capabilities/file_system_provider_capabilities_handler.h +++ b/chrome/common/extensions/api/file_system_provider_capabilities/file_system_provider_capabilities_handler.h @@ -48,6 +48,7 @@ class FileSystemProviderCapabilitiesHandler : public ManifestHandler { // ManifestHandler overrides. bool Parse(Extension* extension, base::string16* error) override; + bool AlwaysParseForType(Manifest::Type type) const override; private: const std::vector Keys() const override; diff --git a/chrome/test/data/extensions/api_test/file_system_provider/add_watcher/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/add_watcher/manifest.json index 2ebef7a3f42d..76c063835dc8 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/add_watcher/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/add_watcher/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/big_file/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/big_file/manifest.json index 0368eb6be5d0..08ef632a1263 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/big_file/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/big_file/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/copy_entry/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/copy_entry/manifest.json index 3c5ce0656ec2..4cfeeef5cb9d 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/copy_entry/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/copy_entry/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/create_directory/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/create_directory/manifest.json index ea177718bc9c..ffaa9158ea07 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/create_directory/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/create_directory/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/create_file/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/create_file/manifest.json index 4fefa4acd71b..25c779c8159a 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/create_file/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/create_file/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/delete_entry/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/delete_entry/manifest.json index c8435b23c7dd..311f42c87661 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/delete_entry/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/delete_entry/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/evil/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/evil/manifest.json index 083ea7abf7c4..7af20757aa18 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/evil/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/evil/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/extension/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/extension/manifest.json index 3443af6c1027..380a7a19ec06 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/extension/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/extension/manifest.json @@ -6,6 +6,11 @@ "manifest_version": 2, "description": "Test for chrome.fileSystemProvider.mount() in an extension.", "permissions": ["fileSystemProvider", "fileManagerPrivate"], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "background": { "persistent": false, "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/get_all/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/get_all/manifest.json index 1f3e11390633..f8b2c7929dcc 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/get_all/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/get_all/manifest.json @@ -12,6 +12,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/get_metadata/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/get_metadata/manifest.json index 85548303bbec..7f4f723191ef 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/get_metadata/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/get_metadata/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/mime_type/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/mime_type/manifest.json index 6529f3b2390c..cc3e78fe0a95 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/mime_type/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/mime_type/manifest.json @@ -18,6 +18,11 @@ "types": ["text/secret-testing-mime-type"] } }, + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/move_entry/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/move_entry/manifest.json index 3c5ce0656ec2..4cfeeef5cb9d 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/move_entry/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/move_entry/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/notify/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/notify/manifest.json index de6d85b5bb12..b8d17a7f668c 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/notify/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/notify/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/read_directory/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/read_directory/manifest.json index 423816b79804..c779c1cba1ed 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/read_directory/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/read_directory/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/read_file/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/read_file/manifest.json index b6647541f83d..8931bf21ebff 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/read_file/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/read_file/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/remove_watcher/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/remove_watcher/manifest.json index 4bf2a80602dd..66d7cd758f9b 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/remove_watcher/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/remove_watcher/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/thumbnail/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/thumbnail/manifest.json index d90308d03e58..7dd236aced1d 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/thumbnail/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/thumbnail/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/truncate/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/truncate/manifest.json index 8f3ce4c28725..7fa995c6cef0 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/truncate/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/truncate/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/unmount/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/unmount/manifest.json index 7d4f3c4882c1..8d17487522d6 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/unmount/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/unmount/manifest.json @@ -5,6 +5,11 @@ "manifest_version": 2, "description": "Test for chrome.fileSystemProvider.unmount().", "permissions": ["fileSystemProvider", "fileManagerPrivate"], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/api_test/file_system_provider/write_file/manifest.json b/chrome/test/data/extensions/api_test/file_system_provider/write_file/manifest.json index 397e2dd7329a..bc1615579f80 100644 --- a/chrome/test/data/extensions/api_test/file_system_provider/write_file/manifest.json +++ b/chrome/test/data/extensions/api_test/file_system_provider/write_file/manifest.json @@ -13,6 +13,11 @@ }, "fileManagerPrivate" ], + "file_system_provider_capabilities": { + "configurable": true, + "multiple_mounts": true, + "source": "device" + }, "app": { "background": { "scripts": [ diff --git a/chrome/test/data/extensions/manifest_tests/filesystemprovider_missing_capabilities.json b/chrome/test/data/extensions/manifest_tests/filesystemprovider_missing_capabilities.json new file mode 100644 index 000000000000..d3bb33c8c9a9 --- /dev/null +++ b/chrome/test/data/extensions/manifest_tests/filesystemprovider_missing_capabilities.json @@ -0,0 +1,7 @@ +{ + "name": "test", + "version": "1", + "permissions": [ + "fileSystemProvider" + ] +} diff --git a/chrome/test/data/extensions/manifest_tests/filesystemprovider_missing_permission.json b/chrome/test/data/extensions/manifest_tests/filesystemprovider_missing_permission.json new file mode 100644 index 000000000000..8003fbcc2b90 --- /dev/null +++ b/chrome/test/data/extensions/manifest_tests/filesystemprovider_missing_permission.json @@ -0,0 +1,9 @@ +{ + "name": "test", + "version": "1", + "file_system_provider_capabilities": { + "configurable": false, + "multiple_mounts": false, + "source": "device" + } +} diff --git a/chrome/test/data/extensions/manifest_tests/filesystemprovider_valid.json b/chrome/test/data/extensions/manifest_tests/filesystemprovider_valid.json new file mode 100644 index 000000000000..6c04c69b6531 --- /dev/null +++ b/chrome/test/data/extensions/manifest_tests/filesystemprovider_valid.json @@ -0,0 +1,12 @@ +{ + "name": "test", + "version": "1", + "permissions": [ + "fileSystemProvider" + ], + "file_system_provider_capabilities": { + "configurable": false, + "multiple_mounts": false, + "source": "device" + } +} diff --git a/extensions/common/manifest_constants.cc b/extensions/common/manifest_constants.cc index 01bcbfb2227a..7b2ebbdf77c6 100644 --- a/extensions/common/manifest_constants.cc +++ b/extensions/common/manifest_constants.cc @@ -713,7 +713,14 @@ const char kWebRequestConflictsWithLazyBackground[] = "The 'webRequest' API cannot be used with event pages."; #if defined(OS_CHROMEOS) const char kIllegalPlugins[] = - "Extensions cannot install plugins on Chrome OS"; + "Extensions cannot install plugins on Chrome OS."; +const char kInvalidFileSystemProviderMissingCapabilities[] = + "The 'fileSystemProvider' permission requires the " + "'file_system_provider_capabilities' section to be specified in the " + "manifest."; +const char kInvalidFileSystemProviderMissingPermission[] = + "The 'file_system_provider_capabilities' section requires the " + "'fileSystemProvider' permission to be specified in the manifest."; #endif } // namespace manifest_errors diff --git a/extensions/common/manifest_constants.h b/extensions/common/manifest_constants.h index a5dc92accefd..d5ffa63f30c4 100644 --- a/extensions/common/manifest_constants.h +++ b/extensions/common/manifest_constants.h @@ -479,6 +479,8 @@ extern const char kUnrecognizedManifestProperty[]; extern const char kWebRequestConflictsWithLazyBackground[]; #if defined(OS_CHROMEOS) extern const char kIllegalPlugins[]; +extern const char kInvalidFileSystemProviderMissingCapabilities[]; +extern const char kInvalidFileSystemProviderMissingPermission[]; #endif } // namespace manifest_errors -- 2.11.4.GIT