Title: [220459] trunk
Revision
220459
Author
dba...@webkit.org
Date
2017-08-09 09:30:41 -0700 (Wed, 09 Aug 2017)

Log Message

REGRESSION (r219013): OAuth flows are broken when redirecting back to application after authentication
https://bugs.webkit.org/show_bug.cgi?id=175247
<rdar://problem/33679804>

Reviewed by Brady Eidson.

Source/WebCore:

Add SPI so that Safari can differentiate between a form submission and a redirected form submission
and have PolicyCheck notify the frame loader client if the navigation was in response to receiving a
redirect response. This is the WebKit portion to fix an issue when a native app makes use of an OAuth
OAuth flow that bounces to Safari for user login and then bounce back to the app. Microsoft Graph's
OAuth flow is one example.

Safari was differentiating between a form submission and a redirected form submission based on the
nullity of WKNavigationAction.sourceFrame because in both cases the navigation type was WKNavigationTypeFormSubmitted.
The navigation type is the same for both navigations because WebKit always used the navigation
action from the original request for the redirect request when the original request redirected.
Prior to r219013, WKNavigationAction.sourceFrame would be nil for a form submission that redirects.
Following r219013, WKNavigationAction.sourceFrame is non-nil unless the navigation was initiated by
API. In particular, WKNavigationAction.sourceFrame is non-nil for the redirect navigation corresponding
to a form submission that redirects.

* loader/EmptyClients.cpp:
(WebCore::EmptyFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
* loader/FrameLoaderClient.h:
Have dispatchDecidePolicyForNavigationAction() take a boolean as to whether the navigation was in
response to receiving a redirect response.
* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNavigationPolicy): Notify the frame loader client whether the navigation
is in response to receiving a redirect response.

Source/WebKit:

Add SPI WKNavigationAction._isRedirect to query whether the navigation was in response to receiving
a redirect response. The majority of the WebKit change is plumbing this knowledge through to connect
it with the SPI.

* Shared/NavigationActionData.cpp:
(WebKit::NavigationActionData::encode const):
(WebKit::NavigationActionData::decode):
Encode and decode the boolean NavigationActionData::isRedirect.
* Shared/NavigationActionData.h:
* UIProcess/API/APINavigationAction.h:
* UIProcess/API/Cocoa/WKNavigationAction.mm:
(-[WKNavigationAction _isRedirect]): Added.
* UIProcess/API/Cocoa/WKNavigationActionPrivate.h:
* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction): Set NavigationActionData::isRedirect
depending on whether the navigation is in response to receiving a redirect response.
* WebProcess/WebCoreSupport/WebFrameLoaderClient.h:

Source/WebKitLegacy/mac:

Plumb knowledge of whether a navigation was in response to receiving a redirect response.
We do not actually make use of this knowledge in WebKitLegacy because we do not know of any
clients that need to make use of this information at this time. If such a needs comes up
then we can expose API/SPI similar to what we do for WebKit.

* WebCoreSupport/WebFrameLoaderClient.h:
* WebCoreSupport/WebFrameLoaderClient.mm:
(WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):

Source/WebKitLegacy/win:

Plumb knowledge of whether a navigation was in response to receiving a redirect response.
We do not actually make use of this knowledge in WebKitLegacy because we do not know of any
clients that need to make use of this information at this time. If such a needs comes up
then we can expose API/SPI similar to what we do for WebKit.

