Log Message
Fix more edge cases in URLParser https://bugs.webkit.org/show_bug.cgi?id=162051
Reviewed by Tim Horton. Source/WebCore: Added new API tests. * platform/URLParser.cpp: (WebCore::URLParser::parse): Some edge case handling was wrong. Also, some of the terminal states are not possible to reach because we transition to those states without incrementing the iterator. Tools: * TestWebKitAPI/Tests/WebCore/URLParser.cpp: (TestWebKitAPI::TEST_F):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (206034 => 206035)
--- trunk/Source/WebCore/ChangeLog 2016-09-16 18:49:31 UTC (rev 206034)
+++ trunk/Source/WebCore/ChangeLog 2016-09-16 18:57:07 UTC (rev 206035)
@@ -1,5 +1,19 @@
2016-09-16 Alex Christensen <achristen...@webkit.org>
+ Fix more edge cases in URLParser
+ https://bugs.webkit.org/show_bug.cgi?id=162051
+
+ Reviewed by Tim Horton.
+
+ Added new API tests.
+
+ * platform/URLParser.cpp:
+ (WebCore::URLParser::parse):
+ Some edge case handling was wrong. Also, some of the terminal states are not possible
+ to reach because we transition to those states without incrementing the iterator.
+
+2016-09-16 Alex Christensen <achristen...@webkit.org>
+
Fix Windows clean build after r205929
* DerivedSources.cpp:
Modified: trunk/Source/WebCore/platform/URLParser.cpp (206034 => 206035)
--- trunk/Source/WebCore/platform/URLParser.cpp 2016-09-16 18:49:31 UTC (rev 206034)
+++ trunk/Source/WebCore/platform/URLParser.cpp 2016-09-16 18:57:07 UTC (rev 206035)
@@ -919,6 +919,8 @@
m_buffer.append(':');
if (isSpecialScheme(urlScheme)) {
m_urlIsSpecial = true;
+ // FIXME: This is unnecessarily allocating a String.
+ // This should be easy to optimize once https://bugs.webkit.org/show_bug.cgi?id=162035 lands.
if (base.protocol() == urlScheme)
state = State::SpecialRelativeOrAuthority;
else
@@ -1056,9 +1058,7 @@
++c;
while (!c.atEnd() && isTabOrNewline(*c))
++c;
- if (c.atEnd())
- return failure(input, length);
- if (*c == '/' || *c == '\\')
+ if (!c.atEnd() && (*c == '/' || *c == '\\'))
++c;
}
state = State::SpecialAuthorityIgnoreSlashes;
@@ -1347,10 +1347,10 @@
return failure(input, length);
case State::Scheme:
LOG_FINAL_STATE("Scheme");
- break;
+ return failure(input, length);
case State::NoScheme:
LOG_FINAL_STATE("NoScheme");
- break;
+ RELEASE_ASSERT_NOT_REACHED();
case State::SpecialRelativeOrAuthority:
LOG_FINAL_STATE("SpecialRelativeOrAuthority");
copyURLPartsUntil(base, URLPart::QueryEnd);
@@ -1358,6 +1358,9 @@
break;
case State::PathOrAuthority:
LOG_FINAL_STATE("PathOrAuthority");
+ m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
+ m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
+ m_url.m_fragmentEnd = m_url.m_pathAfterLastSlash;
break;
case State::Relative:
LOG_FINAL_STATE("Relative");
@@ -1387,6 +1390,7 @@
case State::SpecialAuthorityIgnoreSlashes:
LOG_FINAL_STATE("SpecialAuthorityIgnoreSlashes");
return failure(input, length);
+ break;
case State::AuthorityOrHost:
LOG_FINAL_STATE("AuthorityOrHost");
m_url.m_userEnd = m_buffer.length();
@@ -1465,7 +1469,7 @@
break;
case State::PathStart:
LOG_FINAL_STATE("PathStart");
- break;
+ RELEASE_ASSERT_NOT_REACHED();
case State::Path:
LOG_FINAL_STATE("Path");
m_url.m_pathEnd = m_buffer.length();
Modified: trunk/Tools/ChangeLog (206034 => 206035)
--- trunk/Tools/ChangeLog 2016-09-16 18:49:31 UTC (rev 206034)
+++ trunk/Tools/ChangeLog 2016-09-16 18:57:07 UTC (rev 206035)
@@ -1,3 +1,13 @@
+2016-09-16 Alex Christensen <achristen...@webkit.org>
+
+ Fix more edge cases in URLParser
+ https://bugs.webkit.org/show_bug.cgi?id=162051
+
+ Reviewed by Tim Horton.
+
+ * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+ (TestWebKitAPI::TEST_F):
+
2016-09-16 Wenson Hsieh <wenson_hs...@apple.com>
Inserting a space after inserting an accepted candidate scrolls the document and causes a flicker
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp (206034 => 206035)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp 2016-09-16 18:49:31 UTC (rev 206034)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp 2016-09-16 18:57:07 UTC (rev 206035)
@@ -203,6 +203,10 @@
checkURL("sc:/pa/", {"sc", "", "", "", 0, "/pa/", "", "", "sc:/pa/"});
checkURL("sc://pa/", {"sc", "", "", "pa", 0, "/", "", "", "sc://pa/"});
checkURL("http://host \a ", {"http", "", "", "host", 0, "/", "", "", "http://host/"});
+ checkURL("notspecial:/", {"notspecial", "", "", "", 0, "/", "", "", "notspecial:/"});
+ checkURL("notspecial:/a", {"notspecial", "", "", "", 0, "/a", "", "", "notspecial:/a"});
+ checkURL("notspecial:", {"notspecial", "", "", "", 0, "", "", "", "notspecial:"});
+ checkURL("http:/a", {"http", "", "", "a", 0, "/", "", "", "http://a/"});
// FIXME: Fix and add a test with an invalid surrogate pair at the end with a space as the second code unit.
// This disagrees with the web platform test for http://:@www.example.com but agrees with Chrome and URL::parse,
@@ -279,6 +283,12 @@
checkRelativeURL("#x", "about:blank", {"about", "", "", "", 0, "blank", "", "x", "about:blank#x"});
checkRelativeURL(" foo.com ", "http://example.org/foo/bar", {"http", "", "", "example.org", 0, "/foo/foo.com", "", "", "http://example.org/foo/foo.com"});
checkRelativeURL(" \a baz", "http://example.org/foo/bar", {"http", "", "", "example.org", 0, "/foo/baz", "", "", "http://example.org/foo/baz"});
+ checkRelativeURL("~", "http://example.org", {"http", "", "", "example.org", 0, "/~", "", "", "http://example.org/~"});
+ checkRelativeURL("notspecial:/", "about:blank", {"notspecial", "", "", "", 0, "/", "", "", "notspecial:/"});
+ checkRelativeURL("notspecial:", "about:blank", {"notspecial", "", "", "", 0, "", "", "", "notspecial:"});
+ checkRelativeURL("notspecial:/", "http://host", {"notspecial", "", "", "", 0, "/", "", "", "notspecial:/"});
+ checkRelativeURL("notspecial:", "http://host", {"notspecial", "", "", "", 0, "", "", "", "notspecial:"});
+ checkRelativeURL("http:", "http://host", {"http", "", "", "host", 0, "/", "", "", "http://host/"});
// 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.
@@ -446,6 +456,21 @@
checkURLDifferences("file://notuser:notpassword@test/",
{"", "", "", "", 0, "", "", "", "file://notuser:notpassword@test/"},
{"file", "notuser", "notpassword", "test", 0, "/", "", "", "file://notuser:notpassword@test/"});
+ checkRelativeURLDifferences("http:/", "about:blank",
+ {"", "", "", "", 0, "", "", "", "http:/"},
+ {"http", "", "", "", 0, "/", "", "", "http:/"});
+ checkRelativeURLDifferences("http:", "about:blank",
+ {"http", "", "", "", 0, "", "", "", "http:"},
+ {"http", "", "", "", 0, "/", "", "", "http:/"});
+ checkRelativeURLDifferences("http:/", "http://host",
+ {"", "", "", "", 0, "", "", "", "http:/"},
+ {"http", "", "", "", 0, "/", "", "", "http:/"});
+ checkURLDifferences("http:/",
+ {"", "", "", "", 0, "", "", "", "http:/"},
+ {"http", "", "", "", 0, "/", "", "", "http:/"});
+ checkURLDifferences("http:",
+ {"http", "", "", "", 0, "", "", "", "http:"},
+ {"http", "", "", "", 0, "/", "", "", "http:/"});
// This behavior matches Chrome and Firefox, but not WebKit using URL::parse.
// The behavior of URL::parse is clearly wrong because reparsing file://path would make path the host.
@@ -572,6 +597,7 @@
TEST_F(URLParserTest, ParserFailures)
{
shouldFail(" ");
+ shouldFail(" \a ");
shouldFail("");
shouldFail("http://127.0.0.1:abc");
shouldFail("http://host:abc");
@@ -592,6 +618,13 @@
shouldFail("http://[www.example.com]/", "about:blank");
shouldFail("http://192.168.0.1 hello", "http://other.com/");
shouldFail("http://[example.com]", "http://other.com/");
+ shouldFail("i", "sc:sd");
+ shouldFail("i", "sc:sd/sd");
+ shouldFail("i");
+ shouldFail("asdf");
+ shouldFail("~");
+ shouldFail("~", "about:blank");
+ shouldFail("~~~");
}
// These are in the spec but not in the web platform tests.
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes
