Attachment 105654 Details for Bug 65330 – Remove didReceiveAuthenticationChallenge() #4

[patch] Remove didReceiveAuthenticationChallenge() #4

rc5.txt (text/plain), 18.03 KB, created by Nate Chapin on 2011-08-30 11:09:39 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Nate Chapin

Created: 2011-08-30 11:09:39 PDT

Size: 18.03 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 94090)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,44 @@
>+2011-08-30  Nate Chapin  <japhet@chromium.org>
>+
>+        Remove didReceiveAuthenticationChallenge() from SubresourceLoaderClient.
>+        Instead, add a load-specific policy for showing the user authentication
>+        challenge down to ResourceLoaderOptions and enforce it in ResourceLoader.
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No new tests, refactor only.
>+
>+        * loader/DocumentThreadableLoader.cpp:
>+        * loader/DocumentThreadableLoader.h:
>+        * loader/MainResourceLoader.cpp:
>+        * loader/NetscapePlugInStreamLoader.cpp:
>+        * loader/ResourceLoadScheduler.h:
>+        * loader/ResourceLoader.cpp:
>+        (WebCore::ResourceLoader::didReceiveAuthenticationChallenge):
>+           For resource types that always send a challenge to the embedder,
>+           this patch doesn't change anything. For those that don't, we will
>+           always try to continue without credentials when they are forbidden
>+           and the platform supports it.
>+           When continuing without credentials was initially implemented in
>+           DocumentThreadableLoader, we sent the ThreadableLoaderClient a didFail(),
>+           then canceled the SubresourceLoader. This was necessary because of the
>+           quirks of ThreadableLoader cancellation (we sever the client/loader connections
>+           before the load actually cancels), but a simple didFail() should suffice at
>+           the ResourceLoader layer.
>+        * loader/ResourceLoaderOptions.h:
>+        * loader/SubresourceLoader.cpp:
>+        * loader/SubresourceLoader.h:
>+        * loader/SubresourceLoaderClient.h:
>+        * loader/cache/CachedResource.cpp:
>+        * loader/cache/CachedResourceLoader.cpp:
>+        * loader/cache/CachedResourceLoader.h:
>+        * loader/icon/IconLoader.cpp: The ResourceLoader implementation of
>+            didReceiveAuthenticationChallege means that IconLoader will now
>+            try to continue with credentials on platforms that support it,
>+            rather than just canceling outright. We still will never prompt
>+            for authentication for icons.
>+        * loader/icon/IconLoader.h:
>+
> 2011-08-30  Antti Koivisto  <antti@apple.com>
> 
>         querySelector/querySelectorAll should use selector checker fast path 
>Index: Source/WebCore/loader/ResourceLoaderOptions.h
>===================================================================
>--- Source/WebCore/loader/ResourceLoaderOptions.h	(revision 94022)
>+++ Source/WebCore/loader/ResourceLoaderOptions.h	(working copy)
>@@ -49,14 +49,26 @@
>     BufferData,
>     DoNotBufferData
> };
>+    
>+enum ClientCrossOriginCredentialPolicy {
>+    AllowClientCrossOriginCredentials,
>+    BlockClientCrossOriginCredentials
>+};
> 
> struct ResourceLoaderOptions {
>-    ResourceLoaderOptions() : sendLoadCallbacks(DoNotSendCallbacks), sniffContent(DoNotSniffContent), shouldBufferData(BufferData), allowCredentials(DoNotAllowStoredCredentials) { }
>-    ResourceLoaderOptions(SendCallbackPolicy sendLoadCallbacksArg, ContentSniffingPolicy sniffContentArg, DataBufferingPolicy shouldBufferDataArg, StoredCredentials allowCredentialsArg) : sendLoadCallbacks(sendLoadCallbacksArg), sniffContent(sniffContentArg), shouldBufferData(shouldBufferDataArg), allowCredentials(allowCredentialsArg) { }
>+    ResourceLoaderOptions() : sendLoadCallbacks(DoNotSendCallbacks), sniffContent(DoNotSniffContent), shouldBufferData(BufferData), allowCredentials(DoNotAllowStoredCredentials), crossOriginCredentialPolicy(BlockClientCrossOriginCredentials) { }
>+    ResourceLoaderOptions(SendCallbackPolicy sendLoadCallbacksArg, ContentSniffingPolicy sniffContentArg, DataBufferingPolicy shouldBufferDataArg, StoredCredentials allowCredentialsArg, ClientCrossOriginCredentialPolicy crossOriginCredentialPolicyArg)
>+        : sendLoadCallbacks(sendLoadCallbacksArg)
>+        , sniffContent(sniffContentArg)
>+        , shouldBufferData(shouldBufferDataArg)
>+        , allowCredentials(allowCredentialsArg)
>+        , crossOriginCredentialPolicy(crossOriginCredentialPolicyArg)
>+    { }
>     SendCallbackPolicy sendLoadCallbacks;
>     ContentSniffingPolicy sniffContent;
>     DataBufferingPolicy shouldBufferData;
>     StoredCredentials allowCredentials; // Whether HTTP credentials and cookies are sent with the request.
>+    ClientCrossOriginCredentialPolicy crossOriginCredentialPolicy; // Whether we will ask the client for credentials (if we allow credentials at all).
> };
> 
> } // namespace WebCore    
>Index: Source/WebCore/loader/cache/CachedResourceLoader.cpp
>===================================================================
>--- Source/WebCore/loader/cache/CachedResourceLoader.cpp	(revision 94022)
>+++ Source/WebCore/loader/cache/CachedResourceLoader.cpp	(working copy)
>@@ -171,7 +171,7 @@
>     if (!inCache)
>         userSheet->setInCache(true);
> 
>-    userSheet->setResourceLoaderOptions(ResourceLoaderOptions(DoNotSendCallbacks, SniffContent, BufferData, AllowStoredCredentials));
>+    userSheet->setResourceLoaderOptions(ResourceLoaderOptions(DoNotSendCallbacks, SniffContent, BufferData, AllowStoredCredentials, AllowClientCrossOriginCredentials));
>     userSheet->load(this, /*incremental*/ false, SkipSecurityCheck);
> 
>     if (!inCache)
>Index: Source/WebCore/loader/cache/CachedResourceLoader.h
>===================================================================
>--- Source/WebCore/loader/cache/CachedResourceLoader.h	(revision 94022)
>+++ Source/WebCore/loader/cache/CachedResourceLoader.h	(working copy)
>@@ -109,7 +109,7 @@
> private:
>     // FIXME: The default value for ResourceLoaderOptions will always be used currently.
>     // It is plumbed for http://bugs.webkit.org/show_bug.cgi?id=61225 .
>-    CachedResource* requestResource(CachedResource::Type, ResourceRequest&, const String& charset, ResourceLoadPriority = ResourceLoadPriorityUnresolved, bool isPreload = false, const ResourceLoaderOptions& = ResourceLoaderOptions(SendCallbacks, SniffContent, BufferData, AllowStoredCredentials));
>+    CachedResource* requestResource(CachedResource::Type, ResourceRequest&, const String& charset, ResourceLoadPriority = ResourceLoadPriorityUnresolved, bool isPreload = false, const ResourceLoaderOptions& = ResourceLoaderOptions(SendCallbacks, SniffContent, BufferData, AllowStoredCredentials, AllowClientCrossOriginCredentials));
>     CachedResource* revalidateResource(CachedResource*, ResourceLoadPriority);
>     CachedResource* loadResource(CachedResource::Type, ResourceRequest&, const String& charset, ResourceLoadPriority, const ResourceLoaderOptions&);
>     void requestPreload(CachedResource::Type, ResourceRequest&, const String& charset);
>Index: Source/WebCore/loader/cache/CachedResource.cpp
>===================================================================
>--- Source/WebCore/loader/cache/CachedResource.cpp	(revision 94022)
>+++ Source/WebCore/loader/cache/CachedResource.cpp	(working copy)
>@@ -96,7 +96,7 @@
>     , m_loading(false)
>     , m_type(type)
>     , m_status(Pending)
>-    , m_options(SendCallbacks, SniffContent, BufferData, AllowStoredCredentials)
>+    , m_options(SendCallbacks, SniffContent, BufferData, AllowStoredCredentials, AllowClientCrossOriginCredentials)
> #ifndef NDEBUG
>     , m_deleted(false)
>     , m_lruIndex(0)
>Index: Source/WebCore/loader/NetscapePlugInStreamLoader.cpp
>===================================================================
>--- Source/WebCore/loader/NetscapePlugInStreamLoader.cpp	(revision 94022)
>+++ Source/WebCore/loader/NetscapePlugInStreamLoader.cpp	(working copy)
>@@ -36,7 +36,7 @@
> namespace WebCore {
> 
> NetscapePlugInStreamLoader::NetscapePlugInStreamLoader(Frame* frame, NetscapePlugInStreamLoaderClient* client)
>-    : ResourceLoader(frame, ResourceLoaderOptions(SendCallbacks, SniffContent, DoNotBufferData, AllowStoredCredentials))
>+    : ResourceLoader(frame, ResourceLoaderOptions(SendCallbacks, SniffContent, DoNotBufferData, AllowStoredCredentials, AllowClientCrossOriginCredentials))
>     , m_client(client)
> {
> }
>Index: Source/WebCore/loader/SubresourceLoaderClient.h
>===================================================================
>--- Source/WebCore/loader/SubresourceLoaderClient.h	(revision 94022)
>+++ Source/WebCore/loader/SubresourceLoaderClient.h	(working copy)
>@@ -50,8 +50,6 @@
>     virtual void didReceiveCachedMetadata(SubresourceLoader*, const char*, int /*dataLength*/) { }
>     virtual void didFinishLoading(SubresourceLoader*, double /*finishTime*/) { }
>     virtual void didFail(SubresourceLoader*, const ResourceError&) { }
>-    
>-    virtual void didReceiveAuthenticationChallenge(SubresourceLoader*, const AuthenticationChallenge&) { }
> };
> 
> } // namespace WebCore
>Index: Source/WebCore/loader/icon/IconLoader.cpp
>===================================================================
>--- Source/WebCore/loader/icon/IconLoader.cpp	(revision 94022)
>+++ Source/WebCore/loader/icon/IconLoader.cpp	(working copy)
>@@ -72,7 +72,7 @@
>     ResourceRequest resourceRequest(m_frame->loader()->icon()->url());
>     resourceRequest.setPriority(ResourceLoadPriorityLow);
> 
>-    RefPtr<SubresourceLoader> loader = resourceLoadScheduler()->scheduleSubresourceLoad(m_frame, this, resourceRequest);
>+    RefPtr<SubresourceLoader> loader = resourceLoadScheduler()->scheduleSubresourceLoad(m_frame, this, resourceRequest, ResourceLoadPriorityLow, DoSecurityCheck, ResourceLoaderOptions(SendCallbacks, SniffContent, BufferData, DoNotAllowStoredCredentials, BlockClientCrossOriginCredentials));
>     if (!loader)
>         LOG_ERROR("Failed to start load for icon at url %s", m_frame->loader()->icon()->url().string().ascii().data());
> 
>@@ -124,13 +124,6 @@
>     }
> }
> 
>-void IconLoader::didReceiveAuthenticationChallenge(SubresourceLoader*, const AuthenticationChallenge&)
>-{
>-    // We don't ever want to prompt for authentication just for a site icon, so
>-    // implement this method to cancel the resource load
>-    m_resourceLoader->cancel();
>-}
>-
> void IconLoader::didFinishLoading(SubresourceLoader* resourceLoader, double)
> {
>     LOG(IconDatabase, "IconLoader::didFinishLoading() - Loader %p", resourceLoader);
>Index: Source/WebCore/loader/icon/IconLoader.h
>===================================================================
>--- Source/WebCore/loader/icon/IconLoader.h	(revision 94022)
>+++ Source/WebCore/loader/icon/IconLoader.h	(working copy)
>@@ -54,8 +54,6 @@
>     virtual void didFinishLoading(SubresourceLoader*, double);
>     virtual void didFail(SubresourceLoader*, const ResourceError&);
> 
>-    virtual void didReceiveAuthenticationChallenge(SubresourceLoader*, const AuthenticationChallenge&);
>-
>     void finishLoading(const KURL&, PassRefPtr<SharedBuffer> data);
>     void clearLoadingState();
> 
>Index: Source/WebCore/loader/ResourceLoader.cpp
>===================================================================
>--- Source/WebCore/loader/ResourceLoader.cpp	(revision 94022)
>+++ Source/WebCore/loader/ResourceLoader.cpp	(working copy)
>@@ -42,6 +42,7 @@
> #include "ResourceError.h"
> #include "ResourceHandle.h"
> #include "ResourceLoadScheduler.h"
>+#include "SecurityOrigin.h"
> #include "Settings.h"
> #include "SharedBuffer.h"
> 
>@@ -527,10 +528,26 @@
> 
> void ResourceLoader::didReceiveAuthenticationChallenge(const AuthenticationChallenge& challenge)
> {
>+    ASSERT(handle()->hasAuthenticationChallenge());
>     // Protect this in this delegate method since the additional processing can do
>     // anything including possibly derefing this; one example of this is Radar 3266216.
>     RefPtr<ResourceLoader> protector(this);
>-    frameLoader()->notifier()->didReceiveAuthenticationChallenge(this, challenge);
>+
>+    if (m_options.allowCredentials == AllowStoredCredentials) {
>+        if (m_options.crossOriginCredentialPolicy == AllowClientCrossOriginCredentials || m_frame->document()->securityOrigin()->canRequest(originalRequest().url())) {
>+            frameLoader()->notifier()->didReceiveAuthenticationChallenge(this, challenge);
>+            return;
>+        }
>+    
>+    }
>+    // Only these platforms provide a way to continue without credentials.
>+    // If we can't continue with credentials, we need to cancel the load altogether.
>+#if PLATFORM(MAC) || USE(CFNETWORK) || USE(CURL)
>+    handle()->receivedRequestToContinueWithoutCredential(challenge);
>+    ASSERT(!handle()->hasAuthenticationChallenge());
>+#else
>+    didFail(blockedError());
>+#endif
> }
> 
> void ResourceLoader::didCancelAuthenticationChallenge(const AuthenticationChallenge& challenge)
>Index: Source/WebCore/loader/MainResourceLoader.cpp
>===================================================================
>--- Source/WebCore/loader/MainResourceLoader.cpp	(revision 94022)
>+++ Source/WebCore/loader/MainResourceLoader.cpp	(working copy)
>@@ -60,7 +60,7 @@
> namespace WebCore {
> 
> MainResourceLoader::MainResourceLoader(Frame* frame)
>-    : ResourceLoader(frame, ResourceLoaderOptions(SendCallbacks, SniffContent, BufferData, AllowStoredCredentials))
>+    : ResourceLoader(frame, ResourceLoaderOptions(SendCallbacks, SniffContent, BufferData, AllowStoredCredentials, AllowClientCrossOriginCredentials))
>     , m_dataLoadTimer(this, &MainResourceLoader::handleDataLoadNow)
>     , m_loadingMultipartContent(false)
>     , m_waitingForContentPolicy(false)
>Index: Source/WebCore/loader/ResourceLoadScheduler.h
>===================================================================
>--- Source/WebCore/loader/ResourceLoadScheduler.h	(revision 94022)
>+++ Source/WebCore/loader/ResourceLoadScheduler.h	(working copy)
>@@ -51,7 +51,7 @@
> public:
>     friend ResourceLoadScheduler* resourceLoadScheduler();
> 
>-    PassRefPtr<SubresourceLoader> scheduleSubresourceLoad(Frame*, SubresourceLoaderClient*, const ResourceRequest&, ResourceLoadPriority = ResourceLoadPriorityLow, SecurityCheckPolicy = DoSecurityCheck, const ResourceLoaderOptions& = ResourceLoaderOptions(SendCallbacks, SniffContent, BufferData, AllowStoredCredentials));
>+    PassRefPtr<SubresourceLoader> scheduleSubresourceLoad(Frame*, SubresourceLoaderClient*, const ResourceRequest&, ResourceLoadPriority, SecurityCheckPolicy, const ResourceLoaderOptions&);
>     PassRefPtr<NetscapePlugInStreamLoader> schedulePluginStreamLoad(Frame*, NetscapePlugInStreamLoaderClient*, const ResourceRequest&);
>     void addMainResourceLoad(ResourceLoader*);
>     void remove(ResourceLoader*);
>Index: Source/WebCore/loader/DocumentThreadableLoader.h
>===================================================================
>--- Source/WebCore/loader/DocumentThreadableLoader.h	(revision 94022)
>+++ Source/WebCore/loader/DocumentThreadableLoader.h	(working copy)
>@@ -82,8 +82,6 @@
>         virtual void didFinishLoading(SubresourceLoader*, double);
>         virtual void didFail(SubresourceLoader*, const ResourceError&);
> 
>-        virtual void didReceiveAuthenticationChallenge(SubresourceLoader*, const AuthenticationChallenge&);
>-
>         void didReceiveResponse(unsigned long identifier, const ResourceResponse&);
>         void didFinishLoading(unsigned long identifier, double finishTime);
>         void makeSimpleCrossOriginAccessRequest(const ResourceRequest& request);
>Index: Source/WebCore/loader/SubresourceLoader.cpp
>===================================================================
>--- Source/WebCore/loader/SubresourceLoader.cpp	(revision 94022)
>+++ Source/WebCore/loader/SubresourceLoader.cpp	(working copy)
>@@ -234,25 +234,4 @@
>     m_documentLoader->removeSubresourceLoader(this);
> }
> 
>-void SubresourceLoader::didReceiveAuthenticationChallenge(const AuthenticationChallenge& challenge)
>-{
>-    RefPtr<SubresourceLoader> protect(this);
>-
>-    ASSERT(handle()->hasAuthenticationChallenge());
>-
>-    if (m_client)
>-        m_client->didReceiveAuthenticationChallenge(this, challenge);
>-    
>-    // The SubResourceLoaderClient may have cancelled this ResourceLoader in response to the challenge.  
>-    // If that's the case, don't call didReceiveAuthenticationChallenge.
>-    if (reachedTerminalState())
>-        return;
>-
>-    // It may have also handled authentication on its own.
>-    if (!handle()->hasAuthenticationChallenge())
>-        return;
>-
>-    ResourceLoader::didReceiveAuthenticationChallenge(challenge);
> }
>-
>-}
>Index: Source/WebCore/loader/DocumentThreadableLoader.cpp
>===================================================================
>--- Source/WebCore/loader/DocumentThreadableLoader.cpp	(revision 94022)
>+++ Source/WebCore/loader/DocumentThreadableLoader.cpp	(working copy)
>@@ -288,23 +288,6 @@
>     m_client->didFail(error);
> }
> 
>-void DocumentThreadableLoader::didReceiveAuthenticationChallenge(SubresourceLoader* loader, const AuthenticationChallenge& challenge)
>-{
>-    ASSERT(loader == m_loader);
>-    // Users are not prompted for credentials for cross-origin requests.
>-    if (!m_sameOriginRequest) {
>-#if PLATFORM(MAC) || USE(CFNETWORK) || USE(CURL)
>-        loader->handle()->receivedRequestToContinueWithoutCredential(challenge);
>-#else
>-        // These platforms don't provide a way to continue without credentials, cancel the load altogether.
>-        UNUSED_PARAM(challenge);
>-        RefPtr<DocumentThreadableLoader> protect(this);
>-        m_client->didFail(loader->blockedError());
>-        cancel();
>-#endif
>-    }
>-}
>-
> void DocumentThreadableLoader::preflightSuccess()
> {
>     OwnPtr<ResourceRequest> actualRequest;
>@@ -331,6 +314,7 @@
> 
>     if (m_async) {
>         ThreadableLoaderOptions options = m_options;
>+        options.crossOriginCredentialPolicy = BlockClientCrossOriginCredentials;
>         if (m_actualRequest) {
>             // Don't sniff content or send load callbacks for the preflight request.
>             options.sendLoadCallbacks = DoNotSendCallbacks;
>Index: Source/WebCore/loader/SubresourceLoader.h
>===================================================================
>--- Source/WebCore/loader/SubresourceLoader.h	(revision 94022)
>+++ Source/WebCore/loader/SubresourceLoader.h	(working copy)
>@@ -56,7 +56,6 @@
>         virtual void didReceiveCachedMetadata(const char*, int);
>         virtual void didFinishLoading(double finishTime);
>         virtual void didFail(const ResourceError&);
>-        virtual void didReceiveAuthenticationChallenge(const AuthenticationChallenge&);
>         virtual void willCancel(const ResourceError&);
>         virtual void didCancel(const ResourceError&);
>

Actions: View | Formatted Diff | Diff

Attachments on bug 65330: 102275 | 102970 | 102985 | 102990 | 103077 | 103316 | 104190 | 104198 | 104727 | 105410 | 105654 | 105664 | 108242