verify action id before parsing body

packages/next/src/server/app-render/action-handler.ts

@@ -238,6 +238,16 @@ function limitUntrustedHeaderValueForLogs(value: string) {
   return value.length > 100 ? value.slice(0, 100) + '...' : value
 }
 
+type ServerModuleMap = Record<
+  string,
+  | {
+      id: string
+      chunks: string[]
+      name: string
+    }
+  | undefined
+>
+
 export async function handleAction({
   req,
   res,
@@ -252,13 +262,7 @@ export async function handleAction({
   req: IncomingMessage
   res: ServerResponse
   ComponentMod: AppPageModule
-  serverModuleMap: {
-    [id: string]: {
-      id: string
-      chunks: string[]
-      name: string
-    }
-  }
+  serverModuleMap: ServerModuleMap
   generateFlight: GenerateFlight
   staticGenerationStore: StaticGenerationStore
   requestStore: RequestStore
@@ -392,6 +396,7 @@ export async function handleAction({
 
   let actionResult: RenderResult | undefined
   let formState: any | undefined
+  let actionModId: string | undefined
 
   try {
     await actionAsyncStorage.run({ isAction: true }, async () => {
@@ -418,6 +423,15 @@ export async function handleAction({
             return
           }
         } else {
+          try {
+            actionModId = getActionModIdOrError(actionId, serverModuleMap)
+          } catch (err) {
+            console.error(err)
+            return {
+              type: 'not-found',
+            }
+          }
+
           let actionData = ''
 
           const reader = webRequest.body.getReader()
@@ -487,6 +501,15 @@ export async function handleAction({
             return
           }
         } else {
+          try {
+            actionModId = getActionModIdOrError(actionId, serverModuleMap)
+          } catch (err) {
+            console.error(err)
+            return {
+              type: 'not-found',
+            }
+          }
+
           const chunks = []
 
           for await (const chunk of req) {
@@ -528,27 +551,22 @@ To configure the body size limit for Server Actions, see: https://nextjs.org/doc
       // / -> fire action -> POST / -> appRender1 -> modId for the action file
       // /foo -> fire action -> POST /foo -> appRender2 -> modId for the action file
 
-      let actionModId: string
       try {
-        if (!actionId) {
-          throw new Error('Invariant: actionId should be set')
-        }
-
-        actionModId = serverModuleMap[actionId].id
+        actionModId =
+          actionModId ?? getActionModIdOrError(actionId, serverModuleMap)
       } catch (err) {
-        // When this happens, it could be a deployment skew where the action came
-        // from a different deployment. We'll just return a 404 with a message logged.
-        console.error(
-          `Failed to find Server Action "${actionId}". This request might be from an older or newer deployment.`
-        )
+        console.error(err)
         return {
           type: 'not-found',
         }
       }
 
       const actionHandler = (
         await ComponentMod.__next_app__.require(actionModId)
-      )[actionId]
+      )[
+        // `actionId` must exist if we got here, as otherwise we would have thrown an error above
+        actionId!
+      ]
 
       const returnVal = await actionHandler.apply(null, bound)
 
@@ -675,3 +693,36 @@ To configure the body size limit for Server Actions, see: https://nextjs.org/doc
     throw err
   }
 }
+
+/**
+ * Attempts to find the module ID for the action from the module map. When this fails, it could be a deployment skew where
+ * the action came from a different deployment. It could also simply be an invalid POST request that is not a server action.
+ * In either case, we'll throw an error to be handled by the caller.
+ */
+function getActionModIdOrError(
+  actionId: string | null,
+  serverModuleMap: ServerModuleMap
+): string {
+  try {
+    // if we're missing the action ID header, we can't do any further processing
+    if (!actionId) {
+      throw new Error("Invariant: Missing 'next-action' header.")
+    }
+
+    const actionModId = serverModuleMap?.[actionId]?.id
+
+    if (!actionModId) {
+      throw new Error(
+        "Invariant: Couldn't find action module ID from module map."
+      )
+    }
+
+    return actionModId
+  } catch (err) {
+    throw new Error(
+      `Failed to find Server Action "${actionId}". This request might be from an older or newer deployment. ${
+        err instanceof Error ? `Original error: ${err.message}` : ''
+      }`
+    )
+  }
+}

test/e2e/app-dir/actions/app-action.test.ts

@@ -409,6 +409,34 @@ createNextDescribe(
       )
     })
 
+    it('should 404 when POSTing an invalid server action', async () => {
+      const res = await next.fetch('/non-existent-route', {
+        method: 'POST',
+        headers: {
+          'content-type': 'application/x-www-form-urlencoded',
+        },
+        body: 'foo=bar',
+      })
+
+      expect(res.status).toBe(404)
+    })
+
+    it('should log a warning when a server action is not found but an id is provided', async () => {
+      await next.fetch('/server', {
+        method: 'POST',
+        headers: {
+          'content-type': 'application/x-www-form-urlencoded',
+          'next-action': 'abc123',
+        },
+        body: 'foo=bar',
+      })
+
+      await check(
+        () => next.cliOutput,
+        /Failed to find Server Action "abc123". This request might be from an older or newer deployment./
+      )
+    })
+
     if (isNextStart) {
       it('should not expose action content in sourcemaps', async () => {
         const sourcemap = (
@@ -972,6 +1000,46 @@ createNextDescribe(
         expect(postRequests).toEqual(['/redirects/api-redirect-permanent'])
         expect(responseCodes).toEqual([303])
       })
+
+      it.each(['307', '308'])(
+        `redirects properly when server action handler redirects with a %s status code`,
+        async (statusCode) => {
+          const postRequests = []
+          const responseCodes = []
+
+          const browser = await next.browser('/redirects', {
+            beforePageLoad(page) {
+              page.on('request', (request: Request) => {
+                const url = new URL(request.url())
+                if (request.method() === 'POST') {
+                  postRequests.push(`${url.pathname}${url.search}`)
+                }
+              })
+
+              page.on('response', (response: Response) => {
+                const url = new URL(response.url())
+                const status = response.status()
+
+                if (postRequests.includes(`${url.pathname}${url.search}`)) {
+                  responseCodes.push(status)
+                }
+              })
+            },
+          })
+
+          await browser.elementById(`submit-api-redirect-${statusCode}`).click()
+          await check(() => browser.url(), /success=true/)
+          expect(await browser.elementById('redirect-page')).toBeTruthy()
+
+          // since a 307/308 status code follows the redirect, the POST request should be made to both the action handler and the redirect target
+          expect(postRequests).toEqual([
+            `/redirects/api-redirect-${statusCode}`,
+            `/redirects?success=true`,
+          ])
+
+          expect(responseCodes).toEqual([Number(statusCode), 200])
+        }
+      )
     })
   }
 )

test/e2e/app-dir/actions/app/redirects/api-redirect-307/route.js

@@ -0,0 +1,6 @@
+export function POST(request) {
+  return Response.redirect(
+    `${request.nextUrl.origin}/redirects?success=true`,
+    307
+  )
+}

test/e2e/app-dir/actions/app/redirects/api-redirect-308/route.js

@@ -0,0 +1,6 @@
+export function POST(request) {
+  return Response.redirect(
+    `${request.nextUrl.origin}/redirects?success=true`,
+    308
+  )
+}

test/e2e/app-dir/actions/app/redirects/page.js

@@ -13,6 +13,14 @@ export default function Home() {
           id="submit-api-redirect-permanent"
         />
       </form>
+      <h1>POST /api-reponse-redirect-307</h1>
+      <form action="/redirects/api-redirect-307" method="POST">
+        <input type="submit" value="Submit" id="submit-api-redirect-307" />
+      </form>
+      <h1>POST /api-reponse-redirect-308</h1>
+      <form action="/redirects/api-redirect-308" method="POST">
+        <input type="submit" value="Submit" id="submit-api-redirect-308" />
+      </form>
     </main>
   )
 }