Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

### Fixed

- Reject JSON and multipart bodies that webonyx parses as a batch with non-object entries, such as `{"query":"…","0":"x"}`, instead of letting them cause a `TypeError`

## v2.3.1

### Fixed
Expand Down
37 changes: 23 additions & 14 deletions src/RequestParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,8 @@ protected function bodyParams(Request $request): array

$bodyParams = $request->post();

if (is_array($bodyParams) && count($bodyParams) > 0) {
if (Arr::isAssoc($bodyParams)) {
return $bodyParams;
}

$allAssoc = true;
foreach ($bodyParams as $bodyParam) {
if (! is_array($bodyParam) || ! Arr::isAssoc($bodyParam)) {
$allAssoc = false;
}
}
if ($allAssoc) {
return $bodyParams;
}
if (is_array($bodyParams) && count($bodyParams) > 0 && $this->isOperationOrBatch($bodyParams)) {
return $bodyParams;
}

if ($request->isJson()) {
Expand Down Expand Up @@ -158,6 +146,27 @@ protected function inlineFiles(Request $request): array
}
}

if (! $this->isOperationOrBatch($operations)) {
throw new BadMultipartRequestGraphQLException('Expected parameter operations to be an object or a list of objects.');
}

return $operations;
}

/** @param array<mixed> $params */
protected function isOperationOrBatch(array $params): bool
{
// \GraphQL\Server\Helper::parseRequestParams() treats any body with a non-null key 0 as a batch
if (! isset($params[0]) && Arr::isAssoc($params)) {
return true;
}

foreach ($params as $operation) {
if (! is_array($operation) || ! Arr::isAssoc($operation)) {
return false;
}
}

return true;
}
}
115 changes: 115 additions & 0 deletions tests/Unit/RequestParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,57 @@ public function testInvalidJson(): void
$parser->parseRequest($request);
}

public function testObjectWithNumericKeyIsNotABatch(): void
{
$request = $this->makeRequest(
'POST',
[],
[],
['Content-Type' => 'application/json'],
'{"query":"{ foo }","0":"not an operation"}'
);
$parser = new RequestParser();

$this->expectException(BadRequestGraphQLException::class);
$this->expectExceptionMessage('GraphQL Server expects JSON object or array, but got: {"query":"{ foo }","0":"not an operation"}');

$parser->parseRequest($request);
}

public function testBatchWithNonObjectEntry(): void
{
$request = $this->makeRequest(
'POST',
[],
[],
['Content-Type' => 'application/json'],
'[{"query":"{ foo }"},"not an operation"]'
);
$parser = new RequestParser();

$this->expectException(BadRequestGraphQLException::class);
$this->expectExceptionMessage('GraphQL Server expects JSON object or array, but got: [{"query":"{ foo }"},"not an operation"]');

$parser->parseRequest($request);
}

public function testBatchStartingWithNull(): void
{
$request = $this->makeRequest(
'POST',
[],
[],
['Content-Type' => 'application/json'],
'[null]'
);
$parser = new RequestParser();

$this->expectException(BadRequestGraphQLException::class);
$this->expectExceptionMessage('GraphQL Server expects JSON object or array, but got: [null]');

$parser->parseRequest($request);
}

public function testNonArrayJson(): void
{
$request = $this->makeRequest(
Expand Down Expand Up @@ -385,6 +436,70 @@ public function testMultipartFormWithNonArrayOperations(): void
$parser->parseRequest($request);
}

public function testMultipartFormWithOperationsObjectWithNumericKey(): void
{
$request = $this->makeRequest(
'POST',
[
'operations' => /** @lang JSON */ '{"query":"{ foo }","0":"not an operation"}',
'map' => /** @lang JSON */ '{}',
],
[],
[
'Content-Type' => 'multipart/form-data',
]
);

$parser = new RequestParser();

$this->expectException(BadMultipartRequestGraphQLException::class);
$parser->parseRequest($request);
}

public function testMultipartFormWithBatchWithNonObjectEntry(): void
{
$request = $this->makeRequest(
'POST',
[
'operations' => /** @lang JSON */ '[{"query":"{ foo }"},"not an operation"]',
'map' => /** @lang JSON */ '{}',
],
[],
[
'Content-Type' => 'multipart/form-data',
]
);

$parser = new RequestParser();

$this->expectException(BadMultipartRequestGraphQLException::class);
$parser->parseRequest($request);
}

public function testMultipartFormWithMapToNumericKey(): void
{
$file = UploadedFile::fake()->create('image.jpg', 500);

$request = $this->makeRequest(
'POST',
[
'operations' => /** @lang JSON */ '{"query":"{ foo }"}',
'map' => /** @lang JSON */ '{"file":["0"]}',
],
[
'file' => $file,
],
[
'Content-Type' => 'multipart/form-data',
]
);

$parser = new RequestParser();

$this->expectException(BadMultipartRequestGraphQLException::class);
$parser->parseRequest($request);
}

public function testMultipartFormWithoutMap(): void
{
$request = $this->makeRequest(
Expand Down
Loading