Skip to content

Commit

Permalink
REGRESSION (URL parser): Relative URLs aren’t resolved correctly when…
Browse files Browse the repository at this point in the history
… the base URL is an applewebdata: URL

https://bugs.webkit.org/show_bug.cgi?id=165621

Reviewed by Dan Bernstein.

Source/WebCore:

Covered by new API tests.

* platform/URLParser.cpp:
(WebCore::URLParser::parse):
URLs with nonspecial schemes and no slash after the host get no slash as the path to maintain compatibility with all browsers.
This was proposed to the URL spec in whatwg/url#148
When such as URL is used as a base URL with a relative path, in order to maintain compatibility with URL::parse we need to prepend
a slash to the path.  For completeness I added tests with a relative path, a relative query, a relative fragment, and a relative empty string,
and because the fate of the spec is unclear in this case, I decided to maintain compatibility with URL::parse in all these cases.

Tools:

* TestWebKitAPI/Tests/WebCore/URLParser.cpp:
(TestWebKitAPI::TEST_F):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@209572 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
achristensen@apple.com committed Dec 8, 2016
1 parent 2ebbdf2 commit ebbd330
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 0 deletions.
17 changes: 17 additions & 0 deletions Source/WebCore/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
2016-12-08 Alex Christensen <achristensen@webkit.org>

REGRESSION (URL parser): Relative URLs aren’t resolved correctly when the base URL is an applewebdata: URL
https://bugs.webkit.org/show_bug.cgi?id=165621

Reviewed by Dan Bernstein.

Covered by new API tests.

* platform/URLParser.cpp:
(WebCore::URLParser::parse):
URLs with nonspecial schemes and no slash after the host get no slash as the path to maintain compatibility with all browsers.
This was proposed to the URL spec in https://github.com/whatwg/url/issues/148
When such as URL is used as a base URL with a relative path, in order to maintain compatibility with URL::parse we need to prepend
a slash to the path. For completeness I added tests with a relative path, a relative query, a relative fragment, and a relative empty string,
and because the fate of the spec is unclear in this case, I decided to maintain compatibility with URL::parse in all these cases.

2016-12-06 Filip Pizlo <fpizlo@apple.com>

Concurrent GC should be stable enough to land enabled on X86_64
Expand Down
4 changes: 4 additions & 0 deletions Source/WebCore/platform/URLParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1355,6 +1355,10 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
break;
default:
copyURLPartsUntil(base, URLPart::PathAfterLastSlash, c, isUTF8Encoding);
if (currentPosition(c) && parsedDataView(currentPosition(c) - 1) != '/') {
appendToASCIIBuffer('/');
m_url.m_pathAfterLastSlash = currentPosition(c);
}
state = State::Path;
break;
}
Expand Down
10 changes: 10 additions & 0 deletions Tools/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
2016-12-08 Alex Christensen <achristensen@webkit.org>

REGRESSION (URL parser): Relative URLs aren’t resolved correctly when the base URL is an applewebdata: URL
https://bugs.webkit.org/show_bug.cgi?id=165621

Reviewed by Dan Bernstein.

* TestWebKitAPI/Tests/WebCore/URLParser.cpp:
(TestWebKitAPI::TEST_F):

2016-12-06 Filip Pizlo <fpizlo@apple.com>

Concurrent GC should be stable enough to land enabled
Expand Down
5 changes: 5 additions & 0 deletions Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,11 @@ TEST_F(URLParserTest, ParseRelative)
{"foo", "", "", "", 0, "", "", "", "foo://"},
{"foo", "", "", "", 0, "//", "", "", "foo://"});
checkRelativeURL(utf16String(u""), "http://example.org/foo/bar", {"http", "", "", "example.org", 0, "/foo/bar", "", "%CE%B2", "http://example.org/foo/bar#%CE%B2"});
checkRelativeURL("index.html", "applewebdata://Host/", {"applewebdata", "", "", "Host", 0, "/index.html", "", "", "applewebdata://Host/index.html"});
checkRelativeURL("index.html", "applewebdata://Host", {"applewebdata", "", "", "Host", 0, "/index.html", "", "", "applewebdata://Host/index.html"});
checkRelativeURL("", "applewebdata://Host", {"applewebdata", "", "", "Host", 0, "", "", "", "applewebdata://Host"});
checkRelativeURL("?query", "applewebdata://Host", {"applewebdata", "", "", "Host", 0, "", "query", "", "applewebdata://Host?query"});
checkRelativeURL("#fragment", "applewebdata://Host", {"applewebdata", "", "", "Host", 0, "", "", "fragment", "applewebdata://Host#fragment"});

// 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.
Expand Down

0 comments on commit ebbd330

Please sign in to comment.