* WebCoreSupport/WebFrameLoaderClient.cpp:
(WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
* WebCoreSupport/WebFrameLoaderClient.h:

Tools:

Add tests for redirects.

* TestWebKitAPI/Tests/WebKit2Cocoa/DecidePolicyForNavigationAction.mm:
(TEST):
* TestWebKitAPI/cocoa/TestProtocol.mm:
(createRedirectURL):
(-[TestProtocol startLoading]):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (220458 => 220459)


--- trunk/Source/WebCore/ChangeLog	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebCore/ChangeLog	2017-08-09 16:30:41 UTC (rev 220459)
@@ -1,3 +1,35 @@
+2017-08-09  Daniel Bates  <daba...@apple.com>
+
+        REGRESSION (r219013): OAuth flows are broken when redirecting back to application after authentication
+        https://bugs.webkit.org/show_bug.cgi?id=175247
+        <rdar://problem/33679804>
+
+        Reviewed by Brady Eidson.
+
+        Add SPI so that Safari can differentiate between a form submission and a redirected form submission
+        and have PolicyCheck notify the frame loader client if the navigation was in response to receiving a
+        redirect response. This is the WebKit portion to fix an issue when a native app makes use of an OAuth
+        OAuth flow that bounces to Safari for user login and then bounce back to the app. Microsoft Graph's
+        OAuth flow is one example.
+
+        Safari was differentiating between a form submission and a redirected form submission based on the
+        nullity of WKNavigationAction.sourceFrame because in both cases the navigation type was WKNavigationTypeFormSubmitted.
+        The navigation type is the same for both navigations because WebKit always used the navigation
+        action from the original request for the redirect request when the original request redirected.
+        Prior to r219013, WKNavigationAction.sourceFrame would be nil for a form submission that redirects.
+        Following r219013, WKNavigationAction.sourceFrame is non-nil unless the navigation was initiated by
+        API. In particular, WKNavigationAction.sourceFrame is non-nil for the redirect navigation corresponding
+        to a form submission that redirects.
+
+        * loader/EmptyClients.cpp:
+        (WebCore::EmptyFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
+        * loader/FrameLoaderClient.h:
+        Have dispatchDecidePolicyForNavigationAction() take a boolean as to whether the navigation was in
+        response to receiving a redirect response.
+        * loader/PolicyChecker.cpp:
+        (WebCore::PolicyChecker::checkNavigationPolicy): Notify the frame loader client whether the navigation
+        is in response to receiving a redirect response.
+
 2017-08-09  Sam Weinig  <s...@webkit.org>
 
         WTF::Function does not allow for reference / non-default constructible return types

Modified: trunk/Source/WebCore/loader/EmptyClients.cpp (220458 => 220459)


--- trunk/Source/WebCore/loader/EmptyClients.cpp	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebCore/loader/EmptyClients.cpp	2017-08-09 16:30:41 UTC (rev 220459)
@@ -339,7 +339,7 @@
 
     void dispatchDecidePolicyForResponse(const ResourceResponse&, const ResourceRequest&, FramePolicyFunction&&) final { }
     void dispatchDecidePolicyForNewWindowAction(const NavigationAction&, const ResourceRequest&, FormState*, const String&, FramePolicyFunction&&) final;
-    void dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, FormState*, FramePolicyFunction&&) final;
+    void dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, bool didReceiveRedirectResponse, FormState*, FramePolicyFunction&&) final;
     void cancelPolicyCheck() final { }
 
     void dispatchUnableToImplementPolicy(const ResourceError&) final { }
@@ -608,7 +608,7 @@
 {
 }
 
-void EmptyFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, FormState*, FramePolicyFunction&&)
+void EmptyFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, bool, FormState*, FramePolicyFunction&&)
 {
 }
 

Modified: trunk/Source/WebCore/loader/FrameLoaderClient.h (220458 => 220459)


--- trunk/Source/WebCore/loader/FrameLoaderClient.h	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebCore/loader/FrameLoaderClient.h	2017-08-09 16:30:41 UTC (rev 220459)
@@ -179,7 +179,7 @@
 
     virtual void dispatchDecidePolicyForResponse(const ResourceResponse&, const ResourceRequest&, FramePolicyFunction&&) = 0;
     virtual void dispatchDecidePolicyForNewWindowAction(const NavigationAction&, const ResourceRequest&, FormState*, const String& frameName, FramePolicyFunction&&) = 0;
-    virtual void dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, FormState*, FramePolicyFunction&&) = 0;
+    virtual void dispatchDecidePolicyForNavigationAction(const NavigationAction&, const ResourceRequest&, bool didReceiveRedirectResponse, FormState*, FramePolicyFunction&&) = 0;
     virtual void cancelPolicyCheck() = 0;
 
     virtual void dispatchUnableToImplementPolicy(const ResourceError&) = 0;

Modified: trunk/Source/WebCore/loader/PolicyChecker.cpp (220458 => 220459)


--- trunk/Source/WebCore/loader/PolicyChecker.cpp	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebCore/loader/PolicyChecker.cpp	2017-08-09 16:30:41 UTC (rev 220459)
@@ -147,7 +147,7 @@
 
     m_delegateIsDecidingNavigationPolicy = true;
     m_suggestedFilename = action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute();
