Skip to content

Commit

Permalink
fix unpack the same icon twice
Browse files Browse the repository at this point in the history
Extension unpacker may process the icon twice when icon path presents in
an extension manifest twice as different string values, for example:
./icons/myicon.jpg and icons/myicon.jpg.

Bug: 
Change-Id: Ie6d334510b94ee78217aade6906a7c5139b6f204
Reviewed-on: https://chromium-review.googlesource.com/836047
Commit-Queue: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526176}
  • Loading branch information
Ivan Afanasyev authored and Commit Bot committed Dec 25, 2017
1 parent ade0637 commit 21c6dc8
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 3 deletions.
40 changes: 38 additions & 2 deletions extensions/common/extension_icon_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,41 @@

#include "extensions/common/extension_icon_set.h"

#include <vector>

#include "base/files/file_path.h"
#include "base/logging.h"
#include "base/strings/string_util.h"

namespace {

// Normalize relative path that does not reference to parent directory. Removes
// ".". Returns false if path can not be normalized, i.e. it references parent
// or empty after normalization.
bool NormalizeRelativePath(const base::FilePath& path, base::FilePath* result) {
DCHECK(result);

if (path.ReferencesParent())
return false;

std::vector<base::FilePath::StringType> components;
path.GetComponents(&components);

base::FilePath rv;
for (const auto& path_component : components) {
if (path_component != base::FilePath::kCurrentDirectory)
rv = rv.Append(path_component);
}

if (rv.empty())
return false;

*result = std::move(rv);
return true;
}

} // namespace

ExtensionIconSet::ExtensionIconSet() {}

ExtensionIconSet::ExtensionIconSet(const ExtensionIconSet& other) = default;
Expand Down Expand Up @@ -75,6 +106,11 @@ int ExtensionIconSet::GetIconSizeFromPath(base::StringPiece path) const {

void ExtensionIconSet::GetPaths(std::set<base::FilePath>* paths) const {
CHECK(paths);
for (auto iter : map())
paths->insert(base::FilePath::FromUTF8Unsafe(iter.second));
for (const auto& iter : map()) {
base::FilePath normalized_path;
if (NormalizeRelativePath(base::FilePath::FromUTF8Unsafe(iter.second),
&normalized_path)) {
paths->emplace(std::move(normalized_path));
}
}
}
2 changes: 1 addition & 1 deletion extensions/common/extension_icon_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class ExtensionIconSet {
void Add(int size, const std::string& path);

// Gets path value of the icon found when searching for |size| using
// |mathc_type|.
// |match_type|.
const std::string& Get(int size, MatchType match_type) const;

// Returns true iff the set contains the specified path.
Expand Down
27 changes: 27 additions & 0 deletions extensions/common/extension_icon_set_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,31 @@ TEST(ExtensionIconSetTest, FindSize) {
icons.GetIconSizeFromPath("foo"));
}

TEST(ExtensionIconSetTest, CollapsesSameRelativePaths) {
ExtensionIconSet icons;
icons.Add(extension_misc::EXTENSION_ICON_BITTY, "foo/icon.jpg");
icons.Add(extension_misc::EXTENSION_ICON_SMALL, "./foo/icon.jpg");
icons.Add(extension_misc::EXTENSION_ICON_LARGE, "./bar/../foo/icon.jpg");

std::set<base::FilePath> paths;
icons.GetPaths(&paths);

const std::set<base::FilePath> expected_paths = {
base::FilePath::FromUTF8Unsafe("foo").AppendASCII("icon.jpg")};
EXPECT_EQ(expected_paths, paths);
}

TEST(ExtensionIconSetTest, IgnoresIncorrectRelativePaths) {
ExtensionIconSet icons;
icons.Add(extension_misc::EXTENSION_ICON_BITTY, "foo/../../icon.jpg");
icons.Add(extension_misc::EXTENSION_ICON_SMALL, "../icon.jpg");
icons.Add(extension_misc::EXTENSION_ICON_LARGE, "./foo/icons/../..");
icons.Add(extension_misc::EXTENSION_ICON_GIGANTOR, "./foo/../..");

std::set<base::FilePath> paths;
icons.GetPaths(&paths);

EXPECT_TRUE(paths.empty());
}

} // namespace

0 comments on commit 21c6dc8

Please sign in to comment.