Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #147: multipart/form-data validation #162

Closed
wants to merge 4 commits into from
Closed
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
29 changes: 29 additions & 0 deletions src/Foundation/ArrayHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@

use function array_keys;
use function count;
use function is_array;
use function preg_match;
use function range;
use function str_replace;

final class ArrayHelper
{
Expand All @@ -28,4 +31,30 @@ public static function isAssoc(array $arr): bool

return array_keys($arr) !== range(0, count($arr) - 1);
}

/**
* @param array<int|string, mixed> $arr
* @param mixed $value
*/
public static function setRecursive(array &$arr, string $key, $value): void
{
$_arr = &$arr;
while (preg_match('#^([^\]]+)\[([^\]]*)\](.*)$#', $key, $matches)) {
if (! isset($arr[$matches[1]]) || ! is_array($arr[$matches[1]])) {
$_arr[$matches[1]] = [];
}

if ($matches[2] === '') {
$key = count($_arr[$matches[1]]);
} else {
$key = $matches[2];
}

$key .= preg_match('#^\[.*\]$#', $matches[3]) ? $matches[3] : '';
$_arr = &$_arr[$matches[1]];
}

$key = str_replace('[', '_', $key);
$_arr[$key] = $value;
}
}
6 changes: 4 additions & 2 deletions src/PSR7/Validators/BodyValidator/MultipartValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use cebe\openapi\spec\Schema;
use cebe\openapi\spec\Type as CebeType;
use InvalidArgumentException;
use League\OpenAPIValidation\Foundation\ArrayHelper;
use League\OpenAPIValidation\PSR7\Exception\NoPath;
use League\OpenAPIValidation\PSR7\Exception\Validation\InvalidBody;
use League\OpenAPIValidation\PSR7\Exception\Validation\InvalidHeaders;
Expand Down Expand Up @@ -189,7 +190,8 @@ private function parseMultipartData(OperationAddress $addr, StreamedPart $docume
}

// if name is not set, it should be validated with "additionalProperties" keyword
$multipartData[$part->getName() ?? '____' . $i] = $partBody;
$paramName = $part->getName() ?? '____' . $i;
ArrayHelper::setRecursive($multipartData, $paramName, $partBody);
}

return $multipartData;
Expand Down Expand Up @@ -235,7 +237,7 @@ private function validateServerRequestMultipart(

$files = $this->normalizeFiles($message->getUploadedFiles());

$body = array_replace($body, $files);
$body = $this->deserializeBody(array_replace($body, $files), $schema);

$validator = new SchemaValidator($this->detectValidationStrategy($message));
try {
Expand Down
24 changes: 12 additions & 12 deletions src/PSR7/Validators/SerializedParameter.php
Original file line number Diff line number Diff line change
Expand Up @@ -166,18 +166,6 @@ private function castToSchemaType($value, ?string $type)
*/
protected function convertToSerializationStyle($value, ?CebeSchema $schema)
{
if (in_array($this->style, [self::STYLE_FORM, self::STYLE_SPACE_DELIMITED, self::STYLE_PIPE_DELIMITED], true)) {
if ($this->explode === false) {
$value = explode(self::STYLE_DELIMITER_MAP[$this->style], $value);
}

foreach ($value as &$val) {
$val = $this->castToSchemaType($val, $schema->items->type ?? null);
}

return $value;
}

if ($schema && $this->style === self::STYLE_DEEP_OBJECT) {
foreach ($value as $key => &$val) {
$childSchema = $this->getChildSchema($schema, (string) $key);
Expand All @@ -191,6 +179,18 @@ protected function convertToSerializationStyle($value, ?CebeSchema $schema)
return $value;
}

if (in_array($this->style, [self::STYLE_FORM, self::STYLE_SPACE_DELIMITED, self::STYLE_PIPE_DELIMITED], true)) {
if ($this->explode === false) {
$value = explode(self::STYLE_DELIMITER_MAP[$this->style], $value);
}
}

if (is_array($value)) {
foreach ($value as &$val) {
$val = $this->castToSchemaType($val, $schema->items->type ?? null);
}
}

return $value;
}

Expand Down
119 changes: 119 additions & 0 deletions tests/Foundation/ArrayHelperTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
<?php

declare(strict_types=1);

namespace League\OpenAPIValidation\Tests\Foundation;

use Generator;
use League\OpenAPIValidation\Foundation\ArrayHelper;
use PHPUnit\Framework\TestCase;

use function count;
use function parse_str;

class ArrayHelperTest extends TestCase
{
/**
* @param array<int|string, mixed> $initArr
* @param mixed $value
* @param array<int|string, mixed> $expected
*
* @dataProvider getTestDataForSetRecursive
*/
public function testSetRecursive(array $initArr, string $key, $value, array $expected): void
{
$arr = $initArr;

ArrayHelper::setRecursive($arr, $key, $value);
self::assertEquals($expected, $arr);

if (count($initArr) !== 0) {
return;
}

$queryString = "{$key}=$value";
parse_str($queryString, $arr);

self::assertEquals($expected, $arr);
}

public function getTestDataForSetRecursive(): Generator
{
yield 'set value by key' => [
[],
'item',
1,
['item' => 1],
];

yield 'add value to an empty non-existent list' => [
[],
'list[]',
1,
['list' => [1]],
];

yield 'add value to an empty existent list' => [
['list' => []],
'list[]',
1,
['list' => [1]],
];

yield 'add value to a not empty list' => [
['list' => [0]],
'list[]',
1,
['list' => [0, 1]],
];

yield 'override existing value, if it is not array' => [
['list' => 0],
'list[]',
1,
['list' => [1]],
];

yield 'set values with string keys' => [
[],
'list[some_key][another_key]',
1,
['list' => ['some_key' => ['another_key' => 1]]],
];

yield 'set values to a list of objects' => [
['list' => [['another_key' => 0]]],
'list[][another_key]',
1,
['list' => [['another_key' => 0], ['another_key' => 1]]],
];

yield 'somehow sets value on incorrect key #1' => [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are looks like bugs for me. I expect some kind of exception should be thrown here

[],
'list[asd]another_[key]',
1,
['list' => ['asd' => 1]],
];

yield 'somehow sets value on incorrect key #2' => [
[],
'list]asdanother_key',
1,
['list]asdanother_key' => 1],
];

yield 'somehow sets value on incorrect key #3' => [
[],
'list[asdanother_key',
1,
['list_asdanother_key' => 1],
];

yield 'somehow sets value on incorrect key #4' => [
[],
'list]asd[another_key',
1,
['list]asd_another_key' => 1],
];
}
}
136 changes: 136 additions & 0 deletions tests/FromCommunity/Issue147Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
<?php

declare(strict_types=1);

namespace League\OpenAPIValidation\Tests\FromCommunity;

use GuzzleHttp\Psr7\Message;
use GuzzleHttp\Psr7\ServerRequest;
use League\OpenAPIValidation\PSR7\ValidatorBuilder;
use League\OpenAPIValidation\Tests\PSR7\BaseValidatorTest;

/**
* @see https://github.com/thephpleague/openapi-psr7-validator/issues/147
*/
final class Issue147Test extends BaseValidatorTest
{
/** @var string */
private $openApiSpecification;

public function testIssue147WithServerRequest(): void
{
$validator = (new ValidatorBuilder())->fromJson($this->openApiSpecification)->getServerRequestValidator();

$data = [
'arrayOfNumbers' => ['1.2', '0', '2.2'],
'arrayOfIntegers' => ['1', '3'],
'arrayOfBooleans' => ['false', 'true'],
];

$psrRequest = (new ServerRequest('post', 'http://localhost:8000/api/list'))
->withHeader('Content-Type', 'multipart/form-data')
->withParsedBody($data);

$validator->validate($psrRequest);

$this->addToAssertionCount(1);
}

public function testIssue147WithRawBody(): void
{
$validator = (new ValidatorBuilder())->fromJson($this->openApiSpecification)->getServerRequestValidator();

$body = <<<HTTP
POST /api/list HTTP/1.1
Content-Length: 554
Content-Type: multipart/form-data; boundary=----WebKitFormBoundaryOmz20xyMCkE27rN7

------WebKitFormBoundaryOmz20xyMCkE27rN7
Content-Disposition: form-data; name="arrayOfNumbers[]"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://swagger.io/docs/specification/serialization/

shouldn't this work only if deep object serialization specified? this is the only way I see arrayOfNumbers[] can work alongside with openapi

Content-Type: text/plain

123.0
------WebKitFormBoundaryOmz20xyMCkE27rN7
Content-Disposition: form-data; name="arrayOfNumbers[]"
Content-Type: text/plain

14
------WebKitFormBoundaryOmz20xyMCkE27rN7
Content-Disposition: form-data; name="arrayOfBooleans[]"
Content-Type: text/plain

true
------WebKitFormBoundaryOmz20xyMCkE27rN7
Content-Disposition: form-data; name="arrayOfIntegers[]"
Content-Type: text/plain

456
------WebKitFormBoundaryOmz20xyMCkE27rN7--
HTTP;

$message = Message::parseRequest($body);
$psrRequest = new ServerRequest(
$message->getMethod(),
$message->getUri(),
$message->getHeaders(),
$message->getBody()
);

$validator->validate($psrRequest);

$this->addToAssertionCount(1);
}

protected function setUp(): void
{
parent::setUp(); // TODO: Change the autogenerated stub
$this->openApiSpecification = /** @lang json */
<<<JSON
{
"openapi": "3.0.0",
"servers": [
{
"url": "https://localhost"
}
],
"paths": {
"/api/list": {
"post": {
"requestBody": {
"content": {
"multipart/form-data": {
"schema": {
"type": "object",
"properties": {
"arrayOfNumbers": {
"type": "array",
"items": {
"type": "number"
}
},
"arrayOfIntegers": {
"type": "array",
"items": {
"type": "integer"
}
},
"arrayOfBooleans": {
"type": "array",
"items": {
"type": "boolean"
}
}
},
"required": ["arrayOfNumbers", "arrayOfIntegers", "arrayOfBooleans"]
}
}
}
},
"responses": {}
}
}
}
}
JSON;
}
}