Skip to content

Commit 95e878d

Browse files
KocalKocal
committed
[LiveComponent] Cap the number of actions per _batch request
`BatchActionController::__invoke()` iterated over the client-supplied `actions` array and issued a full `HttpKernel` sub-request (subscribers, validators, Doctrine, rendering) for each entry, with no upper bound. An authenticated client could submit a payload with thousands of actions in a single request and exhaust CPU, memory, and database connections. Add a `MAX_ACTIONS_PER_BATCH = 50` cap (well above the 3-action volume the bundle's own tests exercise) and reject larger payloads up front with `BadRequestHttpException`.
1 parent a6e9bf8 commit 95e878d

5 files changed

Lines changed: 187 additions & 6 deletions

File tree

‎src/LiveComponent/assets/dist/live_controller.js‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1494,9 +1494,11 @@ var Component = class {
14941494
this.unsyncedInputsTracker.resetUnsyncedFields();
14951495
const filesToSend = {};
14961496
for (const [key, value] of Object.entries(this.pendingFiles)) if (value.files) filesToSend[key] = value.files;
1497+
const actionsToSend = this.pendingActions.slice(0, 50);
1498+
const remainingActions = this.pendingActions.slice(50);
14971499
const requestConfig = {
14981500
props: this.valueStore.getOriginalProps(),
1499-
actions: this.pendingActions,
1501+
actions: actionsToSend,
15001502
updated: this.valueStore.getDirtyProps(),
15011503
children: {},
15021504
updatedPropsFromParent: this.valueStore.getUpdatedPropsFromParent(),
@@ -1505,9 +1507,9 @@ var Component = class {
15051507
this.hooks.triggerHook("request:started", requestConfig);
15061508
this.backendRequest = this.backend.makeRequest(requestConfig.props, requestConfig.actions, requestConfig.updated, requestConfig.children, requestConfig.updatedPropsFromParent, requestConfig.files);
15071509
this.hooks.triggerHook("loading.state:started", this.element, this.backendRequest);
1508-
this.pendingActions = [];
1510+
this.pendingActions = remainingActions;
15091511
this.valueStore.flushDirtyPropsToPending();
1510-
this.isRequestPending = false;
1512+
this.isRequestPending = remainingActions.length > 0;
15111513
this.backendRequest.promise.then(async (response) => {
15121514
const backendResponse = new BackendResponse_default(response);
15131515
const html = await backendResponse.getBody();

‎src/LiveComponent/assets/src/Component/index.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ import ValueStore from './ValueStore';
1414

1515
declare const Turbo: any;
1616

17+
// Must match BatchActionController::MAX_ACTIONS_PER_BATCH on the PHP side.
18+
export const MAX_ACTIONS_PER_BATCH = 50;
19+
1720
type MaybePromise<T = void> = T | Promise<T>;
1821

1922
export type ComponentHooks = {
@@ -275,9 +278,14 @@ export default class Component {
275278
}
276279
}
277280

281+
// Cap each batch at MAX_ACTIONS_PER_BATCH; the overflow stays queued and
282+
// ships in a follow-up request via the isRequestPending mechanism below.
283+
const actionsToSend = this.pendingActions.slice(0, MAX_ACTIONS_PER_BATCH);
284+
const remainingActions = this.pendingActions.slice(MAX_ACTIONS_PER_BATCH);
285+
278286
const requestConfig = {
279287
props: this.valueStore.getOriginalProps(),
280-
actions: this.pendingActions,
288+
actions: actionsToSend,
281289
updated: this.valueStore.getDirtyProps(),
282290
children: {},
283291
updatedPropsFromParent: this.valueStore.getUpdatedPropsFromParent(),
@@ -294,9 +302,9 @@ export default class Component {
294302
);
295303
this.hooks.triggerHook('loading.state:started', this.element, this.backendRequest);
296304

297-
this.pendingActions = [];
305+
this.pendingActions = remainingActions;
298306
this.valueStore.flushDirtyPropsToPending();
299-
this.isRequestPending = false;
307+
this.isRequestPending = remainingActions.length > 0;
300308

301309
this.backendRequest.promise.then(async (response) => {
302310
const backendResponse = new BackendResponse(response);

‎src/LiveComponent/assets/test/unit/controller/action.test.ts‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,4 +211,94 @@ describe('LiveController Action Tests', () => {
211211

212212
await waitFor(() => expect(test.element).toHaveTextContent('Component Saved!'));
213213
});
214+
215+
it('caps each batch at 50 actions and ships the overflow in a follow-up request', async () => {
216+
const test = await createTest(
217+
{ count: 0 },
218+
(data: any) => `<div ${initComponent(data)}>count: ${data.count}</div>`
219+
);
220+
221+
// 51 actions queued back-to-back must be split into 50 + 1, never one oversized request.
222+
// Distinct {i: N} args on every call pin both count AND ordering: a reorder or
223+
// misplacement across batches fails the matcher (matches() uses ordered isEqual).
224+
const firstBatch = test.expectsAjaxCall().serverWillChangeProps((data: any) => {
225+
data.count = 50;
226+
});
227+
for (let i = 0; i < 50; i++) {
228+
firstBatch.expectActionCalled('save', { i });
229+
}
230+
231+
const secondBatch = test.expectsAjaxCall().serverWillChangeProps((data: any) => {
232+
data.count = 51;
233+
});
234+
secondBatch.expectActionCalled('save', { i: 50 });
235+
236+
for (let i = 0; i < 51; i++) {
237+
test.component.action('save', { i });
238+
}
239+
240+
await waitFor(() => expect(test.element).toHaveTextContent('count: 51'));
241+
});
242+
243+
it('sends a single request for an exact-fit batch of 50 actions', async () => {
244+
const test = await createTest(
245+
{ count: 0 },
246+
(data: any) => `<div ${initComponent(data)}>count: ${data.count}</div>`
247+
);
248+
249+
const batch = test.expectsAjaxCall().serverWillChangeProps((data: any) => {
250+
data.count = 50;
251+
});
252+
for (let i = 0; i < 50; i++) {
253+
batch.expectActionCalled('save', { i });
254+
}
255+
256+
for (let i = 0; i < 50; i++) {
257+
test.component.action('save', { i });
258+
}
259+
260+
await waitFor(() => expect(test.element).toHaveTextContent('count: 50'));
261+
});
262+
263+
it('caps the follow-up batch when actions queue up while a request is in-flight', async () => {
264+
const test = await createTest(
265+
{ count: 0 },
266+
(data: any) => `<div ${initComponent(data)}>count: ${data.count}</div>`
267+
);
268+
269+
// First request: a single debounced action, delayed so we can queue more while it's pending.
270+
const firstBatch = test
271+
.expectsAjaxCall()
272+
.expectActionCalled('save', { i: 0 })
273+
.delayResponse(30)
274+
.serverWillChangeProps((data: any) => {
275+
data.count = 1;
276+
});
277+
278+
// Second request: indices 1..50 (cap exactly hit).
279+
const secondBatch = test.expectsAjaxCall().serverWillChangeProps((data: any) => {
280+
data.count = 51;
281+
});
282+
for (let i = 1; i <= 50; i++) {
283+
secondBatch.expectActionCalled('save', { i });
284+
}
285+
286+
// Third request: indices 51..60 (overflow from the 60 queued during the in-flight request).
287+
const thirdBatch = test.expectsAjaxCall().serverWillChangeProps((data: any) => {
288+
data.count = 61;
289+
});
290+
for (let i = 51; i <= 60; i++) {
291+
thirdBatch.expectActionCalled('save', { i });
292+
}
293+
294+
// Kick off the first request (debounce=0 → flushes immediately).
295+
test.component.action('save', { i: 0 });
296+
// Wait a tick so the first request is in-flight before we enqueue the rest.
297+
await new Promise((resolve) => setTimeout(resolve, 5));
298+
for (let i = 1; i <= 60; i++) {
299+
test.component.action('save', { i });
300+
}
301+
302+
await waitFor(() => expect(test.element).toHaveTextContent('count: 61'));
303+
});
214304
});

‎src/LiveComponent/src/Controller/BatchActionController.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,21 @@
2424
*/
2525
final class BatchActionController
2626
{
27+
/**
28+
* Must match MAX_ACTIONS_PER_BATCH on the JS side (assets/src/Component/index.ts).
29+
*/
30+
public const MAX_ACTIONS_PER_BATCH = 50;
31+
2732
public function __construct(private HttpKernelInterface $kernel)
2833
{
2934
}
3035

3136
public function __invoke(Request $request, MountedComponent $_mounted_component, string $serviceId, array $actions): ?Response
3237
{
38+
if (\count($actions) > self::MAX_ACTIONS_PER_BATCH) {
39+
throw new BadRequestHttpException('Too many actions in batch.');
40+
}
41+
3342
foreach ($actions as $action) {
3443
$name = $action['name'] ?? throw new BadRequestHttpException('Invalid JSON.');
3544

‎src/LiveComponent/tests/Functional/Controller/BatchActionControllerTest.php‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313

1414
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
1515
use Symfony\Component\DomCrawler\Crawler;
16+
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
1617
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
18+
use Symfony\UX\LiveComponent\Controller\BatchActionController;
1719
use Symfony\UX\LiveComponent\Tests\LiveComponentTestHelper;
1820
use Zenstruck\Browser\KernelBrowser;
1921
use Zenstruck\Browser\Test\HasBrowser;
@@ -261,4 +263,74 @@ public function testCannotBatchWithNonLiveAction()
261263
})
262264
;
263265
}
266+
267+
public function testAcceptsBatchAtMaxActions()
268+
{
269+
$dehydrated = $this->dehydrateComponent($this->mountComponent('with_actions'));
270+
271+
$this->browser()
272+
->throwExceptions()
273+
->post('/_components/with_actions', [
274+
'body' => [
275+
'data' => json_encode([
276+
'props' => $dehydrated->getProps(),
277+
]),
278+
],
279+
])
280+
->assertSuccessful()
281+
->use(static function (Crawler $crawler, KernelBrowser $browser) {
282+
$rootElement = $crawler->filter('ul')->first();
283+
$liveProps = json_decode($rootElement->attr('data-live-props-value'), true);
284+
285+
$actions = [];
286+
for ($i = 0; $i < BatchActionController::MAX_ACTIONS_PER_BATCH; ++$i) {
287+
$actions[] = ['name' => 'add', 'args' => ['what' => "item-$i"]];
288+
}
289+
290+
$browser->post('/_components/with_actions/_batch', [
291+
'body' => [
292+
'data' => json_encode([
293+
'props' => $liveProps,
294+
'actions' => $actions,
295+
]),
296+
],
297+
]);
298+
})
299+
->assertSuccessful()
300+
->assertSee('item-0')
301+
->assertSee('item-49')
302+
;
303+
}
304+
305+
public function testRejectsBatchAboveMaxActions()
306+
{
307+
$dehydrated = $this->dehydrateComponent($this->mountComponent('with_actions'));
308+
309+
$this->browser()
310+
->post('/_components/with_actions', [
311+
'body' => [
312+
'data' => json_encode([
313+
'props' => $dehydrated->getProps(),
314+
]),
315+
],
316+
])
317+
->assertSuccessful()
318+
->expectException(BadRequestHttpException::class, 'Too many actions in batch.')
319+
->use(static function (Crawler $crawler, KernelBrowser $browser) {
320+
$rootElement = $crawler->filter('ul')->first();
321+
$liveProps = json_decode($rootElement->attr('data-live-props-value'), true);
322+
323+
$actions = array_fill(0, BatchActionController::MAX_ACTIONS_PER_BATCH + 1, ['name' => 'add', 'args' => ['what' => 'x']]);
324+
325+
$browser->post('/_components/with_actions/_batch', [
326+
'body' => [
327+
'data' => json_encode([
328+
'props' => $liveProps,
329+
'actions' => $actions,
330+
]),
331+
],
332+
]);
333+
})
334+
;
335+
}
264336
}

0 commit comments

Comments
 (0)