WebCore:
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 May 2009 06:25:02 +0000 (06:25 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 May 2009 06:25:02 +0000 (06:25 +0000)
2009-05-27  Adam Barth  <abarth@webkit.org>

        Reviewed by Darin Adler.

        Clean up window.open()'s use of lexical and dynamic scope.

        Test: http/tests/security/frameNavigation/context-for-window-open.html

        * bindings/js/JSDOMBinding.cpp:
        (WebCore::toDynamicFrame):
        (WebCore::processingUserGesture):
        (WebCore::completeURL):
        * bindings/js/JSDOMBinding.h:
        * bindings/js/JSDOMWindowCustom.cpp:
        (WebCore::createWindow):
        (WebCore::JSDOMWindow::open):
        (WebCore::JSDOMWindow::showModalDialog):
        * bindings/v8/custom/V8DOMWindowCustom.cpp:
        (WebCore::CALLBACK_FUNC_DECL):
        (WebCore::createWindow):

LayoutTests:

2009-05-27  Adam Barth  <abarth@webkit.org>

        Reviewed by Darin Adler.

        Test whether lexical or dynamic scope is used for window.open().

        * http/tests/security/frameNavigation/context-for-window-open-expected.txt: Added.
        * http/tests/security/frameNavigation/context-for-window-open.html: Added.
        * http/tests/security/frameNavigation/resources/middle-frame-for-location.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@44221 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/security/frameNavigation/context-for-window-open-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/frameNavigation/context-for-window-open.html [new file with mode: 0644]
LayoutTests/http/tests/security/frameNavigation/resources/middle-frame-for-location.html
WebCore/ChangeLog
WebCore/bindings/js/JSDOMBinding.cpp
WebCore/bindings/js/JSDOMBinding.h
WebCore/bindings/js/JSDOMWindowCustom.cpp
WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp

index ba62c8c1eb5b6291953c1ea2f9fcdc42524a3bee..753fed4e8ba9c6a0be03b35ef6d9ee81e365b2b7 100644 (file)
@@ -1,3 +1,13 @@
+2009-05-27  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        Test whether lexical or dynamic scope is used for window.open().
+
+        * http/tests/security/frameNavigation/context-for-window-open-expected.txt: Added.
+        * http/tests/security/frameNavigation/context-for-window-open.html: Added.
+        * http/tests/security/frameNavigation/resources/middle-frame-for-location.html:
+
 2009-05-27  Dan Bernstein  <mitz@apple.com>
 
         Reviewed by Sam Weinig.
diff --git a/LayoutTests/http/tests/security/frameNavigation/context-for-window-open-expected.txt b/LayoutTests/http/tests/security/frameNavigation/context-for-window-open-expected.txt
new file mode 100644 (file)
index 0000000..5bc4ada
--- /dev/null
@@ -0,0 +1,2 @@
+document.referrer = http://127.0.0.1:8000/security/frameNavigation/resources/middle-frame-for-location.html
+
diff --git a/LayoutTests/http/tests/security/frameNavigation/context-for-window-open.html b/LayoutTests/http/tests/security/frameNavigation/context-for-window-open.html
new file mode 100644 (file)
index 0000000..4a0a677
--- /dev/null
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+}
+
+function log(msg) {
+    var div = document.createElement("div");
+    div.appendChild(document.createTextNode(msg));
+    document.getElementById("console").appendChild(div);
+}
+
+function go() {
+    window.open("target-for-location.html", "target");
+}
+
+window.addEventListener("message", function(e) {
+    log(e.data);
+
+    if (window.layoutTestController)
+        layoutTestController.notifyDone();
+}, false);
+</script>
+</head>
+<body>
+<div id="console"></div>
+<iframe src="resources/middle-frame-for-location.html">
+</body>
+</html>
+
index a3b906eeb9b15132916cc160c5431cd0c02f258f..ba6a3423dcbbb9dce1f05c39f81e86cefe887fc0 100644 (file)
@@ -1,4 +1,4 @@
-<iframe src="about:blank"></iframe>
+<iframe name="target" src="about:blank"></iframe>
 <script>
 top.go();
 </script>
index 66528612d743d077fb0538628a066bb1428a6817..b80c39c2faf64e966c6ef1fda6e983987c53e3d5 100644 (file)
@@ -1,3 +1,24 @@
+2009-05-27  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        Clean up window.open()'s use of lexical and dynamic scope.
+
+        Test: http/tests/security/frameNavigation/context-for-window-open.html
+
+        * bindings/js/JSDOMBinding.cpp:
+        (WebCore::toDynamicFrame):
+        (WebCore::processingUserGesture):
+        (WebCore::completeURL):
+        * bindings/js/JSDOMBinding.h:
+        * bindings/js/JSDOMWindowCustom.cpp:
+        (WebCore::createWindow):
+        (WebCore::JSDOMWindow::open):
+        (WebCore::JSDOMWindow::showModalDialog):
+        * bindings/v8/custom/V8DOMWindowCustom.cpp:
+        (WebCore::CALLBACK_FUNC_DECL):
+        (WebCore::createWindow):
+
 2009-05-27  Alexey Proskuryakov  <ap@webkit.org>
 
         Reviewed by Oliver Hunt.
index dd4078b1ee03c6118f3cb37236000f8305b93bc9..4f587971d6111e9f039a9fe8d84ebba1782acbdb 100644 (file)
@@ -523,22 +523,26 @@ Frame* toLexicalFrame(ExecState* exec)
     return asJSDOMWindow(exec->lexicalGlobalObject())->impl()->frame();
 }
 
+Frame* toDynamicFrame(ExecState* exec)
+{
+    return asJSDOMWindow(exec->dynamicGlobalObject())->impl()->frame();
+}
+
 bool processingUserGesture(ExecState* exec)
 {
-    Frame* frame = asJSDOMWindow(exec->dynamicGlobalObject())->impl()->frame();
+    Frame* frame = toDynamicFrame(exec);
     return frame && frame->script()->processingUserGesture();
 }
 
 KURL completeURL(ExecState* exec, const String& relativeURL)
 {
     // For histoical reasons, we need to complete the URL using the dynamic frame.
-    Frame* frame = asJSDOMWindow(exec->dynamicGlobalObject())->impl()->frame();
+    Frame* frame = toDynamicFrame(exec);
     if (!frame)
         return KURL();
     return frame->loader()->completeURL(relativeURL);
 }
 
-
 JSValue objectToStringFunctionGetter(ExecState* exec, const Identifier& propertyName, const PropertySlot&)
 {
     return new (exec) PrototypeFunction(exec, 0, propertyName, objectProtoFuncToString);
index 7a29abcfaf774234037227fb99a78617cbcc5af1..1378c9189e1d29d7cc7f7dec29c956253193adf6 100644 (file)
@@ -188,6 +188,7 @@ namespace WebCore {
     JSC::JSValue objectToStringFunctionGetter(JSC::ExecState*, const JSC::Identifier& propertyName, const JSC::PropertySlot&);
 
     Frame* toLexicalFrame(JSC::ExecState*);
+    Frame* toDynamicFrame(JSC::ExecState*);
     bool processingUserGesture(JSC::ExecState*);
     KURL completeURL(JSC::ExecState*, const String& relativeURL);
 
index 2a5e6e528c5e044b12d12805f037f29a85b1954b..7f6532d3481579bea338184e17d366fbb7d44ab1 100644 (file)
@@ -282,16 +282,20 @@ JSValue JSDOMWindow::worker(ExecState* exec) const
 // Custom functions
 
 // Helper for window.open() and window.showModalDialog()
-static Frame* createWindow(ExecState* exec, Frame* activeFrame, Frame* openerFrame, 
-                           const String& url, const String& frameName, 
+static Frame* createWindow(ExecState* exec, Frame* lexicalFrame, Frame* dynamicFrame,
+                           Frame* openerFrame, const String& url, const String& frameName, 
                            const WindowFeatures& windowFeatures, JSValue dialogArgs)
 {
-    ASSERT(activeFrame);
+    ASSERT(lexicalFrame);
+    ASSERT(dynamicFrame);
 
     ResourceRequest request;
 
-    request.setHTTPReferrer(activeFrame->loader()->outgoingReferrer());
-    FrameLoader::addHTTPOriginIfNeeded(request, activeFrame->loader()->outgoingOrigin());
+    // For whatever reason, Firefox uses the dynamicGlobalObject to determine
+    // the outgoingReferrer.  We replicate that behavior here.
+    String referrer = dynamicFrame->loader()->outgoingReferrer();
+    request.setHTTPReferrer(referrer);
+    FrameLoader::addHTTPOriginIfNeeded(request, dynamicFrame->loader()->outgoingOrigin());
     FrameLoadRequest frameRequest(request, frameName);
 
     // FIXME: It's much better for client API if a new window starts with a URL, here where we
@@ -305,7 +309,7 @@ static Frame* createWindow(ExecState* exec, Frame* activeFrame, Frame* openerFra
     // We pass in the opener frame here so it can be used for looking up the frame name, in case the active frame
     // is different from the opener frame, and the name references a frame relative to the opener frame, for example
     // "_self" or "_parent".
-    Frame* newFrame = activeFrame->loader()->createWindow(openerFrame->loader(), frameRequest, windowFeatures, created);
+    Frame* newFrame = lexicalFrame->loader()->createWindow(openerFrame->loader(), frameRequest, windowFeatures, created);
     if (!newFrame)
         return 0;
 
@@ -318,13 +322,13 @@ static Frame* createWindow(ExecState* exec, Frame* activeFrame, Frame* openerFra
         newWindow->putDirect(Identifier(exec, "dialogArguments"), dialogArgs);
 
     if (!protocolIsJavaScript(url) || newWindow->allowsAccessFrom(exec)) {
-        KURL completedURL = url.isEmpty() ? KURL("") : activeFrame->document()->completeURL(url);
-        bool userGesture = activeFrame->script()->processingUserGesture();
+        KURL completedURL = url.isEmpty() ? KURL("") : completeURL(exec, url);
+        bool userGesture = processingUserGesture(exec);
 
         if (created)
-            newFrame->loader()->changeLocation(completedURL, activeFrame->loader()->outgoingReferrer(), false, false, userGesture);
+            newFrame->loader()->changeLocation(completedURL, referrer, false, false, userGesture);
         else if (!url.isEmpty())
-            newFrame->loader()->scheduleLocationChange(completedURL.string(), activeFrame->loader()->outgoingReferrer(), !activeFrame->script()->anyPageIsProcessingUserGesture(), false, userGesture);
+            newFrame->loader()->scheduleLocationChange(completedURL.string(), referrer, !lexicalFrame->script()->anyPageIsProcessingUserGesture(), false, userGesture);
     }
 
     return newFrame;
@@ -335,9 +339,12 @@ JSValue JSDOMWindow::open(ExecState* exec, const ArgList& args)
     Frame* frame = impl()->frame();
     if (!frame)
         return jsUndefined();
-    Frame* activeFrame = asJSDOMWindow(exec->dynamicGlobalObject())->impl()->frame();
-    if (!activeFrame)
-        return  jsUndefined();
+    Frame* lexicalFrame = toLexicalFrame(exec);
+    if (!lexicalFrame)
+        return jsUndefined();
+    Frame* dynamicFrame = toDynamicFrame(exec);
+    if (!dynamicFrame)
+        return jsUndefined();
 
     Page* page = frame->page();
 
@@ -346,7 +353,7 @@ JSValue JSDOMWindow::open(ExecState* exec, const ArgList& args)
 
     // Because FrameTree::find() returns true for empty strings, we must check for empty framenames.
     // Otherwise, illegitimate window.open() calls with no name will pass right through the popup blocker.
-    if (!DOMWindow::allowPopUp(activeFrame) && (frameName.isEmpty() || !frame->tree()->find(frameName)))
+    if (!DOMWindow::allowPopUp(lexicalFrame) && (frameName.isEmpty() || !frame->tree()->find(frameName)))
         return jsUndefined();
 
     // Get the target frame for the special cases of _top and _parent.  In those
@@ -361,17 +368,23 @@ JSValue JSDOMWindow::open(ExecState* exec, const ArgList& args)
         topOrParent = true;
     }
     if (topOrParent) {
-        if (!activeFrame->loader()->shouldAllowNavigation(frame))
+        if (!shouldAllowNavigation(exec, frame))
             return jsUndefined();
 
         String completedURL;
         if (!urlString.isEmpty())
-            completedURL = activeFrame->document()->completeURL(urlString).string();
+            completedURL = completeURL(exec, urlString).string();
 
         const JSDOMWindow* targetedWindow = toJSDOMWindow(frame);
         if (!completedURL.isEmpty() && (!protocolIsJavaScript(completedURL) || (targetedWindow && targetedWindow->allowsAccessFrom(exec)))) {
-            bool userGesture = activeFrame->script()->processingUserGesture();
-            frame->loader()->scheduleLocationChange(completedURL, activeFrame->loader()->outgoingReferrer(), !activeFrame->script()->anyPageIsProcessingUserGesture(), false, userGesture);
+            bool userGesture = processingUserGesture(exec);
+
+            // For whatever reason, Firefox uses the dynamicGlobalObject to
+            // determine the outgoingReferrer.  We replicate that behavior
+            // here.
+            String referrer = dynamicFrame->loader()->outgoingReferrer();
+
+            frame->loader()->scheduleLocationChange(completedURL, referrer, !lexicalFrame->script()->anyPageIsProcessingUserGesture(), false, userGesture);
         }
         return toJS(exec, frame->domWindow());
     }
@@ -387,7 +400,7 @@ JSValue JSDOMWindow::open(ExecState* exec, const ArgList& args)
     windowFeatures.height = windowRect.height();
     windowFeatures.width = windowRect.width();
 
-    frame = createWindow(exec, activeFrame, frame, urlString, frameName, windowFeatures, JSValue());
+    frame = createWindow(exec, lexicalFrame, dynamicFrame, frame, urlString, frameName, windowFeatures, JSValue());
 
     if (!frame)
         return jsUndefined();
@@ -400,10 +413,14 @@ JSValue JSDOMWindow::showModalDialog(ExecState* exec, const ArgList& args)
     Frame* frame = impl()->frame();
     if (!frame)
         return jsUndefined();
+    Frame* lexicalFrame = toLexicalFrame(exec);
+    if (!lexicalFrame)
+        return jsUndefined();
+    Frame* dynamicFrame = toDynamicFrame(exec);
+    if (!dynamicFrame)
+        return jsUndefined();
 
-    Frame* activeFrame = asJSDOMWindow(exec->dynamicGlobalObject())->impl()->frame();
-
-    if (!DOMWindow::canShowModalDialogNow(frame) || !DOMWindow::allowPopUp(activeFrame))
+    if (!DOMWindow::canShowModalDialogNow(frame) || !DOMWindow::allowPopUp(lexicalFrame))
         return jsUndefined();
 
     String url = valueToStringWithUndefinedOrNullCheck(exec, args.at(0));
@@ -456,7 +473,7 @@ JSValue JSDOMWindow::showModalDialog(ExecState* exec, const ArgList& args)
     wargs.locationBarVisible = false;
     wargs.fullscreen = false;
 
-    Frame* dialogFrame = createWindow(exec, activeFrame, frame, url, "", wargs, dialogArgs);
+    Frame* dialogFrame = createWindow(exec, lexicalFrame, dynamicFrame, frame, url, "", wargs, dialogArgs);
     if (!dialogFrame)
         return jsUndefined();
 
index c04e0347bcc46acee6c1c1ea8a10cbc5bd373cac..c13c3b29490c49bc9fb073037cd8f489e225851f 100644 (file)
@@ -264,8 +264,6 @@ CALLBACK_FUNC_DECL(DOMWindowPostMessage)
     DOMWindow* source = V8Proxy::retrieveFrameForCallingContext()->domWindow();
     ASSERT(source->frame());
 
-    String uri = source->frame()->loader()->url().string();
-
     v8::TryCatch tryCatch;
 
     String message = toWebCoreString(args[0]);
@@ -477,17 +475,24 @@ static HashMap<String, String> parseModalDialogFeatures(const String& featuresAr
 }
 
 
-static Frame* createWindow(Frame* openerFrame,
+static Frame* createWindow(Frame* callingFrame,
+                           Frame* enteredFrame,
+                           Frame* openerFrame,
                            const String& url,
                            const String& frameName,
                            const WindowFeatures& windowFeatures,
                            v8::Local<v8::Value> dialogArgs)
 {
-    Frame* activeFrame = V8Proxy::retrieveFrameForEnteredContext();
+    ASSERT(callingFrame);
+    ASSERT(enteredFrame);
 
     ResourceRequest request;
-    if (activeFrame)
-        request.setHTTPReferrer(activeFrame->loader()->outgoingReferrer());
+
+    // For whatever reason, Firefox uses the entered frame to determine
+    // the outgoingReferrer.  We replicate that behavior here.
+    String referrer = enteredFrame->loader()->outgoingReferrer();
+    request.setHTTPReferrer(referrer);
+    FrameLoader::addHTTPOriginIfNeeded(request, enteredFrame->loader()->outgoingOrigin());
     FrameLoadRequest frameRequest(request, frameName);
 
     // FIXME: It's much better for client API if a new window starts with a URL,
@@ -504,7 +509,7 @@ static Frame* createWindow(Frame* openerFrame,
     // frame name, in case the active frame is different from the opener frame,
     // and the name references a frame relative to the opener frame, for example
     // "_self" or "_parent".
-    Frame* newFrame = activeFrame->loader()->createWindow(openerFrame->loader(), frameRequest, windowFeatures, created);
+    Frame* newFrame = callingFrame->loader()->createWindow(openerFrame->loader(), frameRequest, windowFeatures, created);
     if (!newFrame)
         return 0;
 
@@ -520,16 +525,15 @@ static Frame* createWindow(Frame* openerFrame,
         }
     }
 
-    if (!parseURL(url).startsWith("javascript:", false)
-        || ScriptController::isSafeScript(newFrame)) {
+    if (protocolIsJavaScript(url) || ScriptController::isSafeScript(newFrame)) {
         KURL completedUrl =
-            url.isEmpty() ? KURL("") : activeFrame->document()->completeURL(url);
-        bool userGesture = activeFrame->script()->processingUserGesture();
+            url.isEmpty() ? KURL("") : completeURL(url);
+        bool userGesture = processingUserGesture();
 
         if (created)
-            newFrame->loader()->changeLocation(completedUrl, activeFrame->loader()->outgoingReferrer(), false, false, userGesture);
+            newFrame->loader()->changeLocation(completedUrl, referrer, false, false, userGesture);
         else if (!url.isEmpty())
-            newFrame->loader()->scheduleLocationChange(completedUrl.string(), activeFrame->loader()->outgoingReferrer(), false, userGesture);
+            newFrame->loader()->scheduleLocationChange(completedUrl.string(), referrer, false, userGesture);
     }
 
     return newFrame;
@@ -547,6 +551,14 @@ CALLBACK_FUNC_DECL(DOMWindowShowModalDialog)
     if (!frame || !V8Proxy::CanAccessFrame(frame, true)) 
         return v8::Undefined();
 
+    Frame* callingFrame = V8Proxy::retrieveFrameForCallingContext();
+    if (!callingFrame)
+        return v8::Undefined();
+
+    Frame* enteredFrame = V8Proxy::retrieveFrameForEnteredContext();
+    if (!enteredFrame)
+        return v8::Undefined();
+
     if (!canShowModalDialogNow(frame) || !allowPopUp())
         return v8::Undefined();
 
@@ -593,7 +605,7 @@ CALLBACK_FUNC_DECL(DOMWindowShowModalDialog)
     windowFeatures.locationBarVisible = false;
     windowFeatures.fullscreen = false;
 
-    Frame* dialogFrame = createWindow(frame, url, "", windowFeatures, dialogArgs);
+    Frame* dialogFrame = createWindow(callingFrame, enteredFrame, frame, url, "", windowFeatures, dialogArgs);
     if (!dialogFrame)
         return v8::Undefined();
 
@@ -624,16 +636,20 @@ CALLBACK_FUNC_DECL(DOMWindowOpen)
     DOMWindow* parent = V8Proxy::ToNativeObject<DOMWindow>(V8ClassIndex::DOMWINDOW, args.Holder());
     Frame* frame = parent->frame();
 
-    if (!V8Proxy::CanAccessFrame(frame, true))
-      return v8::Undefined();
+    if (!frame || !V8Proxy::CanAccessFrame(frame, true))
+        return v8::Undefined();
 
-    Frame* activeFrame = V8Proxy::retrieveFrameForEnteredContext();
-    if (!activeFrame)
-      return v8::Undefined();
+    Frame* callingFrame = V8Proxy::retrieveFrameForCallingContext();
+    if (!callingFrame)
+        return v8::Undefined();
+
+    Frame* enteredFrame = V8Proxy::retrieveFrameForEnteredContext();
+    if (!enteredFrame)
+        return v8::Undefined();
 
     Page* page = frame->page();
     if (!page)
-      return v8::Undefined();
+        return v8::Undefined();
 
     String urlString = valueToStringWithNullOrUndefinedCheck(args[0]);
     AtomicString frameName = (args[1]->IsUndefined() || args[1]->IsNull()) ? "_blank" : AtomicString(toWebCoreString(args[1]));
@@ -658,18 +674,22 @@ CALLBACK_FUNC_DECL(DOMWindowOpen)
         topOrParent = true;
     }
     if (topOrParent) {
-        if (!activeFrame->loader()->shouldAllowNavigation(frame))
+        if (!shouldAllowNavigation(frame))
             return v8::Undefined();
     
         String completedUrl;
         if (!urlString.isEmpty())
-            completedUrl = activeFrame->document()->completeURL(urlString);
+            completedUrl = completeURL(urlString);
     
         if (!completedUrl.isEmpty() &&
-            (!parseURL(urlString).startsWith("javascript:", false)
-             || ScriptController::isSafeScript(frame))) {
-            bool userGesture = activeFrame->script()->processingUserGesture();
-            frame->loader()->scheduleLocationChange(completedUrl, activeFrame->loader()->outgoingReferrer(), false, userGesture);
+            (!protocolIsJavaScript(completedUrl) || ScriptController::isSafeScript(frame))) {
+            bool userGesture = processingUserGesture();
+
+            // For whatever reason, Firefox uses the entered frame to determine
+            // the outgoingReferrer.  We replicate that behavior here.
+            String referrer = enteredFrame->loader()->outgoingReferrer();
+
+            frame->loader()->scheduleLocationChange(completedUrl, referrer, false, userGesture);
         }
         return V8Proxy::ToV8Object(V8ClassIndex::DOMWINDOW, frame->domWindow());
     }
@@ -726,7 +746,7 @@ CALLBACK_FUNC_DECL(DOMWindowOpen)
         windowFeatures.ySet = false;
     }
 
-    frame = createWindow(frame, urlString, frameName, windowFeatures, v8::Local<v8::Value>());
+    frame = createWindow(callingFrame, enteredFrame, frame, urlString, frameName, windowFeatures, v8::Local<v8::Value>());
 
     if (!frame)
         return v8::Undefined();