Skip to content

Fix/action forward loop guard#84525

Closed
LiamBoz wants to merge 2 commits into
vercel:canaryfrom
LiamBoz:fix/action-forward-loop-guard
Closed

Fix/action forward loop guard#84525
LiamBoz wants to merge 2 commits into
vercel:canaryfrom
LiamBoz:fix/action-forward-loop-guard

Conversation

@LiamBoz

@LiamBoz LiamBoz commented Oct 5, 2025

Copy link
Copy Markdown
Contributor

What?

Fixes #84504

Add a minimal loop-detection guard to createForwardedActionResponse so a Server Action request that has already been forwarded (marked by x-action-forwarded=1) is not forwarded again. Instead, we return early (no second forward).

  • File: packages/next/src/server/app-render/action-handler.ts
  • Behavior change: if x-action-forwarded=1 is present on the incoming request, return from createForwardedActionResponse without forwarding again.

Why?

In apps that use Middleware rewrites, a forwarded Server Action request can be rewritten to a path that triggers forwarding again, causing an infinite loop (e.g. /rewrite/other → /rewrite/... → /rewrite/rewrite/... → …). This leads to repeated logs and a server action that never resolves.

This guard breaks the cycle with very low risk by only affecting requests that were already forwarded once.

How?

At the start of createForwardedActionResponse, inspect the incoming headers:

  • If x-action-forwarded=1 is present → return (do not forward again).
  • Otherwise, proceed as before and set x-action-forwarded=1 on the outgoing forwarded request.

Works in both Node and Edge:

  • If req.headers is already a Headers (Edge), use it.
  • Otherwise adapt Node headers via HeadersAdapter.

Test Plan

Manual (using the public repro)

  • With this patch built and packed into the repro app:
    • Trigger the fast Run action → Navigate sequence.
    • Confirm:
      • No repeated /rewrite/... requests or log spam.
      • The page no longer gets stuck due to an infinite forwarding loop.

Notes: This PR intentionally focuses on loop prevention. A broader follow-up could make forwarded requests mirror the pre-rewrite target to ensure resolution semantics remain identical under rewrites. That would be a separate change.

Risk

Low:

  • Only affects requests that already bear x-action-forwarded=1.
  • Does not change the initial legitimate forward.
  • Keeps all other server action behavior intact.

For Contributors Checklist

  • Bug fix
  • Related issue linked (Fixes #84504)
  • Repro steps included (public repo)
  • Ran pnpm prettier-fix locally
  • Tests: this is a minimal guard; happy to add a small e2e that simulates a rewrite + quick navigation if maintainers prefer (guidance welcome)

@ijjk ijjk added the type: next label Oct 5, 2025
@ijjk

ijjk commented Oct 5, 2025

Copy link
Copy Markdown
Member

Allow CI Workflow Run

  • approve CI run for commit: 2c68259

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

elishagreenwald added a commit to elishagreenwald/next.js that referenced this pull request May 8, 2026
Avoid infinite peer-forward loops when middleware rewrites paths or the
request was already forwarded (x-action-forwarded). The receiving worker
short-circuits the forwarding decision and falls through to lenient
serverModuleMap resolution instead of forwarding again.

Adds a self-contained e2e regression test that reproduces the loop via a
proxy.ts rewrite: with the fix the request resolves in ~150ms; without
the fix it hangs until the test's 10s abort timer fires.

Fixes vercel#84504
Supersedes vercel#84525, vercel#92053
unstubbable pushed a commit to elishagreenwald/next.js that referenced this pull request May 12, 2026
Avoid infinite peer-forward loops when middleware rewrites paths or the
request was already forwarded (x-action-forwarded). The receiving worker
short-circuits the forwarding decision and falls through to lenient
serverModuleMap resolution instead of forwarding again.

Adds a self-contained e2e regression test that reproduces the loop via a
proxy.ts rewrite: with the fix the request resolves in ~150ms; without
the fix it hangs until the test's 10s abort timer fires.

Fixes vercel#84504
Supersedes vercel#84525, vercel#92053
unstubbable added a commit that referenced this pull request May 18, 2026
Reopens #93710 from a branch on this repo so the required deploy test
matrix can run — GitHub Actions doesn't expose secrets to fork PRs, and
those deploy jobs are required checks. All commits and credit carry
forward from the original PR by @elishagreenwald, which built on earlier
attempts by @LiamBoz in #84525 and @claygeo in #92053.

When middleware rewrites a Server Action POST to a page that doesn't
bundle the action, the receiving worker forwards the request to a worker
that does. The forwarded request hits middleware again and gets
rewritten the same way, so the new receiving worker forwards again —
looping until undici's headers timeout (~300s) or memory pressure brings
the server down (#84504). The fix is two related changes on the
forwarding code path. The first is an `!actionWasForwarded` guard in
`action-handler.ts` so a request carrying `x-action-forwarded: 1` is
never forwarded a second time. The second is a new branch in
`createForwardedActionResponse` that copies the
`x-nextjs-action-not-found` header and 404 status onto the originating
response when the forwarded worker can't find the action either; without
it the client would see a generic "unexpected response" error instead of
the `UnrecognizedActionError` that the rest of the framework expects.

The e2e test at `test/e2e/app-dir/action-forward-loop` reproduces the
scenario. A `proxy.ts` rewrites POSTs to `/with-action` so GETs still
render the page normally; the page renders a `<form>` with an inline
server action wrapped in a React error boundary that branches on
`unstable_isUnrecognizedActionError`. The test clicks the button and
waits for `#action-not-found-error`. Without the loop guard the test
times out at 10s, and without the pass-through the boundary catches a
generic error and the assertion fails — so both changes are individually
load-bearing.

This is a short-term fix. The mid-term direction is to remove
server-side action forwarding altogether and have the client dispatch
each Server Action to the route that actually bundles it, which makes
the entire forwarding code path (and this bug surface) unnecessary.
#90549 is the draft exploring that approach.

Fixes #84504

Closes #93710
Closes #84525
Closes #92053

---------

Co-Authored-By: Elisha Greenwald <ejgreenwald@gmail.com>
Co-Authored-By: LiamBoz <thisamazingnow@gmail.com>
Co-Authored-By: kmaclip <kmartclips@proton.me>
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.

Forwarded server actions with middleware rewrites cause an infinite request loop

2 participants