[Testing] Default requested parsedBody is null instead of [], 500 error on DELETE /api/discussions
- Dominant language
- No language data
- Stars
- 0
- Forks
- 0
- PR merge metrics
- No merged PRs in 30d
Description
## Bug Report
**Current Behavior**
When performing a non-JSON request with the integration testing package, `$request->getParsedBody()` is `null` whereas it's `[]` (empty array) when used via the web. 500 errors are thrown from Flarum code because Flarum itself cannot handle `null` values in some contexts.
**Steps to Reproduce**
Write integration test:
```php
prepareDatabase([
'discussions' => [
['id' => 1, 'title' => 'A', 'created_at' => Carbon::now(), 'user_id' => 1],
],
]);
}
public function test()
{
$response = $this->send($this->request('DELETE', '/api/discussions/1', [
'authenticatedAs' => 1,
]));
$this->assertEquals(204, $response->getStatusCode());
}
}
```
Output: `Failed asserting that 500 matches expected 204.`
Error log:
```
[2021-06-02 16:32:47] flarum.ERROR: TypeError: Argument 3 passed to Flarum\Discussion\Command\DeleteDiscussion::__construct() must be of the type array, null given, called in /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Api/Controller/DeleteDiscussionController.php on line 43 and defined in /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Discussion/Command/DeleteDiscussion.php:44
Stack trace:
#0 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Api/Controller/DeleteDiscussionController.php(43): Flarum\Discussion\Command\DeleteDiscussion->__construct()
flarum/framework#1 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Api/Controller/AbstractDeleteController.php(24): Flarum\Api\Controller\DeleteDiscussionController->delete()
flarum/framework#2 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Http/RouteHandlerFactory.php(41): Flarum\Api\Controller\AbstractDeleteController->handle()
flarum/framework#3 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Http/Middleware/ExecuteRoute.php(27): Flarum\Http\RouteHandlerFactory->Flarum\Http\{closure}()
flarum/framework#4 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Flarum\Http\Middleware\ExecuteRoute->process()
flarum/framework#5 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Api/Middleware/ThrottleApi.php(33): Laminas\Stratigility\Next->handle()
flarum/framework#6 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Flarum\Api\Middleware\ThrottleApi->process()
flarum/framework#7 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Http/Middleware/CheckCsrfToken.php(40): Laminas\Stratigility\Next->handle()
flarum/framework#8 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Flarum\Http\Middleware\CheckCsrfToken->process()
flarum/framework#9 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Http/Middleware/ResolveRoute.php(67): Laminas\Stratigility\Next->handle()
flarum/framework#10 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Flarum\Http\Middleware\ResolveRoute->process()
flarum/framework#11 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Http/Middleware/SetLocale.php(51): Laminas\Stratigility\Next->handle()
flarum/framework#12 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Flarum\Http\Middleware\SetLocale->process()
flarum/framework#13 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Http/Middleware/AuthenticateWithHeader.php(56): Laminas\Stratigility\Next->handle()
flarum/framework#14 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Flarum\Http\Middleware\AuthenticateWithHeader->process()
flarum/framework#15 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Http/Middleware/AuthenticateWithSession.php(31): Laminas\Stratigility\Next->handle()
flarum/framework#16 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Flarum\Http\Middleware\AuthenticateWithSession->process()
flarum/framework#17 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Http/Middleware/RememberFromCookie.php(52): Laminas\Stratigility\Next->handle()
flarum/framework#18 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Flarum\Http\Middleware\RememberFromCookie->process()
flarum/framework#19 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Http/Middleware/StartSession.php(61): Laminas\Stratigility\Next->handle()
flarum/framework#20 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Flarum\Http\Middleware\StartSession->process()
flarum/framework#21 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Api/Middleware/FakeHttpMethods.php(29): Laminas\Stratigility\Next->handle()
flarum/framework#22 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Flarum\Api\Middleware\FakeHttpMethods->process()
flarum/framework#23 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Http/Middleware/ParseJsonBody.php(28): Laminas\Stratigility\Next->handle()
flarum/framework#24 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Flarum\Http\Middleware\ParseJsonBody->process()
flarum/framework#25 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Http/Middleware/HandleErrors.php(57): Laminas\Stratigility\Next->handle()
flarum/framework#26 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Flarum\Http\Middleware\HandleErrors->process()
flarum/framework#27 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Http/Middleware/InjectActorReference.php(25): Laminas\Stratigility\Next->handle()
flarum/framework#28 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Flarum\Http\Middleware\InjectActorReference->process()
flarum/framework#29 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/MiddlewarePipe.php(84): Laminas\Stratigility\Next->handle()
flarum/framework#30 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/middlewares/request-handler/src/RequestHandler.php(84): Laminas\Stratigility\MiddlewarePipe->process()
flarum/framework#31 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Middlewares\RequestHandler->process()
flarum/framework#32 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/middlewares/base-path-router/src/BasePathRouter.php(101): Laminas\Stratigility\Next->handle()
flarum/framework#33 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Middlewares\BasePathRouter->process()
flarum/framework#34 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Middleware/OriginalMessages.php(42): Laminas\Stratigility\Next->handle()
flarum/framework#35 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Laminas\Stratigility\Middleware\OriginalMessages->process()
flarum/framework#36 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/middlewares/base-path/src/BasePath.php(73): Laminas\Stratigility\Next->handle()
flarum/framework#37 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Middlewares\BasePath->process()
flarum/framework#38 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/core/src/Http/Middleware/ProcessIp.php(24): Laminas\Stratigility\Next->handle()
flarum/framework#39 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/Next.php(61): Flarum\Http\Middleware\ProcessIp->process()
flarum/framework#40 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/MiddlewarePipe.php(84): Laminas\Stratigility\Next->handle()
flarum/framework#41 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/laminas/laminas-stratigility/src/MiddlewarePipe.php(73): Laminas\Stratigility\MiddlewarePipe->process()
flarum/framework#42 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/flarum/testing/src/integration/TestCase.php(238): Laminas\Stratigility\MiddlewarePipe->handle()
flarum/framework#43 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/tests/integration/NullParsedBodyTest.php(23): Flarum\Testing\integration\TestCase->send()
flarum/framework#44 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/phpunit/phpunit/src/Framework/TestCase.php(1526): Kilowhat\Audit\Tests\integration\NullParsedBodyTest->test()
flarum/framework#45 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/phpunit/phpunit/src/Framework/TestCase.php(1132): PHPUnit\Framework\TestCase->runTest()
flarum/framework#46 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/phpunit/phpunit/src/Framework/TestResult.php(722): PHPUnit\Framework\TestCase->runBare()
flarum/framework#47 /home/clark/Projects/flarum-1.0/workbench/flarum-ext-audit/vendor/phpunit/phpunit/src/Framework/TestCase.php(884): PHPUnit\Framework\TestResult->run()
flarum/framework#48 Standard input code(315): PHPUnit\Framework\TestCase->run()
flarum/framework#49 Standard input code(574): __phpunit_run_isolated_test()
flarum/framework#50 {main}
```
**Expected Behavior**
It should be possible to make a DELETE request to `/api/discussions/` without specifying a content type in the integration test, just like it's possible to do from the web.
**Environment**
- Flarum version: 1.0.0
- Hosting environment: local
- PHP version: 7.4.18
**Possible Solution**
More parameters should be passed to the `ServerRequest` constructor in https://github.com/flarum/testing/blob/main/src/integration/TestCase.php#L267 to provide a default `$parsedBody` value of `[]` instead of `null`.
I think it would also make sense to either allow `null` in https://github.com/flarum/core/blob/master/src/Discussion/Command/DeleteDiscussion.php#L44 or add `?? []` in https://github.com/flarum/core/blob/master/src/Api/Controller/DeleteDiscussionController.php#L43 to make sure a `null` parsed body doesn't crash the request with a 500 error, since an extension could also alter the `parsedBody` attribute. The same probably also applies to other `DELETE` endpoints where we pass the `parsedBody` value directly to an `array` type-hint.
Same issue affects https://github.com/flarum/flags/blob/master/src/Api/Controller/DeleteFlagsController.php#L40
**Additional Context**
We default to an empty array in https://github.com/flarum/core/blob/master/src/Http/Middleware/ParseJsonBody.php but the reason an empty array is also present for non-JSON requests is because inside of Lamina's `ServerRequestFactory` class the default value for `parsedBody` is `$body ?: $_POST` (line 74).
In the testing package we build the request from scratch so we never default to `$_POST` for `parsedBody`.
A workaround for the test package is to provide `'json' => [],` in the request options, this forces Flarum to parse the body as JSON and default to an empty array.
Contributor guide
Research direction
Start with vendor/flarum/testing/src/integration/TestCase.php around line 267 and reproduce the failure using tests/integration/NullParsedBodyTest.php. Trace the DELETE path through DeleteDiscussionController.php and DeleteDiscussion.php, then verify the request returns 204 without a null parsed body; the issue also names DeleteFlagsController.php as another affected endpoint.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- api, testing
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100