Bug 1287989. Don't try to do validation or fire submit events in iframes that are sandboxed without allow-forms. r=smaug

dom/html/HTMLButtonElement.cpp

@@ -381,11 +381,7 @@ HTMLButtonElement::PostHandleEvent(EventChainPostVisitor& aVisitor)
         // Using presShell to dispatch the event. It makes sure that
         // event is not handled if the window is being destroyed.
         if (presShell && (event.mMessage != eFormSubmit ||
-                          mForm->HasAttr(kNameSpaceID_None, nsGkAtoms::novalidate) ||
-                          // We know the element is a submit control, if this check is moved,
-                          // make sure formnovalidate is used only if it's a submit control.
-                          HasAttr(kNameSpaceID_None, nsGkAtoms::formnovalidate) ||
-                          mForm->CheckValidFormSubmission())) {
+                          mForm->SubmissionCanProceed(this))) {
           // TODO: removing this code and have the submit event sent by the form
           // see bug 592124.
           // Hold a strong ref while dispatching

dom/html/HTMLFormElement.cpp

@@ -1932,14 +1932,6 @@ HTMLFormElement::CheckValidFormSubmission()
   NS_ASSERTION(!HasAttr(kNameSpaceID_None, nsGkAtoms::novalidate),
                "We shouldn't be there if novalidate is set!");
 
-  // Don't do validation for a form submit done by a sandboxed document that
-  // doesn't have 'allow-forms', the submit will have been blocked and the
-  // HTML5 spec says we shouldn't validate in this case.
-  nsIDocument* doc = GetComposedDoc();
-  if (doc && (doc->GetSandboxFlags() & SANDBOXED_FORMS)) {
-    return true;
-  }
-
   // When .submit() is called aEvent = nullptr so we can rely on that to know if
   // we have to check the validity of the form.
   nsCOMPtr<nsIObserverService> service =
@@ -2027,6 +2019,41 @@ One should be implemented!");
   return true;
 }
 
+bool
+HTMLFormElement::SubmissionCanProceed(Element* aSubmitter)
+{
+#ifdef DEBUG
+  if (aSubmitter) {
+    nsCOMPtr<nsIFormControl> fc = do_QueryInterface(aSubmitter);
+    MOZ_ASSERT(fc);
+
+    uint32_t type = fc->GetType();
+    MOZ_ASSERT(type == NS_FORM_INPUT_SUBMIT ||
+               type == NS_FORM_INPUT_IMAGE ||
+               type == NS_FORM_BUTTON_SUBMIT,
+               "aSubmitter is not a submit control?");
+  }
+#endif
+
+  // Modified step 2 of
+  // https://html.spec.whatwg.org/multipage/forms.html#concept-form-submit --
+  // we're not checking whether the node document is disconnected yet...
+  if (OwnerDoc()->GetSandboxFlags() & SANDBOXED_FORMS) {
+    return false;
+  }
+
+  if (HasAttr(kNameSpaceID_None, nsGkAtoms::novalidate)) {
+    return true;
+  }
+
+  if (aSubmitter &&
+      aSubmitter->HasAttr(kNameSpaceID_None, nsGkAtoms::formnovalidate)) {
+    return true;
+  }
+
+  return CheckValidFormSubmission();
+}
+
 void
 HTMLFormElement::UpdateValidity(bool aElementValidity)
 {

dom/html/HTMLFormElement.h

@@ -277,6 +277,15 @@ public:
    */
   bool CheckValidFormSubmission();
 
+  /**
+   * Check whether submission can proceed for this form.  This basically
+   * implements steps 1-4 (more or less) of
+   * <https://html.spec.whatwg.org/multipage/forms.html#concept-form-submit>.
+   * aSubmitter, if not null, is the "submitter" from that algorithm.  Therefore
+   * it must be a valid submit control.
+   */
+  bool SubmissionCanProceed(Element* aSubmitter);
+
   /**
    * Walk over the form elements and call SubmitNamesValues() on them to get
    * their data pumped into the FormSubmitter.

dom/html/HTMLInputElement.cpp

@@ -3301,8 +3301,7 @@ HTMLInputElement::MaybeSubmitForm(nsPresContext* aPresContext)
     nsEventStatus status = nsEventStatus_eIgnore;
     shell->HandleDOMEventWithTarget(submitContent, &event, &status);
   } else if (!mForm->ImplicitSubmissionIsDisabled() &&
-             (mForm->HasAttr(kNameSpaceID_None, nsGkAtoms::novalidate) ||
-              mForm->CheckValidFormSubmission())) {
+             mForm->SubmissionCanProceed(nullptr)) {
     // TODO: removing this code and have the submit event sent by the form,
     // bug 592124.
     // If there's only one text control, just submit the form
@@ -4569,11 +4568,7 @@ HTMLInputElement::PostHandleEvent(EventChainPostVisitor& aVisitor)
             // TODO: removing this code and have the submit event sent by the
             // form, see bug 592124.
             if (presShell && (event.mMessage != eFormSubmit ||
-                              mForm->HasAttr(kNameSpaceID_None, nsGkAtoms::novalidate) ||
-                              // We know the element is a submit control, if this check is moved,
-                              // make sure formnovalidate is used only if it's a submit control.
-                              HasAttr(kNameSpaceID_None, nsGkAtoms::formnovalidate) ||
-                              mForm->CheckValidFormSubmission())) {
+                              mForm->SubmissionCanProceed(this))) {
               // Hold a strong ref while dispatching
               RefPtr<mozilla::dom::HTMLFormElement> form(mForm);
               presShell->HandleDOMEventWithTarget(mForm, &event, &status);

testing/web-platform/meta/MANIFEST.json

@@ -37598,6 +37598,14 @@
             "url": "/html/semantics/grouping-content/the-ol-element/reversed-1d.html"
           }
         ]
+      },
+      "testharness": {
+        "html/semantics/forms/the-form-element/form-submission-sandbox.html": [
+          {
+            "path": "html/semantics/forms/the-form-element/form-submission-sandbox.html",
+            "url": "/html/semantics/forms/the-form-element/form-submission-sandbox.html"
+          }
+        ]
       }
     },
     "reftest_nodes": {

testing/web-platform/tests/html/semantics/forms/the-form-element/form-submission-sandbox.html

@@ -0,0 +1,32 @@
+<!doctype html>
+<meta charset=utf-8>
+<title>Check whether the right events are fired (or not fired, as the case may
+  be) during form submission attempts in sandboxed iframes.</title>
+<script src=/resources/testharness.js></script>
+<script src=/resources/testharnessreport.js></script>
+<script>
+  var t1 = async_test("When form submission is disallowed we should get no events");
+  var t2 = async_test("When form submission is allowed we should get an invalid event");
+  window.onmessage = function(e) {
+    var data = e.data;
+    if (data.source == "?forms-disallowed") {
+      t1.step(function() {
+        assert_array_equals(data.events, []);
+        t1.done();
+      });
+    } else if (data.source == "?forms-allowed") {
+      t2.step(function() {
+        assert_array_equals(data.events, ["invalid"]);
+        t2.done();
+      });
+    } else {
+      test(function() { assert_true(false, `Unknown message ${data.source}`); });
+    }
+  }
+</script>
+<iframe src="support/form-submission-sandbox-iframe.html?forms-disallowed"
+        sandbox="allow-scripts">
+</iframe>
+<iframe src="support/form-submission-sandbox-iframe.html?forms-allowed"
+        sandbox="allow-scripts allow-forms">
+</iframe>

testing/web-platform/tests/html/semantics/forms/the-form-element/support/form-submission-sandbox-iframe.html

@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<script>
+  var events = [];
+
+  onload = function trySubmit() {
+    document.querySelector("input").click();
+    parent.postMessage({ source: location.search,
+                         events }, "*");
+  }
+</script>
+<form onsubmit="events.push('submit'); return false">
+  <input type="submit">
+  <input required oninvalid="events.push('invalid'); return false">
+</form>