Bug 1599256: Fix web compatibility issues by reverting changes and going back to loading about:blank and firing the onload event in case XFO blocks an iframe. r=jkt,smaug
authorChristoph Kerschbaumer <ckerschb@christophkerschbaumer.com>
Fri, 13 Dec 2019 07:07:49 +0000
changeset 506839 f1b4749d24a135ef79f185953812d176b1596427
parent 506838 262025967f526fd669743c45e74f8c07b4c8ba00
child 506840 b0d4fec975e29a402fe50b05528ca89fa9f60b49
push id36913
push useropoprus@mozilla.com
push dateFri, 13 Dec 2019 16:53:24 +0000
treeherdermozilla-central@1ed684598bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjkt, smaug
bugs1599256
milestone73.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1599256: Fix web compatibility issues by reverting changes and going back to loading about:blank and firing the onload event in case XFO blocks an iframe. r=jkt,smaug Differential Revision: https://phabricator.services.mozilla.com/D56166
dom/base/Document.cpp
dom/security/test/csp/test_ignore_xfo.html
testing/web-platform/meta/x-frame-options/deny.sub.html.ini
testing/web-platform/meta/x-frame-options/multiple.sub.html.ini
testing/web-platform/meta/x-frame-options/sameorigin.sub.html.ini
--- a/dom/base/Document.cpp
+++ b/dom/base/Document.cpp
@@ -3014,18 +3014,34 @@ nsresult Document::StartDocumentLoad(con
           aChannel->Cancel(NS_ERROR_CSP_FRAME_ANCESTOR_VIOLATION);
         }
       }
     }
 
     // the checks for type_subdoc or type_object happen within
     // CheckFrameOptions.
     if (!FramingChecker::CheckFrameOptions(aChannel, mCSP)) {
-      // stop!  ERROR page!
-      aChannel->Cancel(NS_ERROR_XFO_VIOLATION);
+      // Bug 1601887: Display error page but still fire onload
+      // event in case x-frame-options blocks a load.
+      // After Bug 1601887 the about:blank load here should disappear
+      // and we should cancel the channel by using
+      // aChannel->Cancel(NS_ERROR_XFO_VIOLATION) which then displays
+      // the error page.
+      aChannel->Cancel(NS_BINDING_ABORTED);
+      if (docShell) {
+        nsCOMPtr<nsIWebNavigation> webNav(do_QueryObject(docShell));
+        if (webNav) {
+          RefPtr<NullPrincipal> principal =
+              NullPrincipal::CreateWithInheritedAttributes(
+                  loadInfo->TriggeringPrincipal());
+          LoadURIOptions loadURIOptions;
+          loadURIOptions.mTriggeringPrincipal = principal;
+          webNav->LoadURI(NS_LITERAL_STRING("about:blank"), loadURIOptions);
+        }
+      }
     }
   }
 
   // Initialize FeaturePolicy
   rv = InitFeaturePolicy(aChannel);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = loadInfo->GetCookieSettings(getter_AddRefs(mCookieSettings));
--- a/dom/security/test/csp/test_ignore_xfo.html
+++ b/dom/security/test/csp/test_ignore_xfo.html
@@ -70,23 +70,27 @@ csp_testframe.onload = function() {
 csp_testframe.onerror = function() {
   ok(false, "sanity: should not fire onerror for csp_testframe");
   checkFinished();
 }
 csp_testframe.src = "file_ignore_xfo.html";
 
 // 2) test XFO with CSP_RO
 var csp_ro_testframe = document.getElementById("csp_ro_testframe");
-// If XFO denies framing the neither the onload, nor the onerror
-// event should fire, because we are displaying a about:neterror.
-// The actual error we are detecting within this test comes from
-// the xfo-on-violation-polcy observer.
+// If XFO denies framing the onload event handler should fire. At the
+// moment we are displaying about:blank (in regular mode and the error page
+// if fission is enabled) in case XFO blocks a frame, but after Bug 1601887
+// we are going to display about:neterror, but the onload event should still
+// fire. Please note that for the purpose of this test, the actual error we
+// are detecting comes from the xfo-on-violation-polcy observer.
 csp_ro_testframe.onload = function() {
-  ok(false, "sanity: should not fire onload for csp_ro_testframe");
-  checkFinished();
+  // Bug 1601887, onload event handler should fire even if frame is blocked.
+  // Please note that we can not add a todo, because the event would
+  // potentially fire after the test has already called SimpleTest.finish()
+  // and hence causes intermittent failures.
 }
 csp_ro_testframe.onerror = function() {
   ok(false, "sanity: should not fire onerror for csp_ro_testframe");
   checkFinished();
 }
 csp_ro_testframe.src = "file_ro_ignore_xfo.html";
 
 </script>
--- a/testing/web-platform/meta/x-frame-options/deny.sub.html.ini
+++ b/testing/web-platform/meta/x-frame-options/deny.sub.html.ini
@@ -1,8 +1,14 @@
 [deny.sub.html]
-  expected: TIMEOUT
+  expected:
+    if fission: TIMEOUT
+    OK
 
   [`XFO: DENY` blocks same-origin framing.]
-    expected: TIMEOUT
+    expected:
+      if fission: TIMEOUT
+      PASS
 
   [`XFO: DENY` blocks cross-origin framing.]
-    expected: TIMEOUT
+    expected:
+      if fission: TIMEOUT
+      PASS
--- a/testing/web-platform/meta/x-frame-options/multiple.sub.html.ini
+++ b/testing/web-platform/meta/x-frame-options/multiple.sub.html.ini
@@ -1,11 +1,19 @@
 [multiple.sub.html]
-  expected: TIMEOUT
+  expected:
+    if fission: TIMEOUT
+    OK
 
   [`XFO: SAMEORIGIN; XFO: DENY` blocks same-origin framing.]
-    expected: TIMEOUT
+    expected:
+      if fission: TIMEOUT
+      PASS
 
   [`XFO: DENY; XFO: SAMEORIGIN` blocks same-origin framing.]
-    expected: TIMEOUT
+    expected:
+      if fission: TIMEOUT
+      PASS
 
   [`XFO: SAMEORIGIN; XFO: SAMEORIGIN` blocks cross-origin framing.]
-    expected: TIMEOUT
+    expected:
+      if fission: TIMEOUT
+      PASS
--- a/testing/web-platform/meta/x-frame-options/sameorigin.sub.html.ini
+++ b/testing/web-platform/meta/x-frame-options/sameorigin.sub.html.ini
@@ -1,14 +1,24 @@
 [sameorigin.sub.html]
-  expected: TIMEOUT
+  expected:
+    if fission: TIMEOUT
+    OK
 
   [`XFO: SAMEORIGIN` blocks cross-origin framing.]
-    expected: TIMEOUT
+    expected:
+      if fission: TIMEOUT
+      PASS
 
   [`XFO: SAMEORIGIN` blocks cross-origin nested in same-origin framing.]
-    expected: TIMEOUT
+    expected:
+      if fission: TIMEOUT
+      PASS
 
   [`XFO: SAMEORIGIN` blocks same-origin nested in cross-origin framing.]
-    expected: TIMEOUT
+    expected:
+      if fission: TIMEOUT
+      PASS
 
   [`XFO: SAMEORIGIN` blocks cross-origin nested in cross-origin framing.]
-    expected: TIMEOUT
+    expected:
+      if fission: TIMEOUT
+      PASS