diff options
author | markusheintz@chromium.org <markusheintz@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-01-23 17:52:08 +0000 |
---|---|---|
committer | markusheintz@chromium.org <markusheintz@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-01-23 17:52:08 +0000 |
commit | ae689aab246d7763cb04ecba1bd382749c599ee7 (patch) | |
tree | 28c10be46648b3327e04b9a3ef78d312851013cd | |
parent | aff8b47835f1f9faa471fd23b8db782cdb492ddd (diff) | |
download | chromium_src-ae689aab246d7763cb04ecba1bd382749c599ee7.zip chromium_src-ae689aab246d7763cb04ecba1bd382749c599ee7.tar.gz chromium_src-ae689aab246d7763cb04ecba1bd382749c599ee7.tar.bz2 |
Added support for file URI path wildcards in content settings
patterns. I.e. "file:///*" matches all file URIs.
Full/explicit/absolute paths (e.g. "file:///foo/bar.html") are still supported.
contributed by Francois Kritzinger (francoisk777@gmail.com)
BUG=77149
TEST=ContentSettingsPattern*
Review URL: http://codereview.chromium.org/9254028
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@118696 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | AUTHORS | 1 | ||||
-rw-r--r-- | chrome/common/content_settings_pattern.cc | 53 | ||||
-rw-r--r-- | chrome/common/content_settings_pattern.h | 10 | ||||
-rw-r--r-- | chrome/common/content_settings_pattern_parser.cc | 23 | ||||
-rw-r--r-- | chrome/common/content_settings_pattern_parser.h | 4 | ||||
-rw-r--r-- | chrome/common/content_settings_pattern_parser_unittest.cc | 93 | ||||
-rw-r--r-- | chrome/common/content_settings_pattern_unittest.cc | 37 | ||||
-rw-r--r-- | chrome/common/render_messages.h | 1 |
8 files changed, 170 insertions, 52 deletions
@@ -158,3 +158,4 @@ Simon Hong <simon.hong81@gmail.com> Lu Guanqun <guanqun.lu@gmail.com> François Beaufort <beaufort.francois@gmail.com> Eriq Augustine <eriq.augustine@gmail.com> +Francois Kritzinger <francoisk777@gmail.com> diff --git a/chrome/common/content_settings_pattern.cc b/chrome/common/content_settings_pattern.cc index 16ec56f..bf00802 100644 --- a/chrome/common/content_settings_pattern.cc +++ b/chrome/common/content_settings_pattern.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -130,6 +130,13 @@ BuilderInterface* ContentSettingsPattern::Builder::WithSchemeWildcard() { BuilderInterface* ContentSettingsPattern::Builder::WithPath( const std::string& path) { parts_.path = path; + parts_.is_path_wildcard = false; + return this; +} + +BuilderInterface* ContentSettingsPattern::Builder::WithPathWildcard() { + parts_.path = ""; + parts_.is_path_wildcard = true; return this; } @@ -157,10 +164,11 @@ bool ContentSettingsPattern::Builder::Canonicalize(PatternParts* parts) { const std::string scheme(StringToLowerASCII(parts->scheme)); parts->scheme = scheme; - if (parts->scheme == std::string(chrome::kFileScheme)) { - GURL url(std::string(chrome::kFileScheme) + - std::string(chrome::kStandardSchemeSeparator) + parts->path); - parts->path = url.path(); + if (parts->scheme == std::string(chrome::kFileScheme) && + !parts->is_path_wildcard) { + GURL url(std::string(chrome::kFileScheme) + + std::string(chrome::kStandardSchemeSeparator) + parts->path); + parts->path = url.path(); } // Canonicalize the host part. @@ -190,12 +198,15 @@ bool ContentSettingsPattern::Builder::Validate(const PatternParts& parts) { } // file:// URL patterns have an empty host and port. - if (parts.scheme == std::string(chrome::kFileScheme)) - return parts.host.empty() && - parts.port.empty() && - !parts.path.empty() && - parts.path != std::string("/") && - parts.path.find("*") == std::string::npos; + if (parts.scheme == std::string(chrome::kFileScheme)) { + if (parts.has_domain_wildcard || !parts.host.empty() || !parts.port.empty()) + return false; + if (parts.is_path_wildcard) + return parts.path.empty(); + return (!parts.path.empty() && + parts.path != "/" && + parts.path.find("*") == std::string::npos); + } // If the pattern is for an extension URL test if it is valid. if (parts.scheme == std::string(chrome::kExtensionScheme) && @@ -267,7 +278,8 @@ bool ContentSettingsPattern::Builder::LegacyValidate( ContentSettingsPattern::PatternParts::PatternParts() : is_scheme_wildcard(false), has_domain_wildcard(false), - is_port_wildcard(false) {} + is_port_wildcard(false), + is_path_wildcard(false) {} ContentSettingsPattern::PatternParts::~PatternParts() {} @@ -371,7 +383,8 @@ ContentSettingsPattern ContentSettingsPattern::LegacyFromString( ContentSettingsPattern ContentSettingsPattern::Wildcard() { scoped_ptr<ContentSettingsPattern::BuilderInterface> builder( ContentSettingsPattern::CreateBuilder(true)); - builder->WithSchemeWildcard()->WithDomainWildcard()->WithPortWildcard(); + builder->WithSchemeWildcard()->WithDomainWildcard()->WithPortWildcard()-> + WithPathWildcard(); return builder->Build(); } @@ -410,16 +423,10 @@ bool ContentSettingsPattern::Matches( return false; } - // File URLs have no host. For file URLs check if the url path matches the - // path in the pattern. - // TODO(markusheintz): This should change in the future. There should be only - // one setting for all file URLs. So the path should be ignored. - if (!parts_.is_scheme_wildcard && - scheme == std::string(chrome::kFileScheme)) { - if (parts_.path == std::string(url.path())) - return true; - return false; - } + // File URLs have no host. Matches if the pattern has the path wildcard set, + // or if the path in the URL is identical to the one in the pattern. + if (!parts_.is_scheme_wildcard && scheme == chrome::kFileScheme) + return parts_.is_path_wildcard || parts_.path == std::string(url.path()); // Match the host part. const std::string host(net::TrimEndingDot(url.host())); diff --git a/chrome/common/content_settings_pattern.h b/chrome/common/content_settings_pattern.h index 3664851..d670b78 100644 --- a/chrome/common/content_settings_pattern.h +++ b/chrome/common/content_settings_pattern.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -93,6 +93,9 @@ class ContentSettingsPattern { // specification. Only used for content settings pattern with a "file" // scheme part. std::string path; + + // True if the path wildcard is set. + bool is_path_wildcard; }; class BuilderInterface { @@ -113,6 +116,8 @@ class ContentSettingsPattern { virtual BuilderInterface* WithPath(const std::string& path) = 0; + virtual BuilderInterface* WithPathWildcard() = 0; + virtual BuilderInterface* Invalid() = 0; // Returns a content settings pattern according to the current configuration @@ -214,9 +219,12 @@ class ContentSettingsPattern { virtual BuilderInterface* WithPath(const std::string& path) OVERRIDE; + virtual BuilderInterface* WithPathWildcard() OVERRIDE; + virtual BuilderInterface* Invalid() OVERRIDE; virtual ContentSettingsPattern Build() OVERRIDE; + private: // Canonicalizes the pattern parts so that they are ASCII only, either // in original (if it was already ASCII) or punycode form. Returns true if diff --git a/chrome/common/content_settings_pattern_parser.cc b/chrome/common/content_settings_pattern_parser.cc index e54834a..e98b4d5 100644 --- a/chrome/common/content_settings_pattern_parser.cc +++ b/chrome/common/content_settings_pattern_parser.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -42,6 +42,8 @@ const char* PatternParser::kHostWildcard = "*"; const char* PatternParser::kPortWildcard = "*"; +const char* PatternParser::kPathWildcard = "*"; + // static void PatternParser::Parse(const std::string& pattern_spec, ContentSettingsPattern::BuilderInterface* builder) { @@ -169,13 +171,18 @@ void PatternParser::Parse(const std::string& pattern_spec, builder->WithPort(port); } } else { - if (scheme != std::string(chrome::kExtensionScheme)) + if (scheme != std::string(chrome::kExtensionScheme) && + scheme != std::string(chrome::kFileScheme)) builder->WithPortWildcard(); } if (path_component.IsNonEmpty()) { - builder->WithPath(pattern_spec.substr(path_component.start, - path_component.len)); + const std::string path = pattern_spec.substr(path_component.start, + path_component.len); + if (path.substr(1) == kPathWildcard) + builder->WithPathWildcard(); + else + builder->WithPath(path); } } @@ -194,8 +201,12 @@ std::string PatternParser::ToString( if (!parts.is_scheme_wildcard) str += parts.scheme + chrome::kStandardSchemeSeparator; - if (parts.scheme == std::string(chrome::kFileScheme)) - return str + parts.path; + if (parts.scheme == chrome::kFileScheme) { + if (parts.is_path_wildcard) + return str + kUrlPathSeparator + kPathWildcard; + else + return str + parts.path; + } if (parts.has_domain_wildcard) { if (parts.host.empty()) diff --git a/chrome/common/content_settings_pattern_parser.h b/chrome/common/content_settings_pattern_parser.h index 3638c5d..19f8d27 100644 --- a/chrome/common/content_settings_pattern_parser.h +++ b/chrome/common/content_settings_pattern_parser.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -33,6 +33,8 @@ class PatternParser { static const char* kPortWildcard; + static const char* kPathWildcard; + DISALLOW_COPY_AND_ASSIGN(PatternParser); }; diff --git a/chrome/common/content_settings_pattern_parser_unittest.cc b/chrome/common/content_settings_pattern_parser_unittest.cc index ed32f59..b6d5f34 100644 --- a/chrome/common/content_settings_pattern_parser_unittest.cc +++ b/chrome/common/content_settings_pattern_parser_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -21,13 +21,25 @@ class MockBuilder : public ContentSettingsPattern::BuilderInterface { MOCK_METHOD1(WithHost, BuilderInterface*(const std::string& host)); MOCK_METHOD1(WithPort, BuilderInterface*(const std::string& port)); MOCK_METHOD1(WithPath, BuilderInterface*(const std::string& path)); + MOCK_METHOD0(WithPathWildcard, BuilderInterface*()); MOCK_METHOD0(Invalid, BuilderInterface*()); MOCK_METHOD0(Build, ContentSettingsPattern()); }; TEST(ContentSettingsPatternParserTest, ParsePatterns) { // Test valid patterns - MockBuilder builder; + ::testing::StrictMock<MockBuilder> builder; + + // WithPathWildcard() is not called for "*". (Need a strict Mock for this + // case.) + EXPECT_CALL(builder, WithSchemeWildcard()).Times(1).WillOnce( + ::testing::Return(&builder)); + EXPECT_CALL(builder, WithDomainWildcard()).Times(1).WillOnce( + ::testing::Return(&builder)); + EXPECT_CALL(builder, WithPortWildcard()).Times(1).WillOnce( + ::testing::Return(&builder)); + content_settings::PatternParser::Parse("*", &builder); + ::testing::Mock::VerifyAndClear(&builder); EXPECT_CALL(builder, WithScheme("http")).Times(1).WillOnce( ::testing::Return(&builder)); @@ -86,14 +98,6 @@ TEST(ContentSettingsPatternParserTest, ParsePatterns) { content_settings::PatternParser::Parse("http://127.0.0.1:8080", &builder); ::testing::Mock::VerifyAndClear(&builder); - EXPECT_CALL(builder, WithScheme("file")).Times(1).WillOnce( - ::testing::Return(&builder)); - EXPECT_CALL(builder, WithPath("/foo/bar/test.html")).Times(1).WillOnce( - ::testing::Return(&builder)); - content_settings::PatternParser::Parse( - "file:///foo/bar/test.html", &builder); - ::testing::Mock::VerifyAndClear(&builder); - // Test valid pattern short forms EXPECT_CALL(builder, WithSchemeWildcard()).Times(1).WillOnce( ::testing::Return(&builder)); @@ -125,22 +129,85 @@ TEST(ContentSettingsPatternParserTest, ParsePatterns) { ::testing::Mock::VerifyAndClear(&builder); // Test invalid patterns + EXPECT_CALL(builder, WithSchemeWildcard()).Times(1).WillOnce( + ::testing::Return(&builder)); EXPECT_CALL(builder, Invalid()).Times(1).WillOnce( ::testing::Return(&builder)); content_settings::PatternParser::Parse("*youtube.com", &builder); ::testing::Mock::VerifyAndClear(&builder); + EXPECT_CALL(builder, WithSchemeWildcard()).Times(1).WillOnce( + ::testing::Return(&builder)); EXPECT_CALL(builder, Invalid()).Times(1).WillOnce( ::testing::Return(&builder)); content_settings::PatternParser::Parse("*.youtube.com", &builder); ::testing::Mock::VerifyAndClear(&builder); + EXPECT_CALL(builder, WithSchemeWildcard()).Times(1).WillOnce( + ::testing::Return(&builder)); EXPECT_CALL(builder, Invalid()).Times(1).WillOnce( ::testing::Return(&builder)); content_settings::PatternParser::Parse("www.youtube.com*", &builder); ::testing::Mock::VerifyAndClear(&builder); } +TEST(ContentSettingsPatternParserTest, ParseFilePatterns) { + ::testing::StrictMock<MockBuilder> builder; + + EXPECT_CALL(builder, WithScheme("file")).Times(1).WillOnce( + ::testing::Return(&builder)); + EXPECT_CALL(builder, WithPath("/foo/bar/test.html")).Times(1).WillOnce( + ::testing::Return(&builder)); + content_settings::PatternParser::Parse( + "file:///foo/bar/test.html", &builder); + ::testing::Mock::VerifyAndClear(&builder); + + EXPECT_CALL(builder, WithScheme("file")).Times(1).WillOnce( + ::testing::Return(&builder)); + EXPECT_CALL(builder, WithDomainWildcard()).Times(1).WillOnce( + ::testing::Return(&builder)); + content_settings::PatternParser::Parse( + "file://*", &builder); + ::testing::Mock::VerifyAndClear(&builder); + + EXPECT_CALL(builder, WithScheme("file")).Times(1).WillOnce( + ::testing::Return(&builder)); + EXPECT_CALL(builder, WithDomainWildcard()).Times(1).WillOnce( + ::testing::Return(&builder)); + EXPECT_CALL(builder, WithPath("/")).Times(1).WillOnce( + ::testing::Return(&builder)); + content_settings::PatternParser::Parse( + "file://*/", &builder); + ::testing::Mock::VerifyAndClear(&builder); + + EXPECT_CALL(builder, WithScheme("file")).Times(1).WillOnce( + ::testing::Return(&builder)); + EXPECT_CALL(builder, WithDomainWildcard()).Times(1).WillOnce( + ::testing::Return(&builder)); + EXPECT_CALL(builder, WithPathWildcard()).Times(1).WillOnce( + ::testing::Return(&builder)); + content_settings::PatternParser::Parse( + "file://*/*", &builder); + ::testing::Mock::VerifyAndClear(&builder); + + EXPECT_CALL(builder, WithScheme("file")).Times(1).WillOnce( + ::testing::Return(&builder)); + EXPECT_CALL(builder, WithPathWildcard()).Times(1).WillOnce( + ::testing::Return(&builder)); + content_settings::PatternParser::Parse( + "file:///*", &builder); + ::testing::Mock::VerifyAndClear(&builder); + + // Invalid file patterns. + EXPECT_CALL(builder, WithScheme("file")).Times(1).WillOnce( + ::testing::Return(&builder)); + EXPECT_CALL(builder, Invalid()).Times(1).WillOnce( + ::testing::Return(&builder)); + content_settings::PatternParser::Parse( + "file://**", &builder); + ::testing::Mock::VerifyAndClear(&builder); +} + TEST(ContentSettingsPatternParserTest, SerializePatterns) { ContentSettingsPattern::PatternParts parts; parts.scheme = "http"; @@ -154,4 +221,10 @@ TEST(ContentSettingsPatternParserTest, SerializePatterns) { parts.path = "/foo/bar/test.html"; EXPECT_STREQ("file:///foo/bar/test.html", content_settings::PatternParser::ToString(parts).c_str()); + + parts = ContentSettingsPattern::PatternParts(); + parts.scheme = "file"; + parts.path = ""; + parts.is_path_wildcard = true; + EXPECT_EQ("file:///*", content_settings::PatternParser::ToString(parts)); } diff --git a/chrome/common/content_settings_pattern_unittest.cc b/chrome/common/content_settings_pattern_unittest.cc index 254df17..f0eeca5 100644 --- a/chrome/common/content_settings_pattern_unittest.cc +++ b/chrome/common/content_settings_pattern_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -86,7 +86,7 @@ TEST(ContentSettingsPatternTest, FromURL) { pattern = ContentSettingsPattern::FromURL(GURL("file:///foo/bar.html")); EXPECT_TRUE(pattern.IsValid()); - EXPECT_STREQ("file:///foo/bar.html", pattern.ToString().c_str()); + EXPECT_EQ("file:///foo/bar.html", pattern.ToString()); } TEST(ContentSettingsPatternTest, FromURLNoWildcard) { @@ -172,37 +172,52 @@ TEST(ContentSettingsPatternTest, FromString_WithNoWildcards) { } TEST(ContentSettingsPatternTest, FromString_FilePatterns) { + // "/" is an invalid file path. EXPECT_FALSE(Pattern("file:///").IsValid()); // Non-empty domains aren't allowed in file patterns. EXPECT_FALSE(Pattern("file://foo/").IsValid()); - - // Domain wildcards aren't allowed in file patterns. + EXPECT_FALSE(Pattern("file://localhost/foo/bar/test.html").IsValid()); + EXPECT_FALSE(Pattern("file://*").IsValid()); EXPECT_FALSE(Pattern("file://*/").IsValid()); + EXPECT_FALSE(Pattern("file://*/*").IsValid()); + EXPECT_FALSE(Pattern("file://*/foo/bar/test.html").IsValid()); EXPECT_FALSE(Pattern("file://[*.]/").IsValid()); - // These specify a path that contains '*', which isn't allowed to avoid - // user confusion. - EXPECT_FALSE(Pattern("file:///*").IsValid()); + // This is the only valid file path wildcard format. + EXPECT_TRUE(Pattern("file:///*").IsValid()); + EXPECT_EQ("file:///*", Pattern("file:///*").ToString()); + + // Wildcards are not allowed anywhere in the file path. + EXPECT_FALSE(Pattern("file:///f*o/bar/file.html").IsValid()); + EXPECT_FALSE(Pattern("file:///*/bar/file.html").IsValid()); + EXPECT_FALSE(Pattern("file:///foo/*").IsValid()); EXPECT_FALSE(Pattern("file:///foo/bar/*").IsValid()); + EXPECT_FALSE(Pattern("file:///foo/*/file.html").IsValid()); + EXPECT_FALSE(Pattern("file:///foo/bar/*.html").IsValid()); + EXPECT_FALSE(Pattern("file:///foo/bar/file.*").IsValid()); EXPECT_TRUE(Pattern("file:///tmp/test.html").IsValid()); - EXPECT_STREQ("file:///tmp/file.html", - Pattern("file:///tmp/file.html").ToString().c_str()); + EXPECT_EQ("file:///tmp/file.html", + Pattern("file:///tmp/file.html").ToString()); EXPECT_TRUE(Pattern("file:///tmp/test.html").Matches( GURL("file:///tmp/test.html"))); EXPECT_FALSE(Pattern("file:///tmp/test.html").Matches( GURL("file:///tmp/other.html"))); EXPECT_FALSE(Pattern("file:///tmp/test.html").Matches( GURL("http://example.org/"))); + + EXPECT_TRUE(Pattern("file:///*").Matches(GURL("file:///tmp/test.html"))); + EXPECT_TRUE(Pattern("file:///*").Matches( + GURL("file://localhost/tmp/test.html"))); } TEST(ContentSettingsPatternTest, FromString_ExtensionPatterns) { EXPECT_TRUE(Pattern("chrome-extension://peoadpeiejnhkmpaakpnompolbglelel/") .IsValid()); - EXPECT_STREQ("chrome-extension://peoadpeiejnhkmpaakpnompolbglelel/", + EXPECT_EQ("chrome-extension://peoadpeiejnhkmpaakpnompolbglelel/", Pattern("chrome-extension://peoadpeiejnhkmpaakpnompolbglelel/") - .ToString().c_str()); + .ToString()); EXPECT_TRUE(Pattern("chrome-extension://peoadpeiejnhkmpaakpnompolbglelel/") .Matches(GURL("chrome-extension://peoadpeiejnhkmpaakpnompolbglelel/"))); } diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h index 39c7116..4b4241d 100644 --- a/chrome/common/render_messages.h +++ b/chrome/common/render_messages.h @@ -131,6 +131,7 @@ IPC_STRUCT_TRAITS_BEGIN(ContentSettingsPattern::PatternParts) IPC_STRUCT_TRAITS_MEMBER(port) IPC_STRUCT_TRAITS_MEMBER(is_port_wildcard) IPC_STRUCT_TRAITS_MEMBER(path) + IPC_STRUCT_TRAITS_MEMBER(is_path_wildcard) IPC_STRUCT_TRAITS_END() IPC_STRUCT_TRAITS_BEGIN(ContentSettingPatternSource) |