- Revision
- 206231
- Author
- achristen...@apple.com
- Date
- 2016-09-21 13:19:07 -0700 (Wed, 21 Sep 2016)
Log Message
URLParser should fail when parsing invalid relative URLs with no schemes
https://bugs.webkit.org/show_bug.cgi?id=162355
Reviewed by Tim Horton.
Source/WebCore:
Covered by new API tests.
* platform/URLParser.cpp:
(WebCore::copyASCIIStringUntil):
When copying from a null String, is8Bit dereferences a null pointer. We don't want to do that.
(WebCore::URLParser::parse):
What the spec calls a "null" URL matches !url.isValid(), not url.isNull().
The former reflects whether the parsing succeeded,
the latter whether the contained String (which could be an invalid URL) is null.
Tools:
* TestWebKitAPI/Tests/WebCore/URLParser.cpp:
(TestWebKitAPI::TEST_F):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (206230 => 206231)
--- trunk/Source/WebCore/ChangeLog 2016-09-21 19:59:47 UTC (rev 206230)
+++ trunk/Source/WebCore/ChangeLog 2016-09-21 20:19:07 UTC (rev 206231)
@@ -1,3 +1,20 @@
+2016-09-21 Alex Christensen <achristen...@webkit.org>
+
+ URLParser should fail when parsing invalid relative URLs with no schemes
+ https://bugs.webkit.org/show_bug.cgi?id=162355
+
+ Reviewed by Tim Horton.
+
+ Covered by new API tests.
+
+ * platform/URLParser.cpp:
+ (WebCore::copyASCIIStringUntil):
+ When copying from a null String, is8Bit dereferences a null pointer. We don't want to do that.
+ (WebCore::URLParser::parse):
+ What the spec calls a "null" URL matches !url.isValid(), not url.isNull().
+ The former reflects whether the parsing succeeded,
+ the latter whether the contained String (which could be an invalid URL) is null.
+
2016-09-21 Antti Koivisto <an...@apple.com>
Document::styleResolverChanged simplification
Modified: trunk/Source/WebCore/platform/URLParser.cpp (206230 => 206231)
--- trunk/Source/WebCore/platform/URLParser.cpp 2016-09-21 19:59:47 UTC (rev 206230)
+++ trunk/Source/WebCore/platform/URLParser.cpp 2016-09-21 20:19:07 UTC (rev 206231)
@@ -705,6 +705,11 @@
inline static void copyASCIIStringUntil(Vector<LChar>& destination, const String& string, size_t lengthIf8Bit, size_t lengthIf16Bit)
{
+ if (string.isNull()) {
+ ASSERT(!lengthIf8Bit);
+ ASSERT(!lengthIf16Bit);
+ return;
+ }
ASSERT(destination.isEmpty());
if (string.is8Bit()) {
RELEASE_ASSERT(lengthIf8Bit <= string.length());
@@ -1069,7 +1074,7 @@
break;
case State::NoScheme:
LOG_STATE("NoScheme");
- if (base.isNull() || (base.m_cannotBeABaseURL && *c != '#'))
+ if (!base.isValid() || (base.m_cannotBeABaseURL && *c != '#'))
return failure(input, length);
if (base.m_cannotBeABaseURL && *c == '#') {
copyURLPartsUntil(base, URLPart::QueryEnd);
@@ -1240,7 +1245,7 @@
++c;
break;
case '?':
- if (!base.isNull() && base.protocolIs("file"))
+ if (base.isValid() && base.protocolIs("file"))
copyURLPartsUntil(base, URLPart::PathEnd);
m_asciiBuffer.append("///?", 4);
m_url.m_userStart = m_asciiBuffer.size() - 2;
@@ -1254,7 +1259,7 @@
++c;
break;
case '#':
- if (!base.isNull() && base.protocolIs("file"))
+ if (base.isValid() && base.protocolIs("file"))
copyURLPartsUntil(base, URLPart::QueryEnd);
m_asciiBuffer.append("///#", 4);
m_url.m_userStart = m_asciiBuffer.size() - 2;
@@ -1269,7 +1274,7 @@
++c;
break;
default:
- if (!base.isNull() && base.protocolIs("file") && shouldCopyFileURL<serialized>(c))
+ if (base.isValid() && base.protocolIs("file") && shouldCopyFileURL<serialized>(c))
copyURLPartsUntil(base, URLPart::PathAfterLastSlash);
else {
m_asciiBuffer.append("///", 3);
@@ -1299,7 +1304,7 @@
state = State::FileHost;
break;
}
- if (!base.isNull() && base.protocolIs("file")) {
+ if (base.isValid() && base.protocolIs("file")) {
// FIXME: This String copy is unnecessary.
String basePath = base.path();
if (basePath.length() >= 2) {
@@ -1459,7 +1464,7 @@
switch (state) {
case State::SchemeStart:
LOG_FINAL_STATE("SchemeStart");
- if (!m_asciiBuffer.size() && !base.isNull())
+ if (!m_asciiBuffer.size() && base.isValid())
return base;
return failure(input, length);
case State::Scheme:
@@ -1544,7 +1549,7 @@
break;
case State::File:
LOG_FINAL_STATE("File");
- if (!base.isNull() && base.protocolIs("file")) {
+ if (base.isValid() && base.protocolIs("file")) {
copyURLPartsUntil(base, URLPart::QueryEnd);
m_asciiBuffer.append(':');
}
@@ -2103,7 +2108,7 @@
}
}
- ASSERT(!serialized || m_hostHasPercentOrNonASCII);
+ ASSERT(!serialized || !m_hostHasPercentOrNonASCII);
if (!m_hostHasPercentOrNonASCII) {
auto hostIterator = iterator;
for (; !iterator.atEnd(); ++iterator) {
Modified: trunk/Tools/ChangeLog (206230 => 206231)
--- trunk/Tools/ChangeLog 2016-09-21 19:59:47 UTC (rev 206230)
+++ trunk/Tools/ChangeLog 2016-09-21 20:19:07 UTC (rev 206231)
@@ -1,3 +1,13 @@
+2016-09-21 Alex Christensen <achristen...@webkit.org>
+
+ URLParser should fail when parsing invalid relative URLs with no schemes
+ https://bugs.webkit.org/show_bug.cgi?id=162355
+
+ Reviewed by Tim Horton.
+
+ * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+ (TestWebKitAPI::TEST_F):
+
2016-09-21 Keith Miller <keith_mil...@apple.com>
Fix build for future versions of Clang.
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp (206230 => 206231)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp 2016-09-21 19:59:47 UTC (rev 206230)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp 2016-09-21 20:19:07 UTC (rev 206231)
@@ -311,6 +311,7 @@
checkRelativeURL("notspecial:/", "about:blank", {"notspecial", "", "", "", 0, "/", "", "", "notspecial:/"});
checkRelativeURL("notspecial:/", "http://host", {"notspecial", "", "", "", 0, "/", "", "", "notspecial:/"});
checkRelativeURL("foo:/", "http://example.org/foo/bar", {"foo", "", "", "", 0, "/", "", "", "foo:/"});
+ checkRelativeURL("://:0/", "http://webkit.org/", {"http", "", "", "webkit.org", 0, "/://:0/", "", "", "http://webkit.org/://:0/"});
// The checking of slashes in SpecialAuthoritySlashes needed to get this to pass contradicts what is in the spec,
// but it is included in the web platform tests.
@@ -710,6 +711,9 @@
shouldFail("~");
shouldFail("~", "about:blank");
shouldFail("~~~");
+ shouldFail("://:0/");
+ shouldFail("://:0/", "");
+ shouldFail("://:0/", "about:blank");
}
// These are in the spec but not in the web platform tests.
