Skip to content

Commit

Permalink
Align some internal URL concepts with the URL Standard
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=259245
rdar://112326744

Reviewed by Alex Christensen.

This is a minor refactoring to align with URL Standard changes:

* Rename cannotBeABaseURL to hasOpaquePath.
* Remove isHierarchical and friends as they are the same as
  hasOpaquePath.
* As an opaque path implies a null host, a null host check suffices.

Also add a FIXME to CSP as it does not appear to follow the
specification for reporting URLs.

* Source/WTF/wtf/URL.cpp:
(WTF::URL::invalidate):
(WTF::URL::isHierarchical const): Deleted.
* Source/WTF/wtf/URL.h:
(WTF::URL::hasOpaquePath const):
(WTF::URL::canSetHostOrPort const): Deleted.
(WTF::URL::canSetPathname const): Deleted.
(WTF::URL::cannotBeABaseURL const): Deleted.
* Source/WTF/wtf/URLParser.cpp:
(WTF::URLParser::URLParser):
(WTF::URLParser::parse):
(WTF::URLParser::allValuesEqual):
* Source/WebCore/html/URLDecomposition.cpp:
(WebCore::URLDecomposition::setUsername):
(WebCore::URLDecomposition::setPassword):
(WebCore::URLDecomposition::setHost):
(WebCore::URLDecomposition::setHostname):
(WebCore::URLDecomposition::setPort):
(WebCore::URLDecomposition::setPathname):
* Source/WebCore/page/csp/ContentSecurityPolicy.cpp:
(WebCore::shouldReportProtocolOnly):

Canonical link: https://commits.webkit.org/266090@main
  • Loading branch information
annevk committed Jul 16, 2023
1 parent 3f69e13 commit 638ac81
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 39 deletions.
10 changes: 1 addition & 9 deletions Source/WTF/wtf/URL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ void URL::invalidate()
{
m_isValid = false;
m_protocolIsInHTTPFamily = false;
m_cannotBeABaseURL = false;
m_hasOpaquePath = false;
m_schemeEnd = 0;
m_userStart = 0;
m_userEnd = 0;
Expand Down Expand Up @@ -796,14 +796,6 @@ String encodeWithURLEscapeSequences(const String& input)
return percentEncodeCharacters(input, URLParser::isInUserInfoEncodeSet);
}

bool URL::isHierarchical() const
{
if (!m_isValid)
return false;
ASSERT(m_string[m_schemeEnd] == ':');
return m_string[m_schemeEnd + 1] == '/';
}

static bool protocolIsInternal(StringView string, ASCIILiteral protocolLiteral)
{
assertProtocolIsGood(protocolLiteral);
Expand Down
11 changes: 2 additions & 9 deletions Source/WTF/wtf/URL.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,6 @@ class URL {
// when placing a URL in an if statment.
operator bool() const = delete;

// Returns true if you can set the host and port for the URL.
// Non-hierarchical URLs don't have a host and port.
bool canSetHostOrPort() const { return isHierarchical(); }

bool canSetPathname() const { return isHierarchical(); }
WTF_EXPORT_PRIVATE bool isHierarchical() const;

const String& string() const { return m_string; }
WTF_EXPORT_PRIVATE String stringCenterEllipsizedToLength(unsigned length = 1024) const;

Expand Down Expand Up @@ -149,7 +142,7 @@ class URL {
WTF_EXPORT_PRIVATE bool protocolIsJavaScript() const;
bool protocolIsInFTPFamily() const { return protocolIs("ftp"_s) || protocolIs("ftps"_s); }
bool protocolIsInHTTPFamily() const { return m_protocolIsInHTTPFamily; }
bool cannotBeABaseURL() const { return m_cannotBeABaseURL; }
bool hasOpaquePath() const { return m_hasOpaquePath; }

WTF_EXPORT_PRIVATE bool isAboutBlank() const;
WTF_EXPORT_PRIVATE bool isAboutSrcDoc() const;
Expand Down Expand Up @@ -230,7 +223,7 @@ class URL {

unsigned m_isValid : 1;
unsigned m_protocolIsInHTTPFamily : 1;
unsigned m_cannotBeABaseURL : 1;
unsigned m_hasOpaquePath : 1;

// This is out of order to align the bits better. The port is after the host.
unsigned m_portLength : 3;
Expand Down
28 changes: 14 additions & 14 deletions Source/WTF/wtf/URLParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1082,7 +1082,7 @@ URLParser::URLParser(String&& input, const URL& base, const URLTextEncoding* non
: m_inputString(WTFMove(input))
{
if (m_inputString.isNull()) {
if (base.isValid() && !base.m_cannotBeABaseURL) {
if (base.isValid() && !base.m_hasOpaquePath) {
m_url = base;
m_url.removeFragmentIdentifier();
}
Expand Down Expand Up @@ -1160,7 +1160,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
FileHost,
PathStart,
Path,
CannotBeABaseURLPath,
OpaquePath,
UTF8Query,
NonUTF8Query,
Fragment,
Expand Down Expand Up @@ -1258,8 +1258,8 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
m_url.m_hostEnd = m_url.m_userStart;
m_url.m_portLength = 0;
m_url.m_pathAfterLastSlash = m_url.m_userStart;
m_url.m_cannotBeABaseURL = true;
state = State::CannotBeABaseURLPath;
m_url.m_hasOpaquePath = true;
state = State::OpaquePath;
}
break;
}
Expand All @@ -1279,11 +1279,11 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
break;
case State::NoScheme:
LOG_STATE("NoScheme");
if (!base.isValid() || (base.m_cannotBeABaseURL && *c != '#')) {
if (!base.isValid() || (base.m_hasOpaquePath && *c != '#')) {
failure();
return;
}
if (base.m_cannotBeABaseURL && *c == '#') {
if (base.m_hasOpaquePath && *c == '#') {
copyURLPartsUntil(base, URLPart::QueryEnd, c, nonUTF8QueryEncoding);
state = State::Fragment;
appendToASCIIBuffer('#');
Expand Down Expand Up @@ -1747,8 +1747,8 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
utf8PercentEncode<isInDefaultEncodeSet>(c);
++c;
break;
case State::CannotBeABaseURLPath:
LOG_STATE("CannotBeABaseURLPath");
case State::OpaquePath:
LOG_STATE("OpaquePath");
if (*c == '?') {
m_url.m_pathEnd = currentPosition(c);
appendToASCIIBuffer('?');
Expand Down Expand Up @@ -1808,7 +1808,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
switch (state) {
case State::SchemeStart:
LOG_FINAL_STATE("SchemeStart");
if (!currentPosition(c) && base.isValid() && !base.m_cannotBeABaseURL) {
if (!currentPosition(c) && base.isValid() && !base.m_hasOpaquePath) {
m_url = base;
m_url.removeFragmentIdentifier();
return;
Expand Down Expand Up @@ -1998,8 +1998,8 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
m_url.m_pathEnd = currentPosition(c);
m_url.m_queryEnd = m_url.m_pathEnd;
break;
case State::CannotBeABaseURLPath:
LOG_FINAL_STATE("CannotBeABaseURLPath");
case State::OpaquePath:
LOG_FINAL_STATE("OpaquePath");
m_url.m_pathEnd = currentPosition(c);
m_url.m_queryEnd = m_url.m_pathEnd;
break;
Expand Down Expand Up @@ -2977,7 +2977,7 @@ bool URLParser::allValuesEqual(const URL& a, const URL& b)
{
URL_PARSER_LOG("%d %d %d %d %d %d %d %d %d %d %d %d %s\n%d %d %d %d %d %d %d %d %d %d %d %d %s",
a.m_isValid,
a.m_cannotBeABaseURL,
a.m_hasOpaquePath,
a.m_protocolIsInHTTPFamily,
a.m_schemeEnd,
a.m_userStart,
Expand All @@ -2990,7 +2990,7 @@ bool URLParser::allValuesEqual(const URL& a, const URL& b)
a.m_queryEnd,
a.m_string.utf8().data(),
b.m_isValid,
b.m_cannotBeABaseURL,
b.m_hasOpaquePath,
b.m_protocolIsInHTTPFamily,
b.m_schemeEnd,
b.m_userStart,
Expand All @@ -3005,7 +3005,7 @@ bool URLParser::allValuesEqual(const URL& a, const URL& b)

return a.m_string == b.m_string
&& a.m_isValid == b.m_isValid
&& a.m_cannotBeABaseURL == b.m_cannotBeABaseURL
&& a.m_hasOpaquePath == b.m_hasOpaquePath
&& a.m_protocolIsInHTTPFamily == b.m_protocolIsInHTTPFamily
&& a.m_schemeEnd == b.m_schemeEnd
&& a.m_userStart == b.m_userStart
Expand Down
12 changes: 6 additions & 6 deletions Source/WebCore/html/URLDecomposition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ String URLDecomposition::username() const
void URLDecomposition::setUsername(StringView user)
{
auto fullURL = this->fullURL();
if (fullURL.host().isEmpty() || fullURL.cannotBeABaseURL() || fullURL.protocolIsFile())
if (fullURL.host().isEmpty() || fullURL.protocolIsFile())
return;
fullURL.setUser(user);
setFullURL(fullURL);
Expand All @@ -73,7 +73,7 @@ String URLDecomposition::password() const
void URLDecomposition::setPassword(StringView password)
{
auto fullURL = this->fullURL();
if (fullURL.host().isEmpty() || fullURL.cannotBeABaseURL() || fullURL.protocolIsFile())
if (fullURL.host().isEmpty() || fullURL.protocolIsFile())
return;
fullURL.setPassword(password);
setFullURL(fullURL);
Expand Down Expand Up @@ -104,7 +104,7 @@ void URLDecomposition::setHost(StringView value)
if (!separator)
return;

if (fullURL.cannotBeABaseURL() || !fullURL.canSetHostOrPort())
if (fullURL.hasOpaquePath())
return;

// No port if no colon or rightmost colon is within the IPv6 section.
Expand Down Expand Up @@ -140,7 +140,7 @@ void URLDecomposition::setHostname(StringView host)
auto fullURL = this->fullURL();
if (host.isEmpty() && !fullURL.protocolIsFile() && fullURL.hasSpecialScheme())
return;
if (fullURL.cannotBeABaseURL() || !fullURL.canSetHostOrPort())
if (fullURL.hasOpaquePath())
return;
fullURL.setHost(host);
if (fullURL.isValid())
Expand Down Expand Up @@ -185,7 +185,7 @@ static std::optional<std::optional<uint16_t>> parsePort(StringView string, Strin
void URLDecomposition::setPort(StringView value)
{
auto fullURL = this->fullURL();
if (fullURL.host().isEmpty() || fullURL.cannotBeABaseURL() || fullURL.protocolIsFile() || !fullURL.canSetHostOrPort())
if (fullURL.host().isEmpty() || fullURL.protocolIsFile())
return;
auto port = parsePort(value, fullURL.protocol());
if (!port)
Expand All @@ -202,7 +202,7 @@ String URLDecomposition::pathname() const
void URLDecomposition::setPathname(StringView value)
{
auto fullURL = this->fullURL();
if (fullURL.cannotBeABaseURL() || !fullURL.canSetPathname())
if (fullURL.hasOpaquePath())
return;
fullURL.setPath(value);
setFullURL(fullURL);
Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/page/csp/ContentSecurityPolicy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,9 @@ bool ContentSecurityPolicy::allowBaseURI(const URL& url, bool overrideContentSec

static bool shouldReportProtocolOnly(const URL& url)
{
return !url.isHierarchical() || url.protocolIsFile();
// FIXME: https://w3c.github.io/webappsec-csp/#strip-url-for-use-in-reports suggests this should
// be url.protocolIsInHTTPFamily().
return url.hasOpaquePath() || url.protocolIsFile();
}

String ContentSecurityPolicy::createURLForReporting(const URL& url, const String& violatedDirective, bool usesReportingAPI) const
Expand Down

0 comments on commit 638ac81

Please sign in to comment.