Skip to content

Commit

Permalink
Bug 269902 - [Curl] Add null check in WebSocketTaskCurl/NetworkDataTa…
Browse files Browse the repository at this point in the history 
…sk/NetworkSession

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

Reviewed by Fujii Hironori.

m_channel.session() referenced in WebSocketTaskCurl is defined as WeakPtr<NetworkSession>.
Therefore, nullptr may be returned depending on the timing, so check for null before using them.

Also, NetworkSession::networkStorageSession() referenced by NetworkDataTaskCurl and NetworkSession
can return nullptr, so do a null check before using them.

* Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp:
* Source/WebKit/NetworkProcess/curl/NetworkSessionCurl.cpp:
* Source/WebKit/NetworkProcess/curl/WebSocketTaskCurl.cpp:

Canonical link: https://commits.webkit.org/275304@main
  • Loading branch information
@kshukuwa @fujii
kshukuwa authored and fujii committed Feb 26, 2024
1 parent fe3d1a8 commit 55dbfac
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 43 deletions.
76 changes: 43 additions & 33 deletions Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,18 @@ NetworkDataTaskCurl::NetworkDataTaskCurl(NetworkSession& session, NetworkDataTas
, m_sourceOrigin(parameters.sourceOrigin)
{
auto request = parameters.request;
if (request.url().protocolIsInHTTPFamily()) {
if (m_storedCredentialsPolicy == StoredCredentialsPolicy::Use) {
auto url = request.url();
m_user = url.user();
m_password = url.password();
request.removeCredentials();

auto url = request.url();
if (m_storedCredentialsPolicy == StoredCredentialsPolicy::Use && url.protocolIsInHTTPFamily()) {
m_user = url.user();
m_password = url.password();
request.removeCredentials();
url = request.url();

if (auto* storageSession = m_session->networkStorageSession()) {
if (m_user.isEmpty() && m_password.isEmpty())
m_initialCredential = m_session->networkStorageSession()->credentialStorage().get(m_partition, request.url());
m_initialCredential = storageSession->credentialStorage().get(m_partition, url);
else
m_session->networkStorageSession()->credentialStorage().set(m_partition, Credential(m_user, m_password, CredentialPersistence::None), request.url());
storageSession->credentialStorage().set(m_partition, Credential(m_user, m_password, CredentialPersistence::None), url);
}
}

Expand Down Expand Up @@ -380,10 +381,12 @@ void NetworkDataTaskCurl::willPerformHTTPRedirection()
// Only consider applying authentication credentials if this is actually a redirect and the redirect
// URL didn't include credentials of its own.
if (m_user.isEmpty() && m_password.isEmpty()) {
auto credential = m_session->networkStorageSession()->credentialStorage().get(m_partition, request.url());
if (!credential.isEmpty()) {
m_initialCredential = credential;
didChangeCredential = true;
if (auto* storageSession = m_session->networkStorageSession()) {
auto credential = storageSession->credentialStorage().get(m_partition, request.url());
if (!credential.isEmpty()) {
m_initialCredential = credential;
didChangeCredential = true;
}
}
}
}
Expand Down Expand Up @@ -428,19 +431,22 @@ void NetworkDataTaskCurl::tryHttpAuthentication(AuthenticationChallenge&& challe
// The stored credential wasn't accepted, stop using it. There is a race condition
// here, since a different credential might have already been stored by another
// NetworkDataTask, but the observable effect should be very minor, if any.
m_session->networkStorageSession()->credentialStorage().remove(m_partition, challenge.protectionSpace());
if (auto* storageSession = m_session->networkStorageSession())
storageSession->credentialStorage().remove(m_partition, challenge.protectionSpace());
}

if (!challenge.previousFailureCount()) {
auto credential = m_session->networkStorageSession()->credentialStorage().get(m_partition, challenge.protectionSpace());
if (!credential.isEmpty() && credential != m_initialCredential) {
ASSERT(credential.persistence() == CredentialPersistence::None);
if (challenge.failureResponse().isUnauthorized()) {
// Store the credential back, possibly adding it as a default for this directory.
m_session->networkStorageSession()->credentialStorage().set(m_partition, credential, challenge.protectionSpace(), challenge.failureResponse().url());
if (auto* storageSession = m_session->networkStorageSession()) {
auto credential = storageSession->credentialStorage().get(m_partition, challenge.protectionSpace());
if (!credential.isEmpty() && credential != m_initialCredential) {
ASSERT(credential.persistence() == CredentialPersistence::None);
if (challenge.failureResponse().isUnauthorized()) {
// Store the credential back, possibly adding it as a default for this directory.
storageSession->credentialStorage().set(m_partition, credential, challenge.protectionSpace(), challenge.failureResponse().url());
}
restartWithCredential(challenge.protectionSpace(), credential);
return;
}
restartWithCredential(challenge.protectionSpace(), credential);
return;
}
}
}
Expand All @@ -456,9 +462,9 @@ void NetworkDataTaskCurl::tryHttpAuthentication(AuthenticationChallenge&& challe
}

if (disposition == AuthenticationChallengeDisposition::UseCredential && !credential.isEmpty()) {
if (m_storedCredentialsPolicy == StoredCredentialsPolicy::Use) {
if (credential.persistence() == CredentialPersistence::ForSession || credential.persistence() == CredentialPersistence::Permanent)
m_session->networkStorageSession()->credentialStorage().set(m_partition, credential, challenge.protectionSpace(), challenge.failureResponse().url());
if (m_storedCredentialsPolicy == StoredCredentialsPolicy::Use && (credential.persistence() == CredentialPersistence::ForSession || credential.persistence() == CredentialPersistence::Permanent)) {
if (auto* storageSession = m_session->networkStorageSession())
storageSession->credentialStorage().set(m_partition, credential, challenge.protectionSpace(), challenge.failureResponse().url());
}

restartWithCredential(challenge.protectionSpace(), credential);
Expand Down Expand Up @@ -533,20 +539,24 @@ void NetworkDataTaskCurl::restartWithCredential(const ProtectionSpace& protectio

void NetworkDataTaskCurl::appendCookieHeader(WebCore::ResourceRequest& request)
{
auto includeSecureCookies = request.url().protocolIs("https"_s) ? IncludeSecureCookies::Yes : IncludeSecureCookies::No;
auto cookieHeaderField = m_session->networkStorageSession()->cookieRequestHeaderFieldValue(request.firstPartyForCookies(), WebCore::SameSiteInfo::create(request), request.url(), std::nullopt, std::nullopt, includeSecureCookies, ApplyTrackingPrevention::Yes, WebCore::ShouldRelaxThirdPartyCookieBlocking::No).first;
if (!cookieHeaderField.isEmpty())
request.addHTTPHeaderField(HTTPHeaderName::Cookie, cookieHeaderField);
if (auto* storageSession = m_session->networkStorageSession()) {
auto includeSecureCookies = request.url().protocolIs("https"_s) ? IncludeSecureCookies::Yes : IncludeSecureCookies::No;
auto cookieHeaderField = storageSession->cookieRequestHeaderFieldValue(request.firstPartyForCookies(), WebCore::SameSiteInfo::create(request), request.url(), std::nullopt, std::nullopt, includeSecureCookies, ApplyTrackingPrevention::Yes, WebCore::ShouldRelaxThirdPartyCookieBlocking::No).first;
if (!cookieHeaderField.isEmpty())
request.addHTTPHeaderField(HTTPHeaderName::Cookie, cookieHeaderField);
}
}

void NetworkDataTaskCurl::handleCookieHeaders(const WebCore::ResourceRequest& request, const CurlResponse& response)
{
static constexpr auto setCookieHeader = "set-cookie: "_s;

for (auto header : response.headers) {
if (header.startsWithIgnoringASCIICase(setCookieHeader)) {
String setCookieString = header.right(header.length() - setCookieHeader.length());
m_session->networkStorageSession()->setCookiesFromHTTPResponse(request.firstPartyForCookies(), response.url, setCookieString);
if (auto* storageSession = m_session->networkStorageSession()) {
for (auto header : response.headers) {
if (header.startsWithIgnoringASCIICase(setCookieHeader)) {
String setCookieString = header.right(header.length() - setCookieHeader.length());
storageSession->setCookiesFromHTTPResponse(request.firstPartyForCookies(), response.url, setCookieString);
}
}
}
}
Expand Down
11 changes: 7 additions & 4 deletions Source/WebKit/NetworkProcess/curl/NetworkSessionCurl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@ using namespace WebCore;
NetworkSessionCurl::NetworkSessionCurl(NetworkProcess& networkProcess, const NetworkSessionCreationParameters& parameters)
: NetworkSession(networkProcess, parameters)
{
if (!parameters.cookiePersistentStorageFile.isEmpty())
networkStorageSession()->setCookieDatabase(makeUniqueRef<CookieJarDB>(parameters.cookiePersistentStorageFile));
networkStorageSession()->setProxySettings(parameters.proxySettings);
if (auto* storageSession = networkStorageSession()) {
if (!parameters.cookiePersistentStorageFile.isEmpty())
storageSession->setCookieDatabase(makeUniqueRef<CookieJarDB>(parameters.cookiePersistentStorageFile));
storageSession->setProxySettings(parameters.proxySettings);
}

m_resourceLoadStatisticsDirectory = parameters.resourceLoadStatisticsParameters.directory;
m_shouldIncludeLocalhostInResourceLoadStatistics = parameters.resourceLoadStatisticsParameters.shouldIncludeLocalhost ? ShouldIncludeLocalhost::Yes : ShouldIncludeLocalhost::No;
Expand All @@ -60,7 +62,8 @@ NetworkSessionCurl::~NetworkSessionCurl()

void NetworkSessionCurl::clearAlternativeServices(WallTime)
{
networkStorageSession()->clearAlternativeServices();
if (auto* storageSession = networkStorageSession())
storageSession->clearAlternativeServices();
}

std::unique_ptr<WebSocketTask> NetworkSessionCurl::createWebSocketTask(WebPageProxyIdentifier webPageProxyID, std::optional<FrameIdentifier>, std::optional<PageIdentifier>, NetworkSocketChannel& channel, const WebCore::ResourceRequest& request, const String& protocol, const WebCore::ClientOrigin& clientOrigin, bool, bool, OptionSet<WebCore::AdvancedPrivacyProtections>, ShouldRelaxThirdPartyCookieBlocking, StoredCredentialsPolicy)
Expand Down
16 changes: 10 additions & 6 deletions Source/WebKit/NetworkProcess/curl/WebSocketTaskCurl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,12 @@ void WebSocketTask::didOpen(WebCore::CurlStreamID)
CString cookieHeader;

if (m_request.allowCookies()) {
auto includeSecureCookies = m_request.url().protocolIs("wss"_s) ? WebCore::IncludeSecureCookies::Yes : WebCore::IncludeSecureCookies::No;
auto cookieHeaderField = m_channel.session()->networkStorageSession()->cookieRequestHeaderFieldValue(m_request.firstPartyForCookies(), WebCore::SameSiteInfo::create(m_request), m_request.url(), std::nullopt, std::nullopt, includeSecureCookies, WebCore::ApplyTrackingPrevention::Yes, WebCore::ShouldRelaxThirdPartyCookieBlocking::No).first;
if (!cookieHeaderField.isEmpty())
cookieHeader = makeString("Cookie: "_s, cookieHeaderField, "\r\n"_s).utf8();
if (auto* storageSession = networkSession() ? networkSession()->networkStorageSession() : nullptr) {
auto includeSecureCookies = m_request.url().protocolIs("wss"_s) ? WebCore::IncludeSecureCookies::Yes : WebCore::IncludeSecureCookies::No;
auto cookieHeaderField = storageSession->cookieRequestHeaderFieldValue(m_request.firstPartyForCookies(), WebCore::SameSiteInfo::create(m_request), m_request.url(), std::nullopt, std::nullopt, includeSecureCookies, WebCore::ApplyTrackingPrevention::Yes, WebCore::ShouldRelaxThirdPartyCookieBlocking::No).first;
if (!cookieHeaderField.isEmpty())
cookieHeader = makeString("Cookie: "_s, cookieHeaderField, "\r\n"_s).utf8();
}
}

auto originalMessage = m_handshake->clientHandshakeMessage();
Expand Down Expand Up @@ -300,8 +302,10 @@ Expected<bool, String> WebSocketTask::validateOpeningHandshake()
}

auto serverSetCookie = m_handshake->serverSetCookie();
if (!serverSetCookie.isEmpty())
m_channel.session()->networkStorageSession()->setCookiesFromHTTPResponse(m_request.firstPartyForCookies(), m_request.url(), serverSetCookie);
if (!serverSetCookie.isEmpty()) {
if (auto* storageSession = networkSession() ? networkSession()->networkStorageSession() : nullptr)
storageSession->setCookiesFromHTTPResponse(m_request.firstPartyForCookies(), m_request.url(), serverSetCookie);
}

m_state = State::Opened;
m_didCompleteOpeningHandshake = true;
Expand Down

0 comments on commit 55dbfac

Please sign in to comment.