-    m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, formState, [this](PolicyAction action) {
+    m_frame.loader().client().dispatchDecidePolicyForNavigationAction(action, request, didReceiveRedirectResponse, formState, [this](PolicyAction action) {
         continueAfterNavigationPolicy(action);
     });
     m_delegateIsDecidingNavigationPolicy = false;

Modified: trunk/Source/WebKit/ChangeLog (220458 => 220459)


--- trunk/Source/WebKit/ChangeLog	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebKit/ChangeLog	2017-08-09 16:30:41 UTC (rev 220459)
@@ -1,3 +1,29 @@
+2017-08-09  Daniel Bates  <daba...@apple.com>
+
+        REGRESSION (r219013): OAuth flows are broken when redirecting back to application after authentication
+        https://bugs.webkit.org/show_bug.cgi?id=175247
+        <rdar://problem/33679804>
+
+        Reviewed by Brady Eidson.
+
+        Add SPI WKNavigationAction._isRedirect to query whether the navigation was in response to receiving
+        a redirect response. The majority of the WebKit change is plumbing this knowledge through to connect
+        it with the SPI.
+
+        * Shared/NavigationActionData.cpp:
+        (WebKit::NavigationActionData::encode const):
+        (WebKit::NavigationActionData::decode):
+        Encode and decode the boolean NavigationActionData::isRedirect.
+        * Shared/NavigationActionData.h:
+        * UIProcess/API/APINavigationAction.h:
+        * UIProcess/API/Cocoa/WKNavigationAction.mm:
+        (-[WKNavigationAction _isRedirect]): Added.
+        * UIProcess/API/Cocoa/WKNavigationActionPrivate.h:
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction): Set NavigationActionData::isRedirect
+        depending on whether the navigation is in response to receiving a redirect response.
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
+
 2017-08-09  Sam Weinig  <s...@webkit.org>
 
         WTF::Function does not allow for reference / non-default constructible return types

Modified: trunk/Source/WebKit/Shared/NavigationActionData.cpp (220458 => 220459)


--- trunk/Source/WebKit/Shared/NavigationActionData.cpp	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebKit/Shared/NavigationActionData.cpp	2017-08-09 16:30:41 UTC (rev 220459)
@@ -46,6 +46,7 @@
     encoder.encodeEnum(shouldOpenExternalURLsPolicy);
     encoder << downloadAttribute;
     encoder << clickLocationInRootViewCoordinates;
+    encoder << isRedirect;
 }
 
 bool NavigationActionData::decode(IPC::Decoder& decoder, NavigationActionData& result)
@@ -68,6 +69,8 @@
         return false;
     if (!decoder.decode(result.clickLocationInRootViewCoordinates))
         return false;
+    if (!decoder.decode(result.isRedirect))
+        return false;
 
     return true;
 }

Modified: trunk/Source/WebKit/Shared/NavigationActionData.h (220458 => 220459)


--- trunk/Source/WebKit/Shared/NavigationActionData.h	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebKit/Shared/NavigationActionData.h	2017-08-09 16:30:41 UTC (rev 220459)
@@ -49,6 +49,7 @@
     WebCore::ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy { WebCore::ShouldOpenExternalURLsPolicy::ShouldNotAllow };
     WTF::String downloadAttribute;
     WebCore::FloatPoint clickLocationInRootViewCoordinates;
+    bool isRedirect { false };
 };
 
 }

Modified: trunk/Source/WebKit/UIProcess/API/APINavigationAction.h (220458 => 220459)


--- trunk/Source/WebKit/UIProcess/API/APINavigationAction.h	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebKit/UIProcess/API/APINavigationAction.h	2017-08-09 16:30:41 UTC (rev 220459)
@@ -56,6 +56,7 @@
     bool shouldOpenExternalSchemes() const { return m_navigationActionData.shouldOpenExternalURLsPolicy == WebCore::ShouldOpenExternalURLsPolicy::ShouldAllow || m_navigationActionData.shouldOpenExternalURLsPolicy == WebCore::ShouldOpenExternalURLsPolicy::ShouldAllowExternalSchemes; }
     bool shouldOpenAppLinks() const { return m_shouldOpenAppLinks && m_navigationActionData.shouldOpenExternalURLsPolicy == WebCore::ShouldOpenExternalURLsPolicy::ShouldAllow; }
     bool shouldPerformDownload() const { return !m_navigationActionData.downloadAttribute.isNull(); }
+    bool isRedirect() const { return m_navigationActionData.isRedirect; }
 
     bool isProcessingUserGesture() const { return m_userInitiatedAction; }
     UserInitiatedAction* userInitiatedAction() const { return m_userInitiatedAction.get(); }

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKNavigationAction.mm (220458 => 220459)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKNavigationAction.mm	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKNavigationAction.mm	2017-08-09 16:30:41 UTC (rev 220459)
@@ -231,6 +231,11 @@
     return nil;
 }
 
+- (BOOL)_isRedirect
+{
+    return _navigationAction->isRedirect();
+}
+
 @end
 
 #endif

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKNavigationActionPrivate.h (220458 => 220459)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKNavigationActionPrivate.h	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKNavigationActionPrivate.h	2017-08-09 16:30:41 UTC (rev 220459)
@@ -54,6 +54,8 @@
 @property (nonatomic, readonly) CGPoint _clickLocationInRootViewCoordinates WK_API_AVAILABLE(ios(11.0));
 #endif
 
+@property (nonatomic, readonly) BOOL _isRedirect WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
+
 @end
 
 #endif

Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (220458 => 220459)


--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp	2017-08-09 16:30:41 UTC (rev 220459)
@@ -757,7 +757,7 @@
     webPage->send(Messages::WebPageProxy::DecidePolicyForNewWindowAction(m_frame->frameID(), SecurityOriginData::fromFrame(coreFrame), navigationActionData, request, frameName, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())));
 }
 
-void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& navigationAction, const ResourceRequest& request, FormState* formState, FramePolicyFunction&& function)
+void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& navigationAction, const ResourceRequest& request, bool didReceiveRedirectResponse, FormState* formState, FramePolicyFunction&& function)
 {
     WebPage* webPage = m_frame->page();
     if (!webPage) {
@@ -808,6 +808,7 @@
     navigationActionData.canHandleRequest = webPage->canHandleRequest(request);
     navigationActionData.shouldOpenExternalURLsPolicy = navigationAction.shouldOpenExternalURLsPolicy();
     navigationActionData.downloadAttribute = navigationAction.downloadAttribute();
+    navigationActionData.isRedirect = didReceiveRedirectResponse;
 
     WebCore::Frame* coreFrame = m_frame->coreFrame();
     WebDocumentLoader* documentLoader = static_cast<WebDocumentLoader*>(coreFrame->loader().policyDocumentLoader());

Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h (220458 => 220459)


--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h	2017-08-09 16:30:41 UTC (rev 220459)
@@ -113,7 +113,7 @@
     
     void dispatchDecidePolicyForResponse(const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, WebCore::FramePolicyFunction&&) final;
     void dispatchDecidePolicyForNewWindowAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, const String& frameName, WebCore::FramePolicyFunction&&) final;
-    void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, WebCore::FramePolicyFunction&&) final;
+    void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, bool didReceiveRedirectResponse, WebCore::FormState*, WebCore::FramePolicyFunction&&) final;
     void cancelPolicyCheck() final;
     
     void dispatchUnableToImplementPolicy(const WebCore::ResourceError&) final;

Modified: trunk/Source/WebKitLegacy/mac/ChangeLog (220458 => 220459)


--- trunk/Source/WebKitLegacy/mac/ChangeLog	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebKitLegacy/mac/ChangeLog	2017-08-09 16:30:41 UTC (rev 220459)
@@ -1,3 +1,20 @@
+2017-08-09  Daniel Bates  <daba...@apple.com>
+
+        REGRESSION (r219013): OAuth flows are broken when redirecting back to application after authentication
+        https://bugs.webkit.org/show_bug.cgi?id=175247
+        <rdar://problem/33679804>
+
+        Reviewed by Brady Eidson.
+
+        Plumb knowledge of whether a navigation was in response to receiving a redirect response.
+        We do not actually make use of this knowledge in WebKitLegacy because we do not know of any
+        clients that need to make use of this information at this time. If such a needs comes up
+        then we can expose API/SPI similar to what we do for WebKit.
+
+        * WebCoreSupport/WebFrameLoaderClient.h:
+        * WebCoreSupport/WebFrameLoaderClient.mm:
+        (WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
+
 2017-08-08  Brady Eidson  <beid...@apple.com>
 
         Don't enable default icon loading in WK1 for apps linked against old SDKs.

Modified: trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.h (220458 => 220459)


--- trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.h	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.h	2017-08-09 16:30:41 UTC (rev 220459)
@@ -120,7 +120,7 @@
 
     void dispatchDecidePolicyForResponse(const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, WebCore::FramePolicyFunction&&) final;
     void dispatchDecidePolicyForNewWindowAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, const WTF::String& frameName, WebCore::FramePolicyFunction&&) final;
-    void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, WebCore::FramePolicyFunction&&) final;
+    void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, bool didReceiveRedirectResponse, WebCore::FormState*, WebCore::FramePolicyFunction&&) final;
     void cancelPolicyCheck() final;
 
     void dispatchUnableToImplementPolicy(const WebCore::ResourceError&) final;

Modified: trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm (220458 => 220459)


--- trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm	2017-08-09 16:30:41 UTC (rev 220459)
@@ -892,7 +892,7 @@
                           decisionListener:setUpPolicyListener(WTFMove(function), tryAppLink ? (NSURL *)request.url() : nil).get()];
 }
 
-void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& action, const ResourceRequest& request, FormState* formState, FramePolicyFunction&& function)
+void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& action, const ResourceRequest& request, bool, FormState* formState, FramePolicyFunction&& function)
 {
     WebView *webView = getWebView(m_webFrame.get());
     BOOL tryAppLink = shouldTryAppLink(webView, action, core(m_webFrame.get()));

Modified: trunk/Source/WebKitLegacy/win/ChangeLog (220458 => 220459)


--- trunk/Source/WebKitLegacy/win/ChangeLog	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebKitLegacy/win/ChangeLog	2017-08-09 16:30:41 UTC (rev 220459)
@@ -1,3 +1,20 @@
+2017-08-09  Daniel Bates  <daba...@apple.com>
+
+        REGRESSION (r219013): OAuth flows are broken when redirecting back to application after authentication
+        https://bugs.webkit.org/show_bug.cgi?id=175247
+        <rdar://problem/33679804>
+
+        Reviewed by Brady Eidson.
+
+        Plumb knowledge of whether a navigation was in response to receiving a redirect response.
+        We do not actually make use of this knowledge in WebKitLegacy because we do not know of any
+        clients that need to make use of this information at this time. If such a needs comes up
+        then we can expose API/SPI similar to what we do for WebKit.
+
+        * WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
+        * WebCoreSupport/WebFrameLoaderClient.h:
+
 2017-07-25  Said Abou-Hallawa  <sabouhall...@apple.com>
 
         Async image decoding for large images should be disabled after the first time a tile is painted

Modified: trunk/Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.cpp (220458 => 220459)


--- trunk/Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.cpp	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.cpp	2017-08-09 16:30:41 UTC (rev 220459)
@@ -549,7 +549,7 @@
     function(PolicyUse);
 }
 
-void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& action, const ResourceRequest& request, FormState* formState, FramePolicyFunction&& function)
+void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const NavigationAction& action, const ResourceRequest& request, bool, FormState* formState, FramePolicyFunction&& function)
 {
     WebView* webView = m_webFrame->webView();
     Frame* coreFrame = core(m_webFrame);

Modified: trunk/Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.h (220458 => 220459)


--- trunk/Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.h	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.h	2017-08-09 16:30:41 UTC (rev 220459)
@@ -98,7 +98,7 @@
 
     void dispatchDecidePolicyForResponse(const WebCore::ResourceResponse&, const WebCore::ResourceRequest&, WebCore::FramePolicyFunction&&) override;
     void dispatchDecidePolicyForNewWindowAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, const WTF::String& frameName, WebCore::FramePolicyFunction&&) override;
-    void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, WebCore::FormState*, WebCore::FramePolicyFunction&&) override;
+    void dispatchDecidePolicyForNavigationAction(const WebCore::NavigationAction&, const WebCore::ResourceRequest&, bool didReceiveRedirectResponse, WebCore::FormState*, WebCore::FramePolicyFunction&&) override;
     void cancelPolicyCheck() override;
 
     void dispatchUnableToImplementPolicy(const WebCore::ResourceError&) override;

Modified: trunk/Tools/ChangeLog (220458 => 220459)


--- trunk/Tools/ChangeLog	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Tools/ChangeLog	2017-08-09 16:30:41 UTC (rev 220459)
@@ -1,3 +1,19 @@
+2017-08-09  Daniel Bates  <daba...@apple.com>
+
+        REGRESSION (r219013): OAuth flows are broken when redirecting back to application after authentication
+        https://bugs.webkit.org/show_bug.cgi?id=175247
+        <rdar://problem/33679804>
+
+        Reviewed by Brady Eidson.
+
+        Add tests for redirects.
+
+        * TestWebKitAPI/Tests/WebKit2Cocoa/DecidePolicyForNavigationAction.mm:
+        (TEST):
+        * TestWebKitAPI/cocoa/TestProtocol.mm:
+        (createRedirectURL):
+        (-[TestProtocol startLoading]):
+
 2017-08-08  Wenson Hsieh  <wenson_hs...@apple.com>
 
         Unreviewed, rolling out r220393.

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/DecidePolicyForNavigationAction.mm (220458 => 220459)


--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/DecidePolicyForNavigationAction.mm	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/DecidePolicyForNavigationAction.mm	2017-08-09 16:30:41 UTC (rev 220459)
@@ -28,6 +28,8 @@
 #if PLATFORM(MAC)
 
 #import "PlatformUtilities.h"
+#import "TestProtocol.h"
+#import <WebKit/WKNavigationActionPrivate.h>
 #import <wtf/RetainPtr.h>
 #import <wtf/mac/AppKitCompatibilityDeclarations.h>
 
@@ -380,6 +382,150 @@
     action = ""
 }
 
+TEST(WebKit2, DecidePolicyForNavigationActionForHyperlinkThatRedirects)
+{
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+
+    auto window = adoptNS([[NSWindow alloc] initWithContentRect:[webView frame] styleMask:NSWindowStyleMaskBorderless backing:NSBackingStoreBuffered defer:YES]);
+    [[window contentView] addSubview:webView.get()];
+
+    auto controller = adoptNS([[DecidePolicyForNavigationActionController alloc] init]);
+    [webView setNavigationDelegate:controller.get()];
+    [webView setUIDelegate:controller.get()];
+
+    [TestProtocol registerWithScheme:@"http"];
+    finishedNavigation = false;
+    [webView loadHTMLString:@"<a style=\"display: block; height: 100%\" href="" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
+    TestWebKitAPI::Util::run(&finishedNavigation);
+
+    decidedPolicy = false;
+    [newWebView setNavigationDelegate:controller.get()];
+    NSPoint clickPoint = NSMakePoint(100, 100);
+    [[webView hitTest:clickPoint] mouseDown:[NSEvent mouseEventWithType:NSEventTypeLeftMouseDown location:clickPoint modifierFlags:0 timestamp:0 windowNumber:[window windowNumber] context:nil eventNumber:0 clickCount:1 pressure:1]];
+    [[webView hitTest:clickPoint] mouseUp:[NSEvent mouseEventWithType:NSEventTypeLeftMouseUp location:clickPoint modifierFlags:0 timestamp:0 windowNumber:[window windowNumber] context:nil eventNumber:0 clickCount:1 pressure:1]];
+    TestWebKitAPI::Util::run(&decidedPolicy);
+
+    EXPECT_EQ(WKNavigationTypeLinkActivated, [action navigationType]);
+    EXPECT_TRUE([action sourceFrame] == [action targetFrame]);
+    EXPECT_WK_STREQ("GET", [[action request] HTTPMethod]);
+    EXPECT_WK_STREQ("http://redirect/?result", [[[action request] URL] absoluteString]);
+    EXPECT_EQ(webView.get(), [[action sourceFrame] webView]);
+    EXPECT_WK_STREQ("http", [[[action sourceFrame] securityOrigin] protocol]);
+    EXPECT_WK_STREQ("webkit.org", [[[action sourceFrame] securityOrigin] host]);
+    EXPECT_FALSE([action _isRedirect]);
+
+    // Wait to decide policy for redirect.
+    decidedPolicy = false;
+    TestWebKitAPI::Util::run(&decidedPolicy);
+
+    EXPECT_EQ(WKNavigationTypeLinkActivated, [action navigationType]);
+    EXPECT_TRUE([action sourceFrame] == [action targetFrame]);
+    EXPECT_WK_STREQ("GET", [[action request] HTTPMethod]);
+    EXPECT_WK_STREQ("http://result/", [[[action request] URL] absoluteString]);
+    EXPECT_EQ(webView.get(), [[action sourceFrame] webView]);
+    EXPECT_WK_STREQ("http", [[[action sourceFrame] securityOrigin] protocol]);
+    EXPECT_WK_STREQ("webkit.org", [[[action sourceFrame] securityOrigin] host]);
+    EXPECT_TRUE([action _isRedirect]);
+
+    [TestProtocol unregister];
+    newWebView = nullptr;
+    action = ""
+}
+
+TEST(WebKit2, DecidePolicyForNavigationActionForPOSTFormSubmissionThatRedirectsToGET)
+{
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+
+    auto window = adoptNS([[NSWindow alloc] initWithContentRect:[webView frame] styleMask:NSWindowStyleMaskBorderless backing:NSBackingStoreBuffered defer:YES]);
+    [[window contentView] addSubview:webView.get()];
+
+    auto controller = adoptNS([[DecidePolicyForNavigationActionController alloc] init]);
+    [webView setNavigationDelegate:controller.get()];
+    [webView setUIDelegate:controller.get()];
+
+    finishedNavigation = false;
+    [webView loadHTMLString:@"<form action="" method=\"POST\"><input type=\"submit\" name=\"submitButton\" value=\"Submit\"></form>" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
+    TestWebKitAPI::Util::run(&finishedNavigation);
+
+    [TestProtocol registerWithScheme:@"http"];
+    decidedPolicy = false;
+    [webView evaluateJavaScript:@"document.forms[0].submit()" completionHandler:nil];
+    TestWebKitAPI::Util::run(&decidedPolicy);
+
+    EXPECT_EQ(WKNavigationTypeFormSubmitted, [action navigationType]);
+    EXPECT_TRUE([action sourceFrame] == [action targetFrame]);
+    EXPECT_WK_STREQ("POST", [[action request] HTTPMethod]);
+    EXPECT_WK_STREQ("http://redirect/?result", [[[action request] URL] absoluteString]);
+    EXPECT_EQ(webView.get(), [[action sourceFrame] webView]);
+    EXPECT_WK_STREQ("http", [[[action sourceFrame] securityOrigin] protocol]);
+    EXPECT_WK_STREQ("webkit.org", [[[action sourceFrame] securityOrigin] host]);
+    EXPECT_FALSE([action _isRedirect]);
+
+    // Wait to decide policy for redirect.
+    decidedPolicy = false;
+    TestWebKitAPI::Util::run(&decidedPolicy);
+
+    EXPECT_EQ(WKNavigationTypeFormSubmitted, [action navigationType]);
+    EXPECT_TRUE([action sourceFrame] == [action targetFrame]);
+    EXPECT_WK_STREQ("GET", [[action request] HTTPMethod]);
+    EXPECT_WK_STREQ("http://result/", [[[action request] URL] absoluteString]);
+    EXPECT_EQ(webView.get(), [[action sourceFrame] webView]);
+    EXPECT_WK_STREQ("http", [[[action sourceFrame] securityOrigin] protocol]);
+    EXPECT_WK_STREQ("webkit.org", [[[action sourceFrame] securityOrigin] host]);
+    EXPECT_TRUE([action _isRedirect]);
+
+    [TestProtocol unregister];
+    newWebView = nullptr;
+    action = ""
+}
+
+TEST(WebKit2, DecidePolicyForNavigationActionForPOSTFormSubmissionThatRedirectsToPOST)
+{
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+
+    auto window = adoptNS([[NSWindow alloc] initWithContentRect:[webView frame] styleMask:NSWindowStyleMaskBorderless backing:NSBackingStoreBuffered defer:YES]);
+    [[window contentView] addSubview:webView.get()];
+
+    auto controller = adoptNS([[DecidePolicyForNavigationActionController alloc] init]);
+    [webView setNavigationDelegate:controller.get()];
+    [webView setUIDelegate:controller.get()];
+
+    finishedNavigation = false;
+    [webView loadHTMLString:@"<form action="" method=\"POST\"><input type=\"submit\" name=\"submitButton\" value=\"Submit\"></form>" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
+    TestWebKitAPI::Util::run(&finishedNavigation);
+
+    [TestProtocol registerWithScheme:@"http"];
+    decidedPolicy = false;
+    [webView evaluateJavaScript:@"document.forms[0].submit()" completionHandler:nil];
+    TestWebKitAPI::Util::run(&decidedPolicy);
+
+    EXPECT_EQ(WKNavigationTypeFormSubmitted, [action navigationType]);
+    EXPECT_TRUE([action sourceFrame] == [action targetFrame]);
+    EXPECT_WK_STREQ("POST", [[action request] HTTPMethod]);
+    EXPECT_WK_STREQ("http://307-redirect/?result", [[[action request] URL] absoluteString]);
+    EXPECT_EQ(webView.get(), [[action sourceFrame] webView]);
+    EXPECT_WK_STREQ("http", [[[action sourceFrame] securityOrigin] protocol]);
+    EXPECT_WK_STREQ("webkit.org", [[[action sourceFrame] securityOrigin] host]);
+    EXPECT_FALSE([action _isRedirect]);
+
+    // Wait to decide policy for redirect.
+    decidedPolicy = false;
+    TestWebKitAPI::Util::run(&decidedPolicy);
+
+    EXPECT_EQ(WKNavigationTypeFormSubmitted, [action navigationType]);
+    EXPECT_TRUE([action sourceFrame] == [action targetFrame]);
+    EXPECT_WK_STREQ("POST", [[action request] HTTPMethod]);
+    EXPECT_WK_STREQ("http://result/", [[[action request] URL] absoluteString]);
+    EXPECT_EQ(webView.get(), [[action sourceFrame] webView]);
+    EXPECT_WK_STREQ("http", [[[action sourceFrame] securityOrigin] protocol]);
+    EXPECT_WK_STREQ("webkit.org", [[[action sourceFrame] securityOrigin] host]);
+    EXPECT_TRUE([action _isRedirect]);
+
+    [TestProtocol unregister];
+    newWebView = nullptr;
+    action = ""
+}
+
 #endif
 
 #endif

Modified: trunk/Tools/TestWebKitAPI/cocoa/TestProtocol.mm (220458 => 220459)


--- trunk/Tools/TestWebKitAPI/cocoa/TestProtocol.mm	2017-08-09 16:02:10 UTC (rev 220458)
+++ trunk/Tools/TestWebKitAPI/cocoa/TestProtocol.mm	2017-08-09 16:30:41 UTC (rev 220459)
@@ -72,17 +72,29 @@
     testScheme = nil;
 }
 
+static NSURL *createRedirectURL(NSString *query)
+{
+    return [NSURL URLWithString:[NSString stringWithFormat:@"%@://%@", testScheme, query]];
+}
+
 - (void)startLoading
 {
     NSURL *requestURL = self.request.URL;
     EXPECT_TRUE([requestURL.scheme isEqualToString:testScheme]);
 
+    if ([requestURL.host isEqualToString:@"307-redirect"]) {
+        RetainPtr<NSHTTPURLResponse> response = adoptNS([[NSHTTPURLResponse alloc] initWithURL:requestURL statusCode:307 HTTPVersion:@"HTTP/1.1" headerFields:@{@"Content-Type" : @"text/html"}]);
+        NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:createRedirectURL(requestURL.query)];
+        request.HTTPMethod = self.request.HTTPMethod;
+        [self.client URLProtocol:self wasRedirectedToRequest:request redirectResponse:response.get()];
+        return;
+    }
+
     NSData *data = "" dataUsingEncoding:NSASCIIStringEncoding];
     RetainPtr<NSURLResponse> response = adoptNS([[NSURLResponse alloc] initWithURL:requestURL MIMEType:@"text/html" expectedContentLength:data.length textEncodingName:nil]);
 
     if ([requestURL.host isEqualToString:@"redirect"]) {
-        NSURL *redirectURL = [NSURL URLWithString:[NSString stringWithFormat:@"%@://%@", testScheme, requestURL.query]];
-        [self.client URLProtocol:self wasRedirectedToRequest:[NSURLRequest requestWithURL:redirectURL] redirectResponse:response.get()];
+        [self.client URLProtocol:self wasRedirectedToRequest:[NSURLRequest requestWithURL:createRedirectURL(requestURL.query)] redirectResponse:response.get()];
         return;
     }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to