Array-typed `form` parameter rejects a single value: `deserializeForm()` sniffs for a comma instead of reading the schema type

[shadowhand]

PHP version

8.5

duyler/openapi version

0.7.0

OpenAPI spec version

3.0

Description

ParameterDeserializer::deserializeForm() decides whether a non-exploded form value is an array by looking for a comma in the string, rather than by looking at the declared schema type:

if (false === $explode && str_contains($value, ',')) {
    $this->assertWithinItemLimit($value, ',');

    return explode(',', $value);
}

return $value;

So for a parameter declared type: array with explode: false, a value carrying two items deserializes to an array, but a value carrying one item stays a string and then fails the type: array check with TypeMismatchError.

The result is an array parameter that rejects exactly one item while accepting two. For a JSON:API include parameter this means ?include=author,comments is accepted and ?include=author is a 400.

deserializeForm() is the only style branch that does this. deserializeSimple(), deserializeMatrix(), deserializeLabel(), and deserializeCookie() all route on isArrayType($param) and pass the value through splitBySeparator(), which correctly returns a one element list for a lone value and an empty list for an empty one. Only form uses the separator-sniffing heuristic.

The same heuristic also drops the empty case: ?include= yields '' rather than [].

Steps to reproduce

<?php

declare(strict_types=1);

use Duyler\OpenApi\Builder\OpenApiValidatorBuilder;
use Nyholm\Psr7\Factory\Psr17Factory;

require __DIR__ . '/vendor/autoload.php';

$yaml = <<<'YAML'
openapi: 3.0.0
info:
  title: Include API
  version: 1.0.0
paths:
  /articles:
    get:
      parameters:
        - name: include
          in: query
          style: form
          explode: false
          schema:
            type: array
            items:
              type: string
      responses:
        '200':
          description: OK
YAML;

$validator = OpenApiValidatorBuilder::create()->fromYamlString($yaml)->build();
$factory = new Psr17Factory();

foreach (['/articles?include=author,comments', '/articles?include=author'] as $uri) {
    try {
        $validator->validateRequest($factory->createServerRequest('GET', $uri));
        printf("%-40s PASS\n", $uri);
    } catch (Throwable $e) {
        printf("%-40s %s: %s\n", $uri, $e::class, $e->getMessage());
    }
}

Equivalently, at the unit level:

$param = new Parameter(
    name: 'include',
    in: 'query',
    style: 'form',
    explode: false,
    schema: new Schema(type: 'array', items: new Schema(type: 'string')),
);

new ParameterDeserializer()->deserialize('author', $param);          // 'author'
new ParameterDeserializer()->deserialize('author,comments', $param); // ['author', 'comments']

Actual result

/articles?include=author,comments        PASS
/articles?include=author                 Duyler\OpenApi\Validator\Exception\TypeMismatchError: Expected type "array", but got "string" at /

Expected: both accepted, the second deserializing to ['author'].

---

Suggested fix

Route on the declared type the way every other style already does, keeping the comma heuristic only as the fallback for parameters that declare no array type:

private function deserializeForm(array|string $value, Parameter $param): array|int|string
{
    if (is_array($value)) {
        if ($param->explode) {
            return $value;
        }

        /** @var array<int, scalar> $value */
        return implode(',', $value);
    }

    if ($param->explode) {
        return $value;
    }

    if ($this->isArrayType($param)) {
        return $this->splitBySeparator($value, ',');
    }

    if (str_contains($value, ',')) {
        $this->assertWithinItemLimit($value, ',');

        return explode(',', $value);
    }

    return $value;
}

This leaves explode: true and array-valued input untouched.

One existing test asserts the current behaviour: HeaderValidationTest::header_array_type_with_single_value_throws_type_mismatch. Its schema is type: array, minItems: 2 and the value is 'solo', so the request is still rejected after the fix — as MinItemsError ("Array has 1 items, but minimum is 2"), which is the accurate reason. "A lone value is not an array" was the wrong one.

With that one test re-pointed, the full suite passes (7221 tests), and php-cs-fixer and psalm are both clean.

Related, but deliberately left alone

Two adjacent problems in the same function that a fix for this issue need not touch, noted in case they are of interest:

1. The mirror case. The comma heuristic fires even when the schema declares type: string, so ?q=a,b deserializes to ['a', 'b'] and then fails the string check. Same root cause — shape decided without consulting the type.
2. explode: true with an array type. ?tags=php against type: array also stays a scalar and fails. Fixing this is entangled with the explode default: OpenAPI specifies explode: true for style: form, but Parameter::$explode defaults to false for every style, and QueryParameterEdgeCasesTest::qp_02/qp_07 are built on that default.

[milinsky]

@shadowhand Thanks for your issue. I will soon release a large number of fixes and improvements from the dev branch and return to your pull requests.