Skip to content

Commit

Permalink
Revert of CSP: Convert CSPSource constructor bools into a enum. (patc…
Browse files Browse the repository at this point in the history 
…hset #1 id:1 of https://codereview.chromium.org/568433002/)

Reason for revert:
Your patch broke quite a number of webkit tests:
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.8%20%28retina%29/builds/19050
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.7/builds/31462

Original issue's description:
> CSP: Convert CSPSource constructor bools into a enum.
> 
> Rather than passing the wildcard disposition of CSPSource's hosts and ports
> as booleans, we should have an enum that allows us to figure out hat we
> mean at the callsite. This patch adds CSPSource::WildcardDisposition in
> order to make this clear.
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181821

[email protected],[email protected]
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/565723002

git-svn-id: svn://svn.chromium.org/blink/trunk@181833 bbb929c8-8fbe-4397-9dbb-9b2b20218538
  • Loading branch information
@jianli-chromium
jianli-chromium committed Sep 11, 2014
1 parent 55d63a3 commit 5af3948
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 33 deletions.
10 changes: 5 additions & 5 deletions Source/core/frame/csp/CSPSource.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@

namespace blink {

CSPSource::CSPSource(ContentSecurityPolicy* policy, const String& scheme, const String& host, int port, const String& path, WildcardDisposition hostWildcard, WildcardDisposition portWildcard)
CSPSource::CSPSource(ContentSecurityPolicy* policy, const String& scheme, const String& host, int port, const String& path, bool hostHasWildcard, bool portHasWildcard)
: m_policy(policy)
, m_scheme(scheme)
, m_host(host)
, m_port(port)
, m_path(path)
, m_hostWildcard(hostWildcard)
, m_portWildcard(portWildcard)
, m_hostHasWildcard(hostHasWildcard)
, m_portHasWildcard(portHasWildcard)
{
}

Expand All @@ -45,7 +45,7 @@ bool CSPSource::hostMatches(const KURL& url) const
const String& host = url.host();
if (equalIgnoringCase(host, m_host))
return true;
return m_hostWildcard == HasWildcard && host.endsWith("." + m_host, false);
return m_hostHasWildcard && host.endsWith("." + m_host, false);

}

Expand All @@ -64,7 +64,7 @@ bool CSPSource::pathMatches(const KURL& url) const

bool CSPSource::portMatches(const KURL& url) const
{
if (m_portWildcard == HasWildcard)
if (m_portHasWildcard)
return true;

int port = url.port();
Expand Down
11 changes: 3 additions & 8 deletions Source/core/frame/csp/CSPSource.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,7 @@ class KURL;

class CSPSource {
public:
enum WildcardDisposition {
HasWildcard,
NoWildcard
};

CSPSource(ContentSecurityPolicy*, const String& scheme, const String& host, int port, const String& path, WildcardDisposition hostWildcard, WildcardDisposition portWildcard);
CSPSource(ContentSecurityPolicy*, const String& scheme, const String& host, int port, const String& path, bool hostHasWildcard, bool portHasWildcard);
bool matches(const KURL&) const;

private:
Expand All @@ -35,8 +30,8 @@ class CSPSource {
int m_port;
String m_path;

WildcardDisposition m_hostWildcard;
WildcardDisposition m_portWildcard;
bool m_hostHasWildcard;
bool m_portHasWildcard;
};

} // namespace
Expand Down
30 changes: 15 additions & 15 deletions Source/core/frame/csp/CSPSourceList.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,18 @@ void CSPSourceList::parse(const UChar* begin, const UChar* end)

String scheme, host, path;
int port = 0;
CSPSource::WildcardDisposition hostWildcard = CSPSource::NoWildcard;
CSPSource::WildcardDisposition portWildcard = CSPSource::NoWildcard;
bool hostHasWildcard = false;
bool portHasWildcard = false;

if (parseSource(beginSource, position, scheme, host, port, path, hostWildcard, portWildcard)) {
if (parseSource(beginSource, position, scheme, host, port, path, hostHasWildcard, portHasWildcard)) {
// Wildcard hosts and keyword sources ('self', 'unsafe-inline',
// etc.) aren't stored in m_list, but as attributes on the source
// list itself.
if (scheme.isEmpty() && host.isEmpty())
continue;
if (m_policy->isDirectiveName(host))
m_policy->reportDirectiveAsSourceExpression(m_directiveName, host);
m_list.append(CSPSource(m_policy, scheme, host, port, path, hostWildcard, portWildcard));
m_list.append(CSPSource(m_policy, scheme, host, port, path, hostHasWildcard, portHasWildcard));
} else {
m_policy->reportInvalidSourceExpression(m_directiveName, String(beginSource, position - beginSource));
}
Expand All @@ -134,7 +134,7 @@ void CSPSourceList::parse(const UChar* begin, const UChar* end)
// source = scheme ":"
// / ( [ scheme "://" ] host [ port ] [ path ] )
// / "'self'"
bool CSPSourceList::parseSource(const UChar* begin, const UChar* end, String& scheme, String& host, int& port, String& path, CSPSource::WildcardDisposition& hostWildcard, CSPSource::WildcardDisposition& portWildcard)
bool CSPSourceList::parseSource(const UChar* begin, const UChar* end, String& scheme, String& host, int& port, String& path, bool& hostHasWildcard, bool& portHasWildcard)
{
if (begin == end)
return false;
Expand Down Expand Up @@ -191,13 +191,13 @@ bool CSPSourceList::parseSource(const UChar* begin, const UChar* end, String& sc
if (position == end) {
// host
// ^
return parseHost(beginHost, position, host, hostWildcard);
return parseHost(beginHost, position, host, hostHasWildcard);
}

if (position < end && *position == '/') {
// host/path || host/ || /
// ^ ^ ^
return parseHost(beginHost, position, host, hostWildcard) && parsePath(position, end, path);
return parseHost(beginHost, position, host, hostHasWildcard) && parsePath(position, end, path);
}

if (position < end && *position == ':') {
Expand Down Expand Up @@ -237,11 +237,11 @@ bool CSPSourceList::parseSource(const UChar* begin, const UChar* end, String& sc
beginPath = position;
}

if (!parseHost(beginHost, beginPort ? beginPort : beginPath, host, hostWildcard))
if (!parseHost(beginHost, beginPort ? beginPort : beginPath, host, hostHasWildcard))
return false;

if (beginPort) {
if (!parsePort(beginPort, beginPath, port, portWildcard))
if (!parsePort(beginPort, beginPath, port, portHasWildcard))
return false;
} else {
port = 0;
Expand Down Expand Up @@ -366,19 +366,19 @@ bool CSPSourceList::parseScheme(const UChar* begin, const UChar* end, String& sc
// / "*"
// host-char = ALPHA / DIGIT / "-"
//
bool CSPSourceList::parseHost(const UChar* begin, const UChar* end, String& host, CSPSource::WildcardDisposition& hostWildcard)
bool CSPSourceList::parseHost(const UChar* begin, const UChar* end, String& host, bool& hostHasWildcard)
{
ASSERT(begin <= end);
ASSERT(host.isEmpty());
ASSERT(hostWildcard == CSPSource::NoWildcard);
ASSERT(!hostHasWildcard);

if (begin == end)
return false;

const UChar* position = begin;

if (skipExactly<UChar>(position, end, '*')) {
hostWildcard = CSPSource::HasWildcard;
hostHasWildcard = true;

if (position == end)
return true;
Expand Down Expand Up @@ -425,11 +425,11 @@ bool CSPSourceList::parsePath(const UChar* begin, const UChar* end, String& path

// port = ":" ( 1*DIGIT / "*" )
//
bool CSPSourceList::parsePort(const UChar* begin, const UChar* end, int& port, CSPSource::WildcardDisposition& portWildcard)
bool CSPSourceList::parsePort(const UChar* begin, const UChar* end, int& port, bool& portHasWildcard)
{
ASSERT(begin <= end);
ASSERT(!port);
ASSERT(portWildcard == CSPSource::NoWildcard);
ASSERT(!portHasWildcard);

if (!skipExactly<UChar>(begin, end, ':'))
ASSERT_NOT_REACHED();
Expand All @@ -439,7 +439,7 @@ bool CSPSourceList::parsePort(const UChar* begin, const UChar* end, int& port, C

if (end - begin == 1 && *begin == '*') {
port = 0;
portWildcard = CSPSource::HasWildcard;
portHasWildcard = true;
return true;
}

Expand Down
6 changes: 3 additions & 3 deletions Source/core/frame/csp/CSPSourceList.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ class CSPSourceList {
bool isHashOrNoncePresent() const;

private:
bool parseSource(const UChar* begin, const UChar* end, String& scheme, String& host, int& port, String& path, CSPSource::WildcardDisposition&, CSPSource::WildcardDisposition&);
bool parseSource(const UChar* begin, const UChar* end, String& scheme, String& host, int& port, String& path, bool& hostHasWildcard, bool& portHasWildcard);
bool parseScheme(const UChar* begin, const UChar* end, String& scheme);
bool parseHost(const UChar* begin, const UChar* end, String& host, CSPSource::WildcardDisposition&);
bool parsePort(const UChar* begin, const UChar* end, int& port, CSPSource::WildcardDisposition&);
bool parseHost(const UChar* begin, const UChar* end, String& host, bool& hostHasWildcard);
bool parsePort(const UChar* begin, const UChar* end, int& port, bool& portHasWildcard);
bool parsePath(const UChar* begin, const UChar* end, String& path);
bool parseNonce(const UChar* begin, const UChar* end, String& nonce);
bool parseHash(const UChar* begin, const UChar* end, DigestValue& hash, ContentSecurityPolicyHashAlgorithm&);
Expand Down
4 changes: 2 additions & 2 deletions Source/core/frame/csp/ContentSecurityPolicy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ void ContentSecurityPolicy::applyPolicySideEffectsToExecutionContext()
{
ASSERT(m_executionContext);
// Ensure that 'self' processes correctly.
m_selfSource = adoptPtr(new CSPSource(this, securityOrigin()->protocol(), securityOrigin()->host(), securityOrigin()->port(), String(), CSPSource::NoWildcard, CSPSource::NoWildcard));
m_selfSource = adoptPtr(new CSPSource(this, securityOrigin()->protocol(), securityOrigin()->host(), securityOrigin()->port(), String(), false, false));

// If we're in a Document, set the referrer policy and sandbox flags, then dump all the
// parsing error messages, then poke at histograms.
Expand Down Expand Up @@ -262,7 +262,7 @@ void ContentSecurityPolicy::setOverrideURLForSelf(const KURL& url)
// an execution context (for 'frame-ancestor' resolution, for example). This CSPSource will
// be overwritten when we bind this object to an execution context.
RefPtr<SecurityOrigin> origin = SecurityOrigin::create(url);
m_selfSource = adoptPtr(new CSPSource(this, origin->protocol(), origin->host(), origin->port(), String(), CSPSource::NoWildcard, CSPSource::NoWildcard));
m_selfSource = adoptPtr(new CSPSource(this, origin->protocol(), origin->host(), origin->port(), String(), false, false));
}

const String& ContentSecurityPolicy::deprecatedHeader() const
Expand Down

0 comments on commit 5af3948

Please sign in to comment.