From 06319bc9f994e9ad0c4980fe60d162adb95ceff9 Mon Sep 17 00:00:00 2001 From: "mrobinson@webkit.org" Date: Sat, 17 Nov 2012 08:03:32 +0000 Subject: [PATCH] [Soup] CredentialStorage should only be used for HTTP-family requests https://bugs.webkit.org/show_bug.cgi?id=102582 Reviewed by Gustavo Noronha Silva. Do not use CredentialStorage when handling non-HTTP family requests. CredentialStorage only expects to handle requests in the HTTP family. No new tests. This is covered by existing tests. * platform/network/ResourceHandle.h: (ResourceHandle): Add a shouldUseCredentialStorage helper to ResourceHandle. This helper returns false when firstRequest() is a non-HTTP family request. * platform/network/soup/ResourceHandleSoup.cpp: (WebCore::applyAuthenticationToRequest): Use the new helper. (WebCore::createSoupRequestAndMessageForHandle): Ditto. (WebCore::ResourceHandle::start): Ditto. (WebCore::ResourceHandle::shouldUseCredentialStorage): Ditto. (WebCore::ResourceHandle::didReceiveAuthenticationChallenge): Ditto. (WebCore::ResourceHandle::receivedCredential): Ditto. git-svn-id: https://svn.webkit.org/repository/webkit/trunk@135040 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebCore/ChangeLog | 23 ++++++++++++++ Source/WebCore/platform/network/ResourceHandle.h | 1 + .../platform/network/soup/ResourceHandleSoup.cpp | 37 +++++++++++++--------- 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 5a08561..e45a68d 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,26 @@ +2012-11-17 Martin Robinson + + [Soup] CredentialStorage should only be used for HTTP-family requests + https://bugs.webkit.org/show_bug.cgi?id=102582 + + Reviewed by Gustavo Noronha Silva. + + Do not use CredentialStorage when handling non-HTTP family requests. CredentialStorage + only expects to handle requests in the HTTP family. + + No new tests. This is covered by existing tests. + + * platform/network/ResourceHandle.h: + (ResourceHandle): Add a shouldUseCredentialStorage helper to ResourceHandle. This + helper returns false when firstRequest() is a non-HTTP family request. + * platform/network/soup/ResourceHandleSoup.cpp: + (WebCore::applyAuthenticationToRequest): Use the new helper. + (WebCore::createSoupRequestAndMessageForHandle): Ditto. + (WebCore::ResourceHandle::start): Ditto. + (WebCore::ResourceHandle::shouldUseCredentialStorage): Ditto. + (WebCore::ResourceHandle::didReceiveAuthenticationChallenge): Ditto. + (WebCore::ResourceHandle::receivedCredential): Ditto. + 2012-11-16 Patrick Gansterer Build fix for !USE(ACCELERATED_COMPOSITING) after r135029. diff --git a/Source/WebCore/platform/network/ResourceHandle.h b/Source/WebCore/platform/network/ResourceHandle.h index e818d8d..a1c50f5 100644 --- a/Source/WebCore/platform/network/ResourceHandle.h +++ b/Source/WebCore/platform/network/ResourceHandle.h @@ -169,6 +169,7 @@ public: #if USE(SOUP) void continueDidReceiveAuthenticationChallenge(const Credential& credentialFromPersistentStorage); void sendPendingRequest(); + bool shouldUseCredentialStorage(); static SoupSession* defaultSession(); static uint64_t getSoupRequestInitiaingPageID(SoupRequest*); static void setHostAllowsAnyHTTPSCertificate(const String&); diff --git a/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp b/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp index a229098..a5ff90e 100644 --- a/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp +++ b/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp @@ -327,7 +327,7 @@ static void applyAuthenticationToRequest(ResourceHandle* handle, bool redirect) String password = d->m_pass; ResourceRequest& request = d->m_firstRequest; - if (!handle->client() || handle->client()->shouldUseCredentialStorage(handle)) { + if (handle->shouldUseCredentialStorage()) { if (d->m_user.isEmpty() && d->m_pass.isEmpty()) d->m_initialCredential = CredentialStorage::get(request.url()); else if (!redirect) { @@ -912,6 +912,11 @@ void ResourceHandle::cancel() g_cancellable_cancel(d->m_cancellable.get()); } +bool ResourceHandle::shouldUseCredentialStorage() +{ + return (!client() || client()->shouldUseCredentialStorage(this)) && firstRequest().url().protocolIsInHTTPFamily(); +} + void ResourceHandle::setHostAllowsAnyHTTPSCertificate(const String& host) { allowsAnyHTTPSCertificateHosts().add(host.lower()); @@ -959,10 +964,10 @@ void ResourceHandle::didReceiveAuthenticationChallenge(const AuthenticationChall { ASSERT(d->m_currentWebChallenge.isNull()); - bool shouldUseCredentialStorage = !client() || client()->shouldUseCredentialStorage(this); + bool useCredentialStorage = shouldUseCredentialStorage(); if (!d->m_user.isNull() && !d->m_pass.isNull()) { Credential credential = Credential(d->m_user, d->m_pass, CredentialPersistenceForSession); - if (shouldUseCredentialStorage) + if (useCredentialStorage) CredentialStorage::set(credential, challenge.protectionSpace(), challenge.failureResponse().url()); soup_auth_authenticate(challenge.soupAuth(), credential.user().utf8().data(), credential.password().utf8().data()); @@ -970,7 +975,7 @@ void ResourceHandle::didReceiveAuthenticationChallenge(const AuthenticationChall } // FIXME: Per the specification, the user shouldn't be asked for credentials if there were incorrect ones provided explicitly. - if (shouldUseCredentialStorage) { + if (useCredentialStorage) { if (!d->m_initialCredential.isEmpty() || challenge.previousFailureCount()) { // 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 @@ -1001,7 +1006,7 @@ void ResourceHandle::didReceiveAuthenticationChallenge(const AuthenticationChall // of all request latency, versus a one-time latency for the small subset of requests that // use HTTP authentication. In the end, this doesn't matter much, because persistent credentials // will become session credentials after the first use. - if (shouldUseCredentialStorage) { + if (useCredentialStorage) { credentialBackingStore().credentialForChallenge(challenge, getCredentialFromPersistentStoreCallback, this); return; } @@ -1032,19 +1037,21 @@ void ResourceHandle::receivedCredential(const AuthenticationChallenge& challenge return; } - // Eventually we will manage per-session credentials only internally or use some newly-exposed API from libsoup, - // because once we authenticate via libsoup, there is no way to ignore it for a particular request. Right now, - // we place the credentials in the store even though libsoup will never fire the authenticate signal again for - // this protection space. - if (credential.persistence() == CredentialPersistenceForSession || credential.persistence() == CredentialPersistencePermanent) - CredentialStorage::set(credential, challenge.protectionSpace(), challenge.failureResponse().url()); + if (shouldUseCredentialStorage()) { + // Eventually we will manage per-session credentials only internally or use some newly-exposed API from libsoup, + // because once we authenticate via libsoup, there is no way to ignore it for a particular request. Right now, + // we place the credentials in the store even though libsoup will never fire the authenticate signal again for + // this protection space. + if (credential.persistence() == CredentialPersistenceForSession || credential.persistence() == CredentialPersistencePermanent) + CredentialStorage::set(credential, challenge.protectionSpace(), challenge.failureResponse().url()); #if PLATFORM(GTK) - if (credential.persistence() == CredentialPersistencePermanent) { - d->m_credentialDataToSaveInPersistentStore.credential = credential; - d->m_credentialDataToSaveInPersistentStore.challenge = challenge; - } + if (credential.persistence() == CredentialPersistencePermanent) { + d->m_credentialDataToSaveInPersistentStore.credential = credential; + d->m_credentialDataToSaveInPersistentStore.challenge = challenge; + } #endif + } ASSERT(challenge.soupSession()); ASSERT(challenge.soupMessage()); -- 1.8.3.1