Throw an exception when denying access to 'Frame's 'location' setter.

Source/bindings/v8/BindingSecurity.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 16 matching lines @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 #include "bindings/v8/BindingSecurity.h"
 
 #include "bindings/v8/V8Binding.h"
 #include "core/dom/Document.h"
 #include "core/html/HTMLFrameElementBase.h"
-#include "core/html/parser/HTMLParserIdioms.h"
 #include "core/page/DOMWindow.h"
 #include "core/page/Frame.h"
 #include "core/page/Settings.h"
 #include "weborigin/SecurityOrigin.h"
 
 namespace WebCore {
 
-static bool canAccessDocument(Document* targetDocument, SecurityReportingOption reportingOption = ReportSecurityError)
+static bool isDocumentAccessibleFromActiveDOMWindow(Document* targetDocument)
 {
     if (!targetDocument)
         return false;
 
     DOMWindow* active = activeDOMWindow();
     if (!active)
         return false;
 
     if (active->document()->securityOrigin()->canAccess(targetDocument->securityOrigin()))
         return true;
 
+    return false;
+}
+
+static bool canAccessDocument(Document* targetDocument, ExceptionState& es)

[abarth-chromium, 2013/08/12 19:57:03]

What's the point of having this be a separate function?  There's exactly one
caller.

[Mike West, 2013/08/13 08:30:51]

This is the first patch that throws an exception on access violation, but my
goal is to move all the current usage over to the exception-throwing version.
So, there's exactly one caller _for now_. :)

+{
+    if (isDocumentAccessibleFromActiveDOMWindow(targetDocument))
+        return true;
+
+    es.throwDOMException(SecurityError, targetDocument->domWindow()->crossDomainAccessErrorMessage(activeDOMWindow()));

[abarth-chromium, 2013/08/12 19:58:26]

Wait a minute.  not lgtm.  This leaks the current URL of the frame, right?

+    return false;
+}
+
+static bool canAccessDocument(Document* targetDocument, SecurityReportingOption reportingOption = ReportSecurityError)
+{
+    if (isDocumentAccessibleFromActiveDOMWindow(targetDocument))
+        return true;
+
     if (reportingOption == ReportSecurityError) {
         if (Frame* frame = targetDocument->frame())
-            frame->domWindow()->printErrorMessage(targetDocument->domWindow()->crossDomainAccessErrorMessage(active));
+            frame->domWindow()->printErrorMessage(targetDocument->domWindow()->crossDomainAccessErrorMessage(activeDOMWindow()));

[abarth-chromium, 2013/08/12 19:57:03]

It's kine of lame that we call activeDOMWindow twice now.

[Mike West, 2013/08/13 08:30:51]

Good point. Fixed.

     }
 
     return false;
 }
 
 bool BindingSecurity::shouldAllowAccessToFrame(Frame* target, SecurityReportingOption reportingOption)
 {
     return target && canAccessDocument(target->document(), reportingOption);
 }
 
+bool BindingSecurity::shouldAllowAccessToFrame(Frame* target, ExceptionState& es)
+{
+    return target && canAccessDocument(target->document(), es);
+}
+
 bool BindingSecurity::shouldAllowAccessToNode(Node* target)
 {
     return target && canAccessDocument(target->document());
 }
 
-bool BindingSecurity::allowSettingFrameSrcToJavascriptUrl(HTMLFrameElementBase* frame, const String& value)
-{
-    return !protocolIsJavaScript(stripLeadingAndTrailingHTMLSpaces(value)) || canAccessDocument(frame->contentDocument());
 }
-
-}