From 7612ce3ed52df226628e66e69b33836998514774 Mon Sep 17 00:00:00 2001
From: Andrew Clark <git@andrewclark.io>
Date: Thu, 9 May 2024 12:43:18 -0400
Subject: [PATCH] Fix: Coerce form submitter action prop

When handling form action functions, we should coerce the submitter
formAction prop using the same logic as ReactDOMComponent before
deciding whether it should override the action on the <form>.

Before this fix, if the submitter's `formAction` prop was a symbol or
a boolean, it would block the form's action function from firing, but
the correct behavior is that symbols and booleans should be treated as
if they were null/empty, because that's how we treat those values when
setting DOM properties.
---
 .../events/plugins/FormActionEventPlugin.js   | 35 ++++++++++++--
 .../src/__tests__/ReactDOMForm-test.js        | 47 +++++++++++++++++++
 2 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js b/packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js
index 8f874e2cdb8c4..b186599ca17ec 100644
--- a/packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js
+++ b/packages/react-dom-bindings/src/events/plugins/FormActionEventPlugin.js
@@ -14,11 +14,36 @@ import type {EventSystemFlags} from '../EventSystemFlags';
 import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
 import type {FormStatus} from 'react-dom-bindings/src/shared/ReactDOMFormActions';
 
+import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags';
 import {getFiberCurrentPropsFromNode} from '../../client/ReactDOMComponentTree';
 import {startHostTransition} from 'react-reconciler/src/ReactFiberReconciler';
+import sanitizeURL from 'react-dom-bindings/src/shared/sanitizeURL';
+import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';
 
 import {SyntheticEvent} from '../SyntheticEvent';
 
+function coerceFormActionProp(
+  actionProp: mixed,
+): string | (FormData => void | Promise<void>) | null {
+  // This should match the logic in ReactDOMComponent
+  if (
+    actionProp == null ||
+    typeof actionProp === 'symbol' ||
+    typeof actionProp === 'boolean'
+  ) {
+    return null;
+  } else if (typeof actionProp === 'function') {
+    return (actionProp: any);
+  } else {
+    if (__DEV__) {
+      checkAttributeStringCoercion(actionProp, 'action');
+    }
+    return (sanitizeURL(
+      enableTrustedTypesIntegration ? actionProp : '' + (actionProp: any),
+    ): any);
+  }
+}
+
 /**
  * This plugin invokes action functions on forms, inputs and buttons if
  * the form doesn't prevent default.
@@ -48,10 +73,12 @@ function extractEvents(
   let submitterAction;
   if (submitter) {
     const submitterProps = getFiberCurrentPropsFromNode(submitter);
-    submitterAction = submitterProps
-      ? (submitterProps: any).formAction
-      : submitter.getAttribute('formAction');
-    if (submitterAction != null) {
+    submitterAction = coerceFormActionProp(
+      submitterProps
+        ? (submitterProps: any).formAction
+        : submitter.getAttribute('formAction'),
+    );
+    if (submitterAction !== null) {
       // The submitter overrides the form action.
       action = submitterAction;
       // If the action is a function, we don't want to pass its name
diff --git a/packages/react-dom/src/__tests__/ReactDOMForm-test.js b/packages/react-dom/src/__tests__/ReactDOMForm-test.js
index 5db3b2fd8a5be..b8ca871c9b14b 100644
--- a/packages/react-dom/src/__tests__/ReactDOMForm-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMForm-test.js
@@ -35,6 +35,7 @@ describe('ReactDOMForm', () => {
   let ReactDOMClient;
   let Scheduler;
   let assertLog;
+  let assertConsoleErrorDev;
   let waitForThrow;
   let useState;
   let Suspense;
@@ -54,6 +55,8 @@ describe('ReactDOMForm', () => {
     act = require('internal-test-utils').act;
     assertLog = require('internal-test-utils').assertLog;
     waitForThrow = require('internal-test-utils').waitForThrow;
+    assertConsoleErrorDev =
+      require('internal-test-utils').assertConsoleErrorDev;
     useState = React.useState;
     Suspense = React.Suspense;
     startTransition = React.startTransition;
@@ -1782,4 +1785,48 @@ describe('ReactDOMForm', () => {
     // The form was reset even though the action didn't finish.
     expect(inputRef.current.value).toBe('Initial');
   });
+
+  test("regression: submitter's formAction prop is coerced correctly before checking if it exists", async () => {
+    function App({submitterAction}) {
+      return (
+        <form action={() => Scheduler.log('Form action')}>
+          <button ref={buttonRef} type="submit" formAction={submitterAction} />
+        </form>
+      );
+    }
+
+    const buttonRef = React.createRef();
+    const root = ReactDOMClient.createRoot(container);
+
+    await act(() =>
+      root.render(
+        <App submitterAction={() => Scheduler.log('Button action')} />,
+      ),
+    );
+    await submit(buttonRef.current);
+    assertLog(['Button action']);
+
+    // When there's no button action, the form action should fire
+    await act(() => root.render(<App submitterAction={null} />));
+    await submit(buttonRef.current);
+    assertLog(['Form action']);
+
+    // Symbols are coerced to null, so this should fire the form action
+    await act(() => root.render(<App submitterAction={Symbol()} />));
+    assertConsoleErrorDev(['Invalid value for prop `formAction`']);
+    await submit(buttonRef.current);
+    assertLog(['Form action']);
+
+    // Booleans are coerced to null, so this should fire the form action
+    await act(() => root.render(<App submitterAction={true} />));
+    await submit(buttonRef.current);
+    assertLog(['Form action']);
+
+    // A string on the submitter should prevent the form action from firing
+    // and trigger the native behavior
+    await act(() => root.render(<App submitterAction="https://react.dev/" />));
+    await expect(submit(buttonRef.current)).rejects.toThrow(
+      'Navigate to: https://react.dev/',
+    );
+  });
 });