rel="noreferrer" should make window.opener null
https://bugs.webkit.org/show_bug.cgi?id=141579
Reviewed by Darin Adler.
Source/WebCore:
Tests: http/tests/navigation/target-blank-opener-post.html
http/tests/navigation/target-blank-opener.html
We used to avoid passing window.opener policy by temporarily storing it in a FrameLoader
member variable. This works for some clients - ones that invoke delegate callbacks
synchronously - but not in the general case.
So, changed to passing the policy explicitly.
* WebCore.exp.in:
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::FrameLoader):
(WebCore::FrameLoader::urlSelected):
(WebCore::FrameLoader::loadURLIntoChildFrame):
(WebCore::FrameLoader::loadFrameRequest):
(WebCore::FrameLoader::loadURL):
(WebCore::FrameLoader::load):
(WebCore::FrameLoader::loadPostRequest):
(WebCore::FrameLoader::continueLoadAfterNewWindowPolicy):
* loader/FrameLoader.h:
(WebCore::FrameLoader::suppressOpenerInNewFrame): Deleted.
* loader/FrameLoaderTypes.h:
* loader/NavigationScheduler.cpp:
* page/ContextMenuController.cpp:
(WebCore::openNewWindow):
(WebCore::ContextMenuController::contextMenuItemSelected):
Source/WebKit/ios:
* WebView/WebPDFViewPlaceholder.mm:
(-[WebPDFViewPlaceholder simulateClickOnLinkToURL:]): Updated for a new WebCore
function signature. There is no rel="noreferrer" in PDF, so we can just always allow.
Source/WebKit/mac:
* WebView/WebPDFView.mm:
(-[WebPDFView PDFViewWillClickOnLink:withURL:]): Updated for a new WebCore
function signature. There is no rel="noreferrer" in PDF, so we can just always allow.
LayoutTests:
Unfortunately, these tests are not quite real, because they pass even without the
fix. There reason is that delegates respond synchronously in WKTR and DRT.
But if there is any large refactoring, there is a non-zero chance that the tests
will catch future mistakes.
* http/tests/navigation/resources/target-blank-opener-post-window.php: Added.
* http/tests/navigation/resources/target-blank-opener-window.php: Added.
* http/tests/navigation/target-blank-opener-expected.txt: Added.
* http/tests/navigation/target-blank-opener-post-expected.txt: Added.
* http/tests/navigation/target-blank-opener-post.html: Added.
* http/tests/navigation/target-blank-opener.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@180110 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 188e464..b33ffa5 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,37 @@
+2015-02-14 Alexey Proskuryakov <ap@apple.com>
+
+ rel="noreferrer" should make window.opener null
+ https://bugs.webkit.org/show_bug.cgi?id=141579
+
+ Reviewed by Darin Adler.
+
+ Tests: http/tests/navigation/target-blank-opener-post.html
+ http/tests/navigation/target-blank-opener.html
+
+ We used to avoid passing window.opener policy by temporarily storing it in a FrameLoader
+ member variable. This works for some clients - ones that invoke delegate callbacks
+ synchronously - but not in the general case.
+
+ So, changed to passing the policy explicitly.
+
+ * WebCore.exp.in:
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::FrameLoader):
+ (WebCore::FrameLoader::urlSelected):
+ (WebCore::FrameLoader::loadURLIntoChildFrame):
+ (WebCore::FrameLoader::loadFrameRequest):
+ (WebCore::FrameLoader::loadURL):
+ (WebCore::FrameLoader::load):
+ (WebCore::FrameLoader::loadPostRequest):
+ (WebCore::FrameLoader::continueLoadAfterNewWindowPolicy):
+ * loader/FrameLoader.h:
+ (WebCore::FrameLoader::suppressOpenerInNewFrame): Deleted.
+ * loader/FrameLoaderTypes.h:
+ * loader/NavigationScheduler.cpp:
+ * page/ContextMenuController.cpp:
+ (WebCore::openNewWindow):
+ (WebCore::ContextMenuController::contextMenuItemSelected):
+
2015-02-14 David Kilzer <ddkilzer@apple.com>
REGRESSION (r180082): WebCore build on Mountain Lion fails due to weak export for i386
diff --git a/Source/WebCore/WebCore.exp.in b/Source/WebCore/WebCore.exp.in
index d71fc1e..439c8d6 100644
--- a/Source/WebCore/WebCore.exp.in
+++ b/Source/WebCore/WebCore.exp.in
@@ -150,7 +150,7 @@
__ZN7WebCore11FrameLoader14detachChildrenEv
__ZN7WebCore11FrameLoader14stopAllLoadersENS_26ClearProvisionalItemPolicyE
__ZN7WebCore11FrameLoader16detachFromParentEv
-__ZN7WebCore11FrameLoader16loadFrameRequestERKNS_16FrameLoadRequestENS_11LockHistoryENS_19LockBackForwardListEN3WTF10PassRefPtrINS_5EventEEENS7_INS_9FormStateEEENS_18ShouldSendReferrerENS_27AllowNavigationToInvalidURLE
+__ZN7WebCore11FrameLoader16loadFrameRequestERKNS_16FrameLoadRequestENS_11LockHistoryENS_19LockBackForwardListEN3WTF10PassRefPtrINS_5EventEEENS7_INS_9FormStateEEENS_18ShouldSendReferrerENS_27AllowNavigationToInvalidURLENS_20NewFrameOpenerPolicyE
__ZN7WebCore11FrameLoader17stopForUserCancelEb
__ZN7WebCore11FrameLoader21loadURLIntoChildFrameERKNS_3URLERKN3WTF6StringEPNS_5FrameE
__ZN7WebCore11FrameLoader22findFrameForNavigationERKN3WTF12AtomicStringEPNS_8DocumentE
diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
index da76f62..05c8206 100644
--- a/Source/WebCore/loader/FrameLoader.cpp
+++ b/Source/WebCore/loader/FrameLoader.cpp
@@ -235,7 +235,6 @@
, m_shouldCallCheckLoadComplete(false)
, m_opener(nullptr)
, m_loadingFromCachedPage(false)
- , m_suppressOpenerInNewFrame(false)
, m_currentNavigationHasShownBeforeUnloadConfirmPanel(false)
, m_loadsSynchronously(false)
, m_forcedSandboxFlags(SandboxNone)
@@ -330,8 +329,6 @@
// corresponding parameter from ScriptController::executeIfJavaScriptURL() is addressed.
void FrameLoader::urlSelected(const FrameLoadRequest& passedRequest, PassRefPtr<Event> triggeringEvent, LockHistory lockHistory, LockBackForwardList lockBackForwardList, ShouldSendReferrer shouldSendReferrer, ShouldReplaceDocumentIfJavaScriptURL shouldReplaceDocumentIfJavaScriptURL, AllowNavigationToInvalidURL allowNavigationToInvalidURL)
{
- ASSERT(!m_suppressOpenerInNewFrame);
-
Ref<Frame> protect(m_frame);
FrameLoadRequest frameRequest(passedRequest);
@@ -341,13 +338,11 @@
if (frameRequest.frameName().isEmpty())
frameRequest.setFrameName(m_frame.document()->baseTarget());
- if (shouldSendReferrer == NeverSendReferrer)
- m_suppressOpenerInNewFrame = true;
addHTTPOriginIfNeeded(frameRequest.resourceRequest(), outgoingOrigin());
- loadFrameRequest(frameRequest, lockHistory, lockBackForwardList, triggeringEvent, 0, shouldSendReferrer, allowNavigationToInvalidURL);
+ NewFrameOpenerPolicy openerPolicy = (shouldSendReferrer == NeverSendReferrer) ? NewFrameOpenerPolicy::Suppress : NewFrameOpenerPolicy::Allow;
- m_suppressOpenerInNewFrame = false;
+ loadFrameRequest(frameRequest, lockHistory, lockBackForwardList, triggeringEvent, 0, shouldSendReferrer, allowNavigationToInvalidURL, openerPolicy);
}
void FrameLoader::submitForm(PassRefPtr<FormSubmission> submission)
@@ -925,7 +920,7 @@
}
}
- childFrame->loader().loadURL(url, referer, "_self", LockHistory::No, FrameLoadType::RedirectWithLockedBackForwardList, 0, 0, AllowNavigationToInvalidURL::Yes);
+ childFrame->loader().loadURL(url, referer, "_self", LockHistory::No, FrameLoadType::RedirectWithLockedBackForwardList, 0, 0, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress);
}
#if ENABLE(WEB_ARCHIVE) || ENABLE(MHTML)
@@ -1150,7 +1145,7 @@
}
void FrameLoader::loadFrameRequest(const FrameLoadRequest& request, LockHistory lockHistory, LockBackForwardList lockBackForwardList,
- PassRefPtr<Event> event, PassRefPtr<FormState> formState, ShouldSendReferrer shouldSendReferrer, AllowNavigationToInvalidURL allowNavigationToInvalidURL)
+ PassRefPtr<Event> event, PassRefPtr<FormState> formState, ShouldSendReferrer shouldSendReferrer, AllowNavigationToInvalidURL allowNavigationToInvalidURL, NewFrameOpenerPolicy openerPolicy)
{
// Protect frame from getting blown away inside dispatchBeforeLoadEvent in loadWithDocumentLoader.
Ref<Frame> protect(m_frame);
@@ -1180,9 +1175,9 @@
loadType = FrameLoadType::Standard;
if (request.resourceRequest().httpMethod() == "POST")
- loadPostRequest(request.resourceRequest(), referrer, request.frameName(), lockHistory, loadType, event, formState.get(), allowNavigationToInvalidURL);
+ loadPostRequest(request.resourceRequest(), referrer, request.frameName(), lockHistory, loadType, event, formState.get(), allowNavigationToInvalidURL, openerPolicy);
else
- loadURL(request.resourceRequest().url(), referrer, request.frameName(), lockHistory, loadType, event, formState.get(), allowNavigationToInvalidURL);
+ loadURL(request.resourceRequest().url(), referrer, request.frameName(), lockHistory, loadType, event, formState.get(), allowNavigationToInvalidURL, openerPolicy);
// FIXME: It's possible this targetFrame will not be the same frame that was targeted by the actual
// load if frame names have changed.
@@ -1197,7 +1192,7 @@
}
void FrameLoader::loadURL(const URL& newURL, const String& referrer, const String& frameName, LockHistory lockHistory, FrameLoadType newLoadType,
- PassRefPtr<Event> event, PassRefPtr<FormState> prpFormState, AllowNavigationToInvalidURL allowNavigationToInvalidURL)
+ PassRefPtr<Event> event, PassRefPtr<FormState> prpFormState, AllowNavigationToInvalidURL allowNavigationToInvalidURL, NewFrameOpenerPolicy openerPolicy)
{
if (m_inStopAllLoaders)
return;
@@ -1226,7 +1221,7 @@
// The search for a target frame is done earlier in the case of form submission.
Frame* targetFrame = isFormSubmission ? 0 : findFrameForNavigation(frameName);
if (targetFrame && targetFrame != &m_frame) {
- targetFrame->loader().loadURL(newURL, referrer, "_self", lockHistory, newLoadType, event, formState.release(), allowNavigationToInvalidURL);
+ targetFrame->loader().loadURL(newURL, referrer, "_self", lockHistory, newLoadType, event, formState.release(), allowNavigationToInvalidURL, openerPolicy);
return;
}
@@ -1236,8 +1231,8 @@
NavigationAction action(request, newLoadType, isFormSubmission, event);
if (!targetFrame && !frameName.isEmpty()) {
- policyChecker().checkNewWindowPolicy(action, request, formState.release(), frameName, [this, allowNavigationToInvalidURL](const ResourceRequest& request, PassRefPtr<FormState> formState, const String& frameName, const NavigationAction& action, bool shouldContinue) {
- continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL);
+ policyChecker().checkNewWindowPolicy(action, request, formState.release(), frameName, [this, allowNavigationToInvalidURL, openerPolicy](const ResourceRequest& request, PassRefPtr<FormState> formState, const String& frameName, const NavigationAction& action, bool shouldContinue) {
+ continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
});
return;
}
@@ -1306,7 +1301,7 @@
if (request.shouldCheckNewWindowPolicy()) {
policyChecker().checkNewWindowPolicy(NavigationAction(request.resourceRequest(), NavigationTypeOther), request.resourceRequest(), nullptr, request.frameName(), [this](const ResourceRequest& request, PassRefPtr<FormState> formState, const String& frameName, const NavigationAction& action, bool shouldContinue) {
- continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, AllowNavigationToInvalidURL::Yes);
+ continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress);
});
return;
@@ -2612,7 +2607,7 @@
request.setHTTPOrigin(origin);
}
-void FrameLoader::loadPostRequest(const ResourceRequest& inRequest, const String& referrer, const String& frameName, LockHistory lockHistory, FrameLoadType loadType, PassRefPtr<Event> event, PassRefPtr<FormState> prpFormState, AllowNavigationToInvalidURL allowNavigationToInvalidURL)
+void FrameLoader::loadPostRequest(const ResourceRequest& inRequest, const String& referrer, const String& frameName, LockHistory lockHistory, FrameLoadType loadType, PassRefPtr<Event> event, PassRefPtr<FormState> prpFormState, AllowNavigationToInvalidURL allowNavigationToInvalidURL, NewFrameOpenerPolicy openerPolicy)
{
RefPtr<FormState> formState = prpFormState;
@@ -2644,8 +2639,8 @@
return;
}
- policyChecker().checkNewWindowPolicy(action, workingResourceRequest, formState.release(), frameName, [this, allowNavigationToInvalidURL](const ResourceRequest& request, PassRefPtr<FormState> formState, const String& frameName, const NavigationAction& action, bool shouldContinue) {
- continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL);
+ policyChecker().checkNewWindowPolicy(action, workingResourceRequest, formState.release(), frameName, [this, allowNavigationToInvalidURL, openerPolicy](const ResourceRequest& request, PassRefPtr<FormState> formState, const String& frameName, const NavigationAction& action, bool shouldContinue) {
+ continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
});
return;
}
@@ -2962,7 +2957,7 @@
}
void FrameLoader::continueLoadAfterNewWindowPolicy(const ResourceRequest& request,
- PassRefPtr<FormState> formState, const String& frameName, const NavigationAction& action, bool shouldContinue, AllowNavigationToInvalidURL allowNavigationToInvalidURL)
+ PassRefPtr<FormState> formState, const String& frameName, const NavigationAction& action, bool shouldContinue, AllowNavigationToInvalidURL allowNavigationToInvalidURL, NewFrameOpenerPolicy openerPolicy)
{
if (!shouldContinue)
return;
@@ -2977,7 +2972,7 @@
mainFrame->page()->setOpenedByDOM();
mainFrame->loader().m_client.dispatchShow();
- if (!m_suppressOpenerInNewFrame) {
+ if (openerPolicy == NewFrameOpenerPolicy::Allow) {
mainFrame->loader().setOpener(frame.ptr());
mainFrame->document()->setReferrerPolicy(frame->document()->referrerPolicy());
}
diff --git a/Source/WebCore/loader/FrameLoader.h b/Source/WebCore/loader/FrameLoader.h
index 619c7c4..633407e 100644
--- a/Source/WebCore/loader/FrameLoader.h
+++ b/Source/WebCore/loader/FrameLoader.h
@@ -107,7 +107,7 @@
// FIXME: These are all functions which start loads. We have too many.
WEBCORE_EXPORT void loadURLIntoChildFrame(const URL&, const String& referer, Frame*);
WEBCORE_EXPORT void loadFrameRequest(const FrameLoadRequest&, LockHistory, LockBackForwardList, // Called by submitForm, calls loadPostRequest and loadURL.
- PassRefPtr<Event>, PassRefPtr<FormState>, ShouldSendReferrer, AllowNavigationToInvalidURL);
+ PassRefPtr<Event>, PassRefPtr<FormState>, ShouldSendReferrer, AllowNavigationToInvalidURL, NewFrameOpenerPolicy);
WEBCORE_EXPORT void load(const FrameLoadRequest&);
@@ -265,8 +265,6 @@
WEBCORE_EXPORT void setOriginalURLForDownloadRequest(ResourceRequest&);
- bool suppressOpenerInNewFrame() const { return m_suppressOpenerInNewFrame; }
-
bool quickRedirectComing() const { return m_quickRedirectComing; }
WEBCORE_EXPORT bool shouldClose();
@@ -318,7 +316,7 @@
bool handleBeforeUnloadEvent(Chrome&, FrameLoader* frameLoaderBeingNavigated);
void continueLoadAfterNavigationPolicy(const ResourceRequest&, PassRefPtr<FormState>, bool shouldContinue, AllowNavigationToInvalidURL);
- void continueLoadAfterNewWindowPolicy(const ResourceRequest&, PassRefPtr<FormState>, const String& frameName, const NavigationAction&, bool shouldContinue, AllowNavigationToInvalidURL);
+ void continueLoadAfterNewWindowPolicy(const ResourceRequest&, PassRefPtr<FormState>, const String& frameName, const NavigationAction&, bool shouldContinue, AllowNavigationToInvalidURL, NewFrameOpenerPolicy);
void continueFragmentScrollAfterNavigationPolicy(const ResourceRequest&, bool shouldContinue);
bool shouldPerformFragmentNavigation(bool isFormSubmission, const String& httpMethod, FrameLoadType, const URL&);
@@ -348,9 +346,9 @@
LockHistory, FrameLoadType, PassRefPtr<FormState>, AllowNavigationToInvalidURL);
void loadPostRequest(const ResourceRequest&, const String& referrer, // Called by loadFrameRequest, calls loadWithNavigationAction
- const String& frameName, LockHistory, FrameLoadType, PassRefPtr<Event>, PassRefPtr<FormState>, AllowNavigationToInvalidURL);
+ const String& frameName, LockHistory, FrameLoadType, PassRefPtr<Event>, PassRefPtr<FormState>, AllowNavigationToInvalidURL, NewFrameOpenerPolicy);
void loadURL(const URL&, const String& referrer, const String& frameName, // Called by loadFrameRequest, calls loadWithNavigationAction or dispatches to navigation policy delegate
- LockHistory, FrameLoadType, PassRefPtr<Event>, PassRefPtr<FormState>, AllowNavigationToInvalidURL);
+ LockHistory, FrameLoadType, PassRefPtr<Event>, PassRefPtr<FormState>, AllowNavigationToInvalidURL, NewFrameOpenerPolicy);
bool shouldReload(const URL& currentURL, const URL& destinationURL);
@@ -429,7 +427,6 @@
HashSet<Frame*> m_openedFrames;
bool m_loadingFromCachedPage;
- bool m_suppressOpenerInNewFrame;
bool m_currentNavigationHasShownBeforeUnloadConfirmPanel;
diff --git a/Source/WebCore/loader/FrameLoaderTypes.h b/Source/WebCore/loader/FrameLoaderTypes.h
index 9956d1a..e020c93 100644
--- a/Source/WebCore/loader/FrameLoaderTypes.h
+++ b/Source/WebCore/loader/FrameLoaderTypes.h
@@ -57,6 +57,11 @@
ReloadFromOrigin,
};
+enum class NewFrameOpenerPolicy {
+ Suppress,
+ Allow
+};
+
enum NavigationType {
NavigationTypeLinkClicked,
NavigationTypeFormSubmitted,
diff --git a/Source/WebCore/loader/NavigationScheduler.cpp b/Source/WebCore/loader/NavigationScheduler.cpp
index 91cba06..3af28db 100644
--- a/Source/WebCore/loader/NavigationScheduler.cpp
+++ b/Source/WebCore/loader/NavigationScheduler.cpp
@@ -246,7 +246,7 @@
return;
FrameLoadRequest frameRequest(requestingDocument->securityOrigin());
m_submission->populateFrameLoadRequest(frameRequest);
- frame.loader().loadFrameRequest(frameRequest, lockHistory(), lockBackForwardList(), m_submission->event(), m_submission->state(), MaybeSendReferrer, AllowNavigationToInvalidURL::Yes);
+ frame.loader().loadFrameRequest(frameRequest, lockHistory(), lockBackForwardList(), m_submission->event(), m_submission->state(), MaybeSendReferrer, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Allow);
}
virtual void didStartTimer(Frame& frame, Timer& timer) override
diff --git a/Source/WebCore/page/ContextMenuController.cpp b/Source/WebCore/page/ContextMenuController.cpp
index 9224eee..13c156a 100644
--- a/Source/WebCore/page/ContextMenuController.cpp
+++ b/Source/WebCore/page/ContextMenuController.cpp
@@ -202,7 +202,7 @@
if (!newPage)
return;
newPage->chrome().show();
- newPage->mainFrame().loader().loadFrameRequest(request, LockHistory::No, LockBackForwardList::No, 0, 0, MaybeSendReferrer, AllowNavigationToInvalidURL::Yes);
+ newPage->mainFrame().loader().loadFrameRequest(request, LockHistory::No, LockBackForwardList::No, 0, 0, MaybeSendReferrer, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress);
}
#if PLATFORM(GTK)
@@ -401,12 +401,12 @@
break;
case ContextMenuItemTagOpenLink:
if (Frame* targetFrame = m_context.hitTestResult().targetFrame())
- targetFrame->loader().loadFrameRequest(FrameLoadRequest(frame->document()->securityOrigin(), ResourceRequest(m_context.hitTestResult().absoluteLinkURL(), frame->loader().outgoingReferrer())), LockHistory::No, LockBackForwardList::No, 0, 0, MaybeSendReferrer, AllowNavigationToInvalidURL::Yes);
+ targetFrame->loader().loadFrameRequest(FrameLoadRequest(frame->document()->securityOrigin(), ResourceRequest(m_context.hitTestResult().absoluteLinkURL(), frame->loader().outgoingReferrer())), LockHistory::No, LockBackForwardList::No, 0, 0, MaybeSendReferrer, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress);
else
openNewWindow(m_context.hitTestResult().absoluteLinkURL(), frame);
break;
case ContextMenuItemTagOpenLinkInThisWindow:
- frame->loader().loadFrameRequest(FrameLoadRequest(frame->document()->securityOrigin(), ResourceRequest(m_context.hitTestResult().absoluteLinkURL(), frame->loader().outgoingReferrer())), LockHistory::No, LockBackForwardList::No, 0, 0, MaybeSendReferrer, AllowNavigationToInvalidURL::Yes);
+ frame->loader().loadFrameRequest(FrameLoadRequest(frame->document()->securityOrigin(), ResourceRequest(m_context.hitTestResult().absoluteLinkURL(), frame->loader().outgoingReferrer())), LockHistory::No, LockBackForwardList::No, 0, 0, MaybeSendReferrer, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress);
break;
case ContextMenuItemTagBold:
frame->editor().command("ToggleBold").execute();
