Changeset 284660 in webkit

Timestamp:

Oct 21, 2021 5:46:04 PM (9 months ago)

Author:

Chris Dumez

Message:

Form submission should be cancelled if the form gets detached from inside the formdata event handler
​https://bugs.webkit.org/show_bug.cgi?id=232114

Reviewed by Alex Christensen.

Source/WebCore:

Per the HTML specification [1], form submission should abort if the form cannot navigate (which is true
when the form is detached). The algorithm in the specification does the check twice, once at the very
beginning (Step 1 in the spec), and again after calling the "constructing the entry list" algorithm
(step 9 in the spec). The reason we need to do the check again is that the "constructing the entry list"
algorithm fires the "formdata" event and may thus run JavaScript and the JS can detach the form element.

In HTMLFormElement::submit(), we were doing only the "form is connected" check only at the beginning
of the function and failing to do so after constructing the FormSubmission object (which ends up constructing
the entry list). This patch fixes that.

[1] ​https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#concept-form-submit

Test: fast/forms/remove-form-inside-formdata-event.html

• html/HTMLFormElement.cpp:

(WebCore::HTMLFormElement::submit):

LayoutTests:

Add layout test coverage.

• fast/forms/remove-form-inside-formdata-event-expected.txt: Added.
• fast/forms/remove-form-inside-formdata-event.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/forms/remove-form-inside-formdata-event-expected.txt (added)
• LayoutTests/fast/forms/remove-form-inside-formdata-event.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/HTMLFormElement.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-10-21  Chris Dumez  <cdumez@apple.com>
+
+        Form submission should be cancelled if the form gets detached from inside the formdata event handler
+        https://bugs.webkit.org/show_bug.cgi?id=232114
+
+        Reviewed by Alex Christensen.
+
+        Add layout test coverage.
+
+        * fast/forms/remove-form-inside-formdata-event-expected.txt: Added.
+        * fast/forms/remove-form-inside-formdata-event.html: Added.
+
 2021-10-21  Eric Hutchison  <ehutchison@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-10-21  Chris Dumez  <cdumez@apple.com>
+
+        Form submission should be cancelled if the form gets detached from inside the formdata event handler
+        https://bugs.webkit.org/show_bug.cgi?id=232114
+
+        Reviewed by Alex Christensen.
+
+        Per the HTML specification [1], form submission should abort if the form cannot navigate (which is true
+        when the form is detached). The algorithm in the specification does the check twice, once at the very
+        beginning (Step 1 in the spec), and again after calling the "constructing the entry list" algorithm
+        (step 9 in the spec). The reason we need to do the check again is that the "constructing the entry list"
+        algorithm fires the "formdata" event and may thus run JavaScript and the JS can detach the form element.
+
+        In HTMLFormElement::submit(), we were doing only the "form is connected" check only at the beginning
+        of the function and failing to do so after constructing the FormSubmission object (which ends up constructing
+        the entry list). This patch fixes that.
+
+        [1] https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#concept-form-submit
+
+        Test: fast/forms/remove-form-inside-formdata-event.html
+
+        * html/HTMLFormElement.cpp:
+        (WebCore::HTMLFormElement::submit):
+
 2021-10-21  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/html/HTMLFormElement.cpp

@@ -403 +403 @@
     auto shouldLockHistory = processingUserGesture ? LockHistory::No : LockHistory::Yes;
     auto formSubmission = FormSubmission::create(*this, submitter, m_attributes, event, shouldLockHistory, trigger);
+
+    if (!isConnected())
+        return;
+
     if (m_plannedFormSubmission)
         m_plannedFormSubmission->cancel();