Changeset 135040 in webkit

Timestamp:

Nov 17, 2012, 12:03:32 AM (13 years ago)

Author:

Martin Robinson

Message:

[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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/network/ResourceHandle.h (modified) (1 diff)
• platform/network/soup/ResourceHandleSoup.cpp (modified) (6 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-11-17  Martin Robinson  <mrobinson@igalia.com>
+
+        [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  <paroga@webkit.org>
 

trunk/Source/WebCore/platform/network/ResourceHandle.h

@@ -170 +170 @@
     void continueDidReceiveAuthenticationChallenge(const Credential& credentialFromPersistentStorage);
     void sendPendingRequest();
+    bool shouldUseCredentialStorage();
     static SoupSession* defaultSession();
     static uint64_t getSoupRequestInitiaingPageID(SoupRequest*);

trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp

@@ -328 +328 @@
 
     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());
@@ -913 +913 @@
 }
 
+bool ResourceHandle::shouldUseCredentialStorage()
+{
+    return (!client() || client()->shouldUseCredentialStorage(this)) && firstRequest().url().protocolIsInHTTPFamily();
+}
+
 void ResourceHandle::setHostAllowsAnyHTTPSCertificate(const String& host)
 {
@@ -960 +965 @@
     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());
@@ -971 +976 @@
 
     // 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
@@ -1002 +1007 @@
     // 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;
@@ -1033 +1038 @@
     }
 
-    // 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;
-    }
-#endif
+        if (credential.persistence() == CredentialPersistencePermanent) {
+            d->m_credentialDataToSaveInPersistentStore.credential = credential;
+            d->m_credentialDataToSaveInPersistentStore.challenge = challenge;
+        }
+#endif
+    }
 
     ASSERT(challenge.soupSession());