Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a bug that could allow duplicate form submissions #19649

Merged
merged 1 commit into from Oct 17, 2019

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Oct 11, 2019

Previous to this CL, and after [1], if a form Submit button had an
onclick handler that also called form.submit() and did not call
event.preventDefault(), the form would get submitted twice. The second
request was eventually cancelled, but not before it hit the network.
This behavior is specified in [2], through the "plan to navigate"
mechanism. In the case of this bug, the "click" event occurs first,
and changes the "planned navigation". Then, because the click handler
does not preventDefault(), the default submit action is executed,
which changes the "planned navigation", replacing the navigation
queued by the onclick handler. Therefore, only the default submit
navigation is performed.

Note that there are other potential interactions which are less
clearly specified, and which are not addressed in this CL.
For example:

<iframe id="test" name="test"></iframe>
<form id=form1 target="test" action="click.html"></form>
<a target="test" onclick="form1.submit()" href="href.html">Test</a>

In this case, clicking the <a> link first submits the form (to
click.html), and then queues a navigation to href.html. Because
the navigation to href.html is specified (in [3]) to "queue a
navigation", independently of the planned navigation specified
in [2], it is unclear when/whether the form submission should
take place. The spec ([4]) does have provisions for canceling
existing navigations, but that leaves room for the form to still
get to the network in this case, before getting canceled.

[1] https://chromium.googlesource.com/chromium/src/+/6931ab86f19aa79abbdd0c1062084e16b5c4f0f6
[2] https://www.w3.org/TR/html52/sec-forms.html#form-submission-algorithm
[3] https://html.spec.whatwg.org/#following-hyperlinks
[4] https://html.spec.whatwg.org/#navigating-across-documents

Bug: 977882
Change-Id: I693f3bdccb17c5e64df75c2e569fab589c02e88c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1850358
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706782}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already reviewed downstream.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1850358 branch 5 times, most recently from 267f45c to 770ff38 Compare October 17, 2019 02:56
Previous to this CL, and after [1], if a form Submit button had an
onclick handler that also called form.submit() and did not call
event.preventDefault(), the form would get submitted twice. The second
request was eventually cancelled, but not before it hit the network.
This behavior is specified in [2], through the "plan to navigate"
mechanism. In the case of this bug, the "click" event occurs first,
and changes the "planned navigation". Then, because the click handler
does not preventDefault(), the default submit action is executed,
which changes the "planned navigation", replacing the navigation
queued by the onclick handler. Therefore, only the default submit
navigation is performed.

Note that there are other potential interactions which are less
clearly specified, and which are not addressed in this CL.
For example:

  <iframe id="test" name="test"></iframe>
  <form id=form1 target="test" action="click.html"></form>
  <a target="test" onclick="form1.submit()" href="href.html">Test</a>

In this case, clicking the <a> link first submits the form (to
click.html), and then queues a navigation to href.html. Because
the navigation to href.html is specified (in [3]) to "queue a
navigation", independently of the planned navigation specified
in [2], it is unclear when/whether the form submission should
take place. The spec ([4]) does have provisions for canceling
existing navigations, but that leaves room for the form to still
get to the network in this case, before getting canceled.

[1] https://chromium.googlesource.com/chromium/src/+/6931ab86f19aa79abbdd0c1062084e16b5c4f0f6
[2] https://www.w3.org/TR/html52/sec-forms.html#form-submission-algorithm
[3] https://html.spec.whatwg.org/#following-hyperlinks
[4] https://html.spec.whatwg.org/#navigating-across-documents

Bug: 977882
Change-Id: I693f3bdccb17c5e64df75c2e569fab589c02e88c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1850358
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706782}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants