Skip to content

Commit

Permalink
Handle non server action post requests safely (#60526)
Browse files Browse the repository at this point in the history
When sending post requests but it's not server action, skip logging
warning or calling non-existed server action. Instead we only log the
warning like missnig headers for server actions when it's a server
action and call the action handler when it's decoded as a function

Fixes #58152 
Closes NEXT-1761
  • Loading branch information
huozhi authored Jan 11, 2024
1 parent a1d0259 commit 9b7a5c0
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 7 deletions.
28 changes: 21 additions & 7 deletions packages/next/src/server/app-render/action-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
getServerActionRequestMetadata,
} from '../lib/server-action-request-meta'
import { isCsrfOriginAllowed } from './csrf-protection'
import { warn } from '../../build/output/log'

function formDataFromSearchQueryString(query: string) {
const searchParams = new URLSearchParams(query)
Expand Down Expand Up @@ -324,14 +325,19 @@ export async function handleAction({
}
: undefined

let warning: string | undefined = undefined

function warnBadServerActionRequest() {
if (warning) {
warn(warning)
}
}
// This is to prevent CSRF attacks. If `x-forwarded-host` is set, we need to
// ensure that the request is coming from the same host.
if (!originDomain) {
// This might be an old browser that doesn't send `host` header. We ignore
// this case.
console.warn(
'Missing `origin` header from a forwarded Server Actions request.'
)
warning = 'Missing `origin` header from a forwarded Server Actions request.'
} else if (!host || originDomain !== host.value) {
// If the customer sets a list of allowed origins, we'll allow the request.
// These are considered safe but might be different from forwarded host set
Expand Down Expand Up @@ -421,8 +427,12 @@ export async function handleAction({
bound = await decodeReply(formData, serverModuleMap)
} else {
const action = await decodeAction(formData, serverModuleMap)
const actionReturnedState = await action()
formState = decodeFormState(actionReturnedState, formData)
if (typeof action === 'function') {
// Only warn if it's a server action, otherwise skip for other post requests
warnBadServerActionRequest()
const actionReturnedState = await action()
formState = decodeFormState(actionReturnedState, formData)
}

// Skip the fetch path
return
Expand Down Expand Up @@ -499,8 +509,12 @@ export async function handleAction({
})
const formData = await fakeRequest.formData()
const action = await decodeAction(formData, serverModuleMap)
const actionReturnedState = await action()
formState = await decodeFormState(actionReturnedState, formData)
if (typeof action === 'function') {
// Only warn if it's a server action, otherwise skip for other post requests
warnBadServerActionRequest()
const actionReturnedState = await action()
formState = await decodeFormState(actionReturnedState, formData)
}

// Skip the fetch path
return
Expand Down
7 changes: 7 additions & 0 deletions test/e2e/app-dir/actions/app-action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ createNextDescribe(
})

it('should 404 when POSTing an invalid server action', async () => {
const cliOutputPosition = next.cliOutput.length
const res = await next.fetch('/non-existent-route', {
method: 'POST',
headers: {
Expand All @@ -418,6 +419,12 @@ createNextDescribe(
body: 'foo=bar',
})

const cliOutput = next.cliOutput.slice(cliOutputPosition)

expect(cliOutput).not.toContain('TypeError')
expect(cliOutput).not.toContain(
'Missing `origin` header from a forwarded Server Actions request'
)
expect(res.status).toBe(404)
})

Expand Down

0 comments on commit 9b7a5c0

Please sign in to comment.