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.

Sign up for GitHub

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

Jump to bottom

Handle non server action post requests safely #60526

Merged
merged 2 commits into from
Jan 11, 2024
Merged

Handle non server action post requests safely #60526

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c23e4c
Handle non server action post requests safely
huozhi Jan 11, 2024
f02c2ac
apply to edge
huozhi Jan 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions packages/next/src/server/app-render/action-handler.ts
View file
Open in desktop
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 @@ -323,14 +324,13 @@ export async function handleAction({
}
: undefined

let warning: string | undefined = undefined
// 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 @@ -498,8 +498,15 @@ export async function handleAction({
})
const formData = await fakeRequest.formData()
const action = await decodeAction(formData, serverModuleMap)
const actionReturnedState = await action()
formState = await decodeFormState(actionReturnedState, formData)
// Only
huozhi marked this conversation as resolved.
Show resolved Hide resolved
huozhi marked this conversation as resolved.
Show resolved Hide resolved
if (typeof action === 'function') {
// Only warn if it's a server action, otherwise skip for other post requests
if (warning) {
warn(warning)
huozhi marked this conversation as resolved.
Show resolved Hide resolved
}
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
View file
Open in desktop
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
Loading