Skip to content

Commit

Permalink
Cherry-pick 269750@main (e68882f). https://bugs.webkit.org/show_bug.c…
Browse files Browse the repository at this point in the history
…gi?id=263615

    The URL move constructor doesn't invalidate the "moved-out" URL
    https://bugs.webkit.org/show_bug.cgi?id=263615

    Reviewed by Ryosuke Niwa.

    The URL move constructor doesn't invalidate the "moved-out" URL. This can lead
    WebKit code to do weird things.

    For example, URLKeepingBlobAlive contains a m_url data member and is often
    moved-out to pass to a lambda. The destructor of the "moved-out"
    URLKeepingBlobAlive then runs and calls `unregisterBlobURLHandleIfNecessary()`.
    `unregisterBlobURLHandleIfNecessary()` will try to use m_url after it's been
    moved out to see if the URL protocol is "blob". This  causes URL::protocolIs()
    to try to do out-of-bound access in the underlying String (since the URL is
    marked as valid, even though it's m_string was moved out and other data members
    that are indexes into that string were not reset). Luckily, String's operator[]
    just returns nil when doing an out of bounds access at the moment.

    * Source/WTF/wtf/URL.h:
    (WTF::URL::URL):
    (WTF::URL::operator=):
    * Tools/TestWebKitAPI/Tests/WTF/URL.cpp:
    (TestWebKitAPI::TEST_F):

    Canonical link: https://commits.webkit.org/269750@main
  • Loading branch information
cdumez authored and aperezdc committed Jan 25, 2024
1 parent 41f2585 commit 2544043
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 0 deletions.
41 changes: 41 additions & 0 deletions Source/WTF/wtf/URL.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,47 @@ class URL {
{
}

URL(const URL&) = default;
URL& operator=(const URL&) = default;

URL(URL&& other)
: m_string(WTFMove(other.m_string))
, m_isValid(other.m_isValid)
, m_protocolIsInHTTPFamily(other.m_protocolIsInHTTPFamily)
, m_hasOpaquePath(other.m_hasOpaquePath)
, m_portLength(other.m_portLength)
, m_schemeEnd(other.m_schemeEnd)
, m_userStart(other.m_userStart)
, m_userEnd(other.m_userEnd)
, m_passwordEnd(other.m_passwordEnd)
, m_hostEnd(other.m_hostEnd)
, m_pathAfterLastSlash(other.m_pathAfterLastSlash)
, m_pathEnd(other.m_pathEnd)
, m_queryEnd(other.m_queryEnd)
{
other.m_isValid = false;
}

URL& operator=(URL&& other)
{
m_string = WTFMove(other.m_string);
m_isValid = other.m_isValid;
other.m_isValid = false;
m_protocolIsInHTTPFamily = other.m_protocolIsInHTTPFamily;
m_hasOpaquePath = other.m_hasOpaquePath;
m_portLength = other.m_portLength;
m_schemeEnd = other.m_schemeEnd;
m_userStart = other.m_userStart;
m_userEnd = other.m_userEnd;
m_passwordEnd = other.m_passwordEnd;
m_hostEnd = other.m_hostEnd;
m_pathAfterLastSlash = other.m_pathAfterLastSlash;
m_pathEnd = other.m_pathEnd;
m_queryEnd = other.m_queryEnd;

return *this;
}

WTF_EXPORT_PRIVATE static URL fakeURLWithRelativePart(StringView);
WTF_EXPORT_PRIVATE static URL fileURLWithFileSystemPath(StringView);

Expand Down
17 changes: 17 additions & 0 deletions Tools/TestWebKitAPI/Tests/WTF/URL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -653,4 +653,21 @@ TEST_F(WTF_URL, IsolatedCopy)
EXPECT_EQ(url2Copy.string().impl(), originalStringImpl); // Should have adopted the StringImpl of url2.
}

TEST_F(WTF_URL, MoveInvalidatesURL)
{
URL url1 { "http://www.apple.com"_str };
EXPECT_TRUE(url1.isValid());
URL url2 { WTFMove(url1) };
EXPECT_TRUE(url2.isValid());
EXPECT_FALSE(url1.isValid());

URL url3 { "http://www.webkit.org"_str };
url3 = WTFMove(url2);
EXPECT_TRUE(url3.isValid());
EXPECT_FALSE(url2.isValid());

url3 = { };
EXPECT_FALSE(url3.isValid());
}

} // namespace TestWebKitAPI

0 comments on commit 2544043

Please sign in to comment.