Skip to content

Commit 2e0ef84

Browse files
smarcetcaseylocker
smarcet
authored and
caseylocker
committed
chore(unit-test): add unit test to demo the toctou bug
The output is .E. — tests 1 and 3 pass, test 2 (testDoubleRejection_BothReservedBeforeEitherValidates) fails with: ValidationException: Promo code DOMAIN-CODE-1 has reached the maximum of 1 tickets per account. Task A throws at SummitOrderService.php:843 (the $existingCount > $quantityPerAccount guard) (exactly the TOCTOU bug). The test asserts Task A should succeed (it's a valid first request), but the inflated count from both orders' tickets being visible causes it to reject. When the race condition is fixed, this test will start passing.
1 parent 77c3059 commit 2e0ef84

1 file changed

Lines changed: 328 additions & 0 deletions

File tree

‎tests/Unit/Services/ApplyPromoCodeTaskConcurrencyTest.php‎

Lines changed: 328 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,328 @@
1+
<?php namespace Tests\Unit\Services;
2+
/**
3+
* Copyright 2026 OpenStack Foundation
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
**/
9+
10+
use App\Services\Model\ApplyPromoCodeTask;
11+
use App\Services\Utils\ILockManagerService;
12+
use libs\utils\ITransactionService;
13+
use models\exceptions\ValidationException;
14+
use models\main\Member;
15+
use models\summit\IDomainAuthorizedPromoCode;
16+
use models\summit\ISummitRegistrationPromoCodeRepository;
17+
use models\summit\ISummitRepository;
18+
use models\summit\Summit;
19+
use models\summit\SummitRegistrationPromoCode;
20+
use models\summit\SummitTicketType;
21+
use Mockery;
22+
use PHPUnit\Framework\TestCase;
23+
24+
/**
25+
* Simulates concurrent QuantityPerAccount enforcement in ApplyPromoCodeTask
26+
* by controlling what getTicketCountByMemberAndPromoCode returns for each call.
27+
*
28+
* The race condition: two ReserveOrderTask executions commit tickets before
29+
* either ApplyPromoCodeTask runs, so both tasks see the combined count and
30+
* both reject — even though individually each was valid. These tests document
31+
* that behavior and verify the serialized (correct) path.
32+
*
33+
* No real DB or pcntl_fork needed — the race is deterministic once we control
34+
* the mock repository return values for each task invocation.
35+
*
36+
* See PR #525 for full context on the TOCTOU risk.
37+
*/
38+
class ApplyPromoCodeTaskConcurrencyTest extends TestCase
39+
{
40+
protected function setUp(): void
41+
{
42+
parent::setUp();
43+
\Illuminate\Support\Facades\Facade::clearResolvedInstances();
44+
$container = new \Illuminate\Container\Container();
45+
$container->instance('app', $container);
46+
$container->instance('log', new class {
47+
public function __call($name, $args) { /* swallow */ }
48+
});
49+
\Illuminate\Support\Facades\Facade::setFacadeApplication($container);
50+
}
51+
52+
protected function tearDown(): void
53+
{
54+
\Illuminate\Support\Facades\Facade::clearResolvedInstances();
55+
\Illuminate\Support\Facades\Facade::setFacadeApplication(null);
56+
Mockery::close();
57+
parent::tearDown();
58+
}
59+
60+
/**
61+
* Serialized execution (FOR UPDATE lock works correctly):
62+
*
63+
* - Limit = 1, each request buys 1 ticket.
64+
* - Task A runs first: count = 1 (own ticket only, B hasn't committed yet).
65+
* Guard: 1 > 1 = false → passes, calls addUsage.
66+
* - Task B runs second: count = 2 (A's committed ticket + B's own).
67+
* Guard: 2 > 1 = true → rejects.
68+
*
69+
* This is the correct behavior under serialization.
70+
*/
71+
public function testSerializedExecution_FirstSucceeds_SecondRejects(): void
72+
{
73+
$promo_code_value = 'DOMAIN-CODE-1';
74+
$ticket_type_id = 42;
75+
$quantityPerAccountLimit = 1;
76+
77+
$ticket_type = Mockery::mock(SummitTicketType::class);
78+
79+
$summit = Mockery::mock(Summit::class);
80+
$summit->shouldReceive('getId')->andReturn(1);
81+
$summit->shouldReceive('getTicketTypeById')->with($ticket_type_id)->andReturn($ticket_type);
82+
83+
$promo_code = Mockery::mock(SummitRegistrationPromoCode::class, IDomainAuthorizedPromoCode::class);
84+
$promo_code->shouldReceive('getSummitId')->andReturn(1);
85+
$promo_code->shouldReceive('getId')->andReturn(101);
86+
$promo_code->shouldReceive('getCode')->andReturn($promo_code_value);
87+
$promo_code->shouldReceive('validate');
88+
$promo_code->shouldReceive('canBeAppliedTo')->with($ticket_type)->andReturn(true);
89+
$promo_code->shouldReceive('getQuantityPerAccount')->andReturn($quantityPerAccountLimit);
90+
// Only Task A succeeds — exactly one addUsage call.
91+
$promo_code->shouldReceive('addUsage')->once();
92+
93+
$owner = Mockery::mock(Member::class);
94+
95+
$repo = Mockery::mock(ISummitRegistrationPromoCodeRepository::class);
96+
$repo->shouldReceive('getByValueExclusiveLock')
97+
->with($summit, $promo_code_value)
98+
->andReturn($promo_code);
99+
100+
// Serialized: Task A sees count=1, Task B sees count=2.
101+
$repo->shouldReceive('getTicketCountByMemberAndPromoCode')
102+
->with($owner, $promo_code)
103+
->twice()
104+
->andReturnValues([1, 2]);
105+
106+
$tx_service = Mockery::mock(ITransactionService::class);
107+
$tx_service->shouldReceive('transaction')->andReturnUsing(fn($fn) => $fn());
108+
109+
$lock_service = Mockery::mock(ILockManagerService::class);
110+
$lock_service->shouldReceive('lock')->andReturnUsing(fn($_k, $fn) => $fn());
111+
112+
$this->bindSummitRepository($summit);
113+
114+
$formerState = [
115+
'promo_codes_usage' => [
116+
$promo_code_value => [
117+
'qty' => 1,
118+
'types' => [$ticket_type_id],
119+
],
120+
],
121+
];
122+
123+
// --- Task A: should succeed ---
124+
$taskA = new ApplyPromoCodeTask(
125+
$summit, ['owner_email' => 'buyer@example.com'], $owner,
126+
$repo, $tx_service, $lock_service,
127+
);
128+
$resultA = $taskA->run($formerState);
129+
$this->assertTrue($resultA['promo_codes_usage'][$promo_code_value]['redeem']);
130+
131+
// --- Task B: should reject ---
132+
$taskB = new ApplyPromoCodeTask(
133+
$summit, ['owner_email' => 'buyer@example.com'], $owner,
134+
$repo, $tx_service, $lock_service,
135+
);
136+
137+
$this->expectException(ValidationException::class);
138+
$this->expectExceptionMessageMatches('/reached the maximum of 1/');
139+
$taskB->run($formerState);
140+
}
141+
142+
/**
143+
* TOCTOU double-rejection bug — demonstrates the race condition.
144+
*
145+
* Both ReserveOrderTask executions commit before either ApplyPromoCodeTask runs.
146+
* Both tasks see count = 2 (both requests' tickets visible in the DB).
147+
*
148+
* - Limit = 1, each request buys 1 ticket.
149+
* - CORRECT behavior: Task A should succeed (it was the first valid request),
150+
* only Task B should reject.
151+
* - ACTUAL behavior (bug): both see count = 2 → 2 > 1 → both reject.
152+
*
153+
* This test asserts the CORRECT behavior and is expected to FAIL until
154+
* the TOCTOU race is fixed (e.g., by moving the count inside the
155+
* exclusive lock or deducting the current order's own tickets from the count).
156+
*/
157+
public function testDoubleRejection_BothReservedBeforeEitherValidates(): void
158+
{
159+
$promo_code_value = 'DOMAIN-CODE-1';
160+
$ticket_type_id = 42;
161+
$quantityPerAccountLimit = 1;
162+
163+
$ticket_type = Mockery::mock(SummitTicketType::class);
164+
165+
$summit = Mockery::mock(Summit::class);
166+
$summit->shouldReceive('getId')->andReturn(1);
167+
$summit->shouldReceive('getTicketTypeById')->with($ticket_type_id)->andReturn($ticket_type);
168+
169+
$promo_code = Mockery::mock(SummitRegistrationPromoCode::class, IDomainAuthorizedPromoCode::class);
170+
$promo_code->shouldReceive('getSummitId')->andReturn(1);
171+
$promo_code->shouldReceive('getId')->andReturn(101);
172+
$promo_code->shouldReceive('getCode')->andReturn($promo_code_value);
173+
$promo_code->shouldReceive('validate');
174+
$promo_code->shouldReceive('canBeAppliedTo')->with($ticket_type)->andReturn(true);
175+
$promo_code->shouldReceive('getQuantityPerAccount')->andReturn($quantityPerAccountLimit);
176+
// Permissive — correct behavior calls addUsage once, bug calls it zero times.
177+
$promo_code->shouldReceive('addUsage')->zeroOrMoreTimes();
178+
179+
$owner = Mockery::mock(Member::class);
180+
181+
$repo = Mockery::mock(ISummitRegistrationPromoCodeRepository::class);
182+
$repo->shouldReceive('getByValueExclusiveLock')
183+
->with($summit, $promo_code_value)
184+
->andReturn($promo_code);
185+
186+
// Both tasks see the inflated count (both orders' tickets visible).
187+
$repo->shouldReceive('getTicketCountByMemberAndPromoCode')
188+
->with($owner, $promo_code)
189+
->andReturn(2);
190+
191+
$tx_service = Mockery::mock(ITransactionService::class);
192+
$tx_service->shouldReceive('transaction')->andReturnUsing(fn($fn) => $fn());
193+
194+
// Permissive — correct behavior reaches lock, bug throws before it.
195+
$lock_service = Mockery::mock(ILockManagerService::class);
196+
$lock_service->shouldReceive('lock')->zeroOrMoreTimes()->andReturnUsing(fn($_k, $fn) => $fn());
197+
198+
$this->bindSummitRepository($summit);
199+
200+
$formerState = [
201+
'promo_codes_usage' => [
202+
$promo_code_value => [
203+
'qty' => 1,
204+
'types' => [$ticket_type_id],
205+
],
206+
],
207+
];
208+
209+
// --- Task A: SHOULD succeed (first valid request) ---
210+
// BUG: Task A sees count=2 (includes Task B's tickets) and rejects.
211+
$taskA = new ApplyPromoCodeTask(
212+
$summit, ['owner_email' => 'buyer@example.com'], $owner,
213+
$repo, $tx_service, $lock_service,
214+
);
215+
try {
216+
$resultA = $taskA->run($formerState);
217+
} catch (ValidationException $ex) {
218+
$this->fail(
219+
'TOCTOU BUG: Task A was incorrectly rejected. '
220+
. 'When two ReserveOrderTask executions commit before either ApplyPromoCodeTask runs, '
221+
. 'both tasks see an inflated ticket count (2) and both reject — even though '
222+
. 'Task A is a valid first request within the limit of 1. '
223+
. 'Exception: ' . $ex->getMessage()
224+
);
225+
}
226+
$this->assertTrue($resultA['promo_codes_usage'][$promo_code_value]['redeem']);
227+
228+
// --- Task B: should reject (over limit) ---
229+
$taskB = new ApplyPromoCodeTask(
230+
$summit, ['owner_email' => 'buyer@example.com'], $owner,
231+
$repo, $tx_service, $lock_service,
232+
);
233+
$this->expectException(ValidationException::class);
234+
$this->expectExceptionMessageMatches('/reached the maximum of 1/');
235+
$taskB->run($formerState);
236+
}
237+
238+
/**
239+
* Serialized execution with higher limit — both requests succeed:
240+
*
241+
* - Limit = 2, each request buys 1 ticket.
242+
* - Task A runs first: count = 1 → 1 > 2 = false → passes.
243+
* - Task B runs second: count = 2 → 2 > 2 = false → passes.
244+
*
245+
* Confirms that serialized execution correctly allows both requests
246+
* when the combined total stays within the limit.
247+
*/
248+
public function testSerializedExecution_BothAllowedWithinLimit(): void
249+
{
250+
$promo_code_value = 'DOMAIN-CODE-1';
251+
$ticket_type_id = 42;
252+
$quantityPerAccountLimit = 2;
253+
254+
$ticket_type = Mockery::mock(SummitTicketType::class);
255+
256+
$summit = Mockery::mock(Summit::class);
257+
$summit->shouldReceive('getId')->andReturn(1);
258+
$summit->shouldReceive('getTicketTypeById')->with($ticket_type_id)->andReturn($ticket_type);
259+
260+
$promo_code = Mockery::mock(SummitRegistrationPromoCode::class, IDomainAuthorizedPromoCode::class);
261+
$promo_code->shouldReceive('getSummitId')->andReturn(1);
262+
$promo_code->shouldReceive('getId')->andReturn(101);
263+
$promo_code->shouldReceive('getCode')->andReturn($promo_code_value);
264+
$promo_code->shouldReceive('validate');
265+
$promo_code->shouldReceive('canBeAppliedTo')->with($ticket_type)->andReturn(true);
266+
$promo_code->shouldReceive('getQuantityPerAccount')->andReturn($quantityPerAccountLimit);
267+
// Both tasks succeed — two addUsage calls.
268+
$promo_code->shouldReceive('addUsage')->twice();
269+
270+
$owner = Mockery::mock(Member::class);
271+
272+
$repo = Mockery::mock(ISummitRegistrationPromoCodeRepository::class);
273+
$repo->shouldReceive('getByValueExclusiveLock')
274+
->with($summit, $promo_code_value)
275+
->andReturn($promo_code);
276+
277+
// Serialized: Task A sees count=1, Task B sees count=2.
278+
$repo->shouldReceive('getTicketCountByMemberAndPromoCode')
279+
->with($owner, $promo_code)
280+
->twice()
281+
->andReturnValues([1, 2]);
282+
283+
$tx_service = Mockery::mock(ITransactionService::class);
284+
$tx_service->shouldReceive('transaction')->andReturnUsing(fn($fn) => $fn());
285+
286+
$lock_service = Mockery::mock(ILockManagerService::class);
287+
$lock_service->shouldReceive('lock')->andReturnUsing(fn($_k, $fn) => $fn());
288+
289+
$this->bindSummitRepository($summit);
290+
291+
$formerState = [
292+
'promo_codes_usage' => [
293+
$promo_code_value => [
294+
'qty' => 1,
295+
'types' => [$ticket_type_id],
296+
],
297+
],
298+
];
299+
300+
// --- Task A: should succeed ---
301+
$taskA = new ApplyPromoCodeTask(
302+
$summit, ['owner_email' => 'buyer@example.com'], $owner,
303+
$repo, $tx_service, $lock_service,
304+
);
305+
$resultA = $taskA->run($formerState);
306+
$this->assertTrue($resultA['promo_codes_usage'][$promo_code_value]['redeem']);
307+
308+
// --- Task B: should also succeed ---
309+
$taskB = new ApplyPromoCodeTask(
310+
$summit, ['owner_email' => 'buyer@example.com'], $owner,
311+
$repo, $tx_service, $lock_service,
312+
);
313+
$resultB = $taskB->run($formerState);
314+
$this->assertTrue($resultB['promo_codes_usage'][$promo_code_value]['redeem']);
315+
}
316+
317+
/**
318+
* Bind a mock ISummitRepository so ApplyPromoCodeTask::run() can re-attach the summit.
319+
*/
320+
private function bindSummitRepository(Summit $summit): void
321+
{
322+
$summit_repo = Mockery::mock(ISummitRepository::class);
323+
$summit_repo->shouldReceive('getById')->andReturn($summit);
324+
325+
$container = \Illuminate\Support\Facades\Facade::getFacadeApplication();
326+
$container->instance(ISummitRepository::class, $summit_repo);
327+
}
328+
}

0 commit comments

Comments
 (0)