Skip to content

Commit

Permalink
fix(actions): better runtime check for invalid usages (#12402)
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico authored Nov 8, 2024
1 parent f5f7109 commit 823e73b
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 5 deletions.
14 changes: 14 additions & 0 deletions .changeset/dull-lemons-check.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
'astro': patch
---

Fixes a case where Astro allowed to call an action without using `Astro.callAction`. This is now invalid, and Astro will show a proper error.

```diff
---
import { actions } from "astro:actions";

-const result = actions.getUser({ userId: 123 });
+const result = Astro.callAction(actions.getUser, { userId: 123 });
---
```
3 changes: 2 additions & 1 deletion packages/astro/src/actions/runtime/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { APIContext, MiddlewareNext } from '../../@types/astro.js';
import { defineMiddleware } from '../../core/middleware/index.js';
import { getOriginPathname } from '../../core/routing/rewrite.js';
import { ACTION_QUERY_PARAMS } from '../consts.js';
import { formContentTypes, hasContentType } from './utils.js';
import { ACTION_API_CONTEXT_SYMBOL, formContentTypes, hasContentType } from './utils.js';
import { getAction } from './virtual/get-action.js';
import {
type SafeResult,
Expand Down Expand Up @@ -100,6 +100,7 @@ async function handlePost({
formData = await request.clone().formData();
}
const { getActionResult, callAction, props, redirect, ...actionAPIContext } = context;
Reflect.set(actionAPIContext, ACTION_API_CONTEXT_SYMBOL, true);
const action = baseAction.bind(actionAPIContext);
const actionResult = await action(formData);

Expand Down
3 changes: 2 additions & 1 deletion packages/astro/src/actions/runtime/route.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { APIRoute } from '../../@types/astro.js';
import { formContentTypes, hasContentType } from './utils.js';
import { ACTION_API_CONTEXT_SYMBOL, formContentTypes, hasContentType } from './utils.js';
import { getAction } from './virtual/get-action.js';
import { serializeActionResult } from './virtual/shared.js';

Expand Down Expand Up @@ -28,6 +28,7 @@ export const POST: APIRoute = async (context) => {
return new Response(null, { status: 415 });
}
const { getActionResult, callAction, props, redirect, ...actionAPIContext } = context;
Reflect.set(actionAPIContext, ACTION_API_CONTEXT_SYMBOL, true);
const action = baseAction.bind(actionAPIContext);
const result = await action(args);
const serialized = serializeActionResult(result);
Expand Down
7 changes: 7 additions & 0 deletions packages/astro/src/actions/runtime/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type { APIContext } from '../../@types/astro.js';

export const ACTION_API_CONTEXT_SYMBOL = Symbol.for('astro.actionAPIContext');

export const formContentTypes = ['application/x-www-form-urlencoded', 'multipart/form-data'];

export function hasContentType(contentType: string, expected: string[]) {
Expand All @@ -26,3 +28,8 @@ export type MaybePromise<T> = T | Promise<T>;
* `result.error.fields` will be typed with the `name` field.
*/
export type ErrorInferenceObject = Record<string, any>;

export function isActionAPIContext(ctx: ActionAPIContext): boolean {
const symbol = Reflect.get(ctx, ACTION_API_CONTEXT_SYMBOL);
return symbol === true;
}
10 changes: 8 additions & 2 deletions packages/astro/src/actions/runtime/virtual/server.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { z } from 'zod';
import { ActionCalledFromServerError } from '../../../core/errors/errors-data.js';
import { AstroError } from '../../../core/errors/errors.js';
import type { ActionAPIContext, ErrorInferenceObject, MaybePromise } from '../utils.js';
import {
type ActionAPIContext,
type ErrorInferenceObject,
type MaybePromise,
isActionAPIContext,
} from '../utils.js';
import { ActionError, ActionInputError, type SafeResult, callSafely } from './shared.js';

export * from './shared.js';
Expand Down Expand Up @@ -60,7 +65,8 @@ export function defineAction<
: getJsonServerHandler(handler, inputSchema);

async function safeServerHandler(this: ActionAPIContext, unparsedInput: unknown) {
if (typeof this === 'function') {
// The ActionAPIContext should always contain the `params` property
if (typeof this === 'function' || !isActionAPIContext(this)) {
throw new AstroError(ActionCalledFromServerError);
}
return callSafely(() => serverHandler(unparsedInput, this));
Expand Down
3 changes: 2 additions & 1 deletion packages/astro/src/actions/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type fsMod from 'node:fs';
import * as eslexer from 'es-module-lexer';
import type { APIContext } from '../@types/astro.js';
import type { Locals } from './runtime/middleware.js';
import type { ActionAPIContext } from './runtime/utils.js';
import { ACTION_API_CONTEXT_SYMBOL, type ActionAPIContext } from './runtime/utils.js';
import { deserializeActionResult, getActionQueryString } from './runtime/virtual/shared.js';

export function hasActionPayload(locals: APIContext['locals']): locals is Locals {
Expand All @@ -23,6 +23,7 @@ export function createGetActionResult(locals: APIContext['locals']): APIContext[

export function createCallAction(context: ActionAPIContext): APIContext['callAction'] {
return (baseAction, input) => {
Reflect.set(context, ACTION_API_CONTEXT_SYMBOL, true);
const action = baseAction.bind(context);
return action(input) as any;
};
Expand Down
6 changes: 6 additions & 0 deletions packages/astro/test/actions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,12 @@ describe('Astro Actions', () => {
assert.equal(data, 'Hello, ben!');
}
});

it('Should fail when calling an action without using Astro.callAction', async () => {
const res = await fixture.fetch('/invalid/');
const text = await res.text();
assert.match(text, /ActionCalledFromServerError/);
});
});

describe('build', () => {
Expand Down
6 changes: 6 additions & 0 deletions packages/astro/test/fixtures/actions/src/pages/invalid.astro
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
import { actions } from "astro:actions";
// this is invalid, it should fail
const result = await actions.imageUploadInChunks();
---

0 comments on commit 823e73b

Please sign in to comment.