Skip to content

[BUG] PHP Client - ObjectSerializer::buildQuery flattens array params resulting invalid URL params (param=a&param=b vs param[]=a&param[]=b) #19233

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

Closed
serbanghita opened this issue Jul 24, 2024 · 5 comments

Comments

@serbanghita
Copy link
Contributor

Description

ObjectSerializer::buildQuery flattens array params (e.g. exclude[]) and instead of
exclude[]=a&exclude[]=b it outputs exclude=a&exclude=b resulting in HTTP contract errors:

[422] Client error: `GET test.com/?exclude=a&exclude=b` resulted in a `422 Unprocessable Entity` response:
{"error":"invalid request query exclude: expected an array, but got `\"exclude\"` instead."}
OpenAPI declaration file content or url

This is the OpenAPI relevant schema:

paths:
  /internal/url:
    get:
      x-stability-level: stable
      parameters:
        - name: exclude
          in: query
          required: false
          style: deepObject  // <---- this affect ObjectSerializer::toQueryValue output
          explode: true         // <----- this also affect ObjectSerializer::toQueryValue output
          schema:
            type: array
            items:
              type: string

Results in generated PHP code from DefaultApi.php:

ObjectSerializer::toQueryValue(
            $exclude,
            'exclude', // param base name
            'array', // openApiType
            'deepObject', // style
            true, // explode
            false // required
        )

Screenshot 2024-07-24 at 13 20 09

exclude = [
 'exclude[0]' => 'a',
 'exclude[1]' => 'b'
]

This appears to confuse `\GuzzleHttp\Psr7\Query::build($data, $encoding_type); in 7.3.0 and it's the same in 7.7.0

such that the output is

exclude=a&exclude=b

causing the HTTP contract to fail.

openapi-generator version
  • regression on 7.3.0
  • also happening on 7.7.0
Suggest a fix

If you change exclude field declaration from array to object then the output

ObjectSerializer::toQueryValue(
            $exclude,
            'exclude', // param base name
            'object', // openApiType <---------- changed from 'array'
            'deepObject', // style
            true, // explode
            false // required
        )

then the output is a single layer array

Screenshot 2024-07-24 at 14 10 43

[
 'exclude[0]' => 'a',
 'exclude[1]' => 'b'
]

Unfortunately this cannot be added to the OpenAPI schema because it doesn't make sense, the param is actually an array not an object.

Then I think the solution is to change a line of code inside ObjectSerializer::toQueryValue 7.3.0

        if ($openApiType === 'object' && ($style === 'deepObject' || $explode)) {
            return $value;
        }

this should also take into account $openApiType === 'array'

@serbanghita
Copy link
Contributor Author

@wing328 @fehguy any chance of you getting some time or recommend someone to look over this?

@serbanghita
Copy link
Contributor Author

serbanghita commented Jul 26, 2024

This is the default behaviour:

// 0=blue&1=black&2=brown
ObjectSerializer::buildQuery(['blue', 'black', 'brown']); 

see test provider

'deepObject array, explode on, required true' => [
$array, 'color', 'array', 'deepObject', true, true, 'color=blue&color=black&color=brown',
],

I believe this is wrong behaviour

It appears that the following results in CORRECT query parameters

// color%5B%5D=blue&color%5B%5D=black&color%5B%5D=brown
ObjectSerializer::buildQuery(['color[]' => ['blue', 'black', 'brown']]);

but there is no way you can format the YAML OpenAPI data and then pass it to
ObjectSerializer::toQueryValue($data, $paramName, $openApiType, $style, $explode, $required); so it can result in $data = ['color[]' => ['blue', 'black', 'brown']] format cc @nadar @ybelenko

I think this is related to #11225

@serbanghita
Copy link
Contributor Author

According to OAI/OpenAPI-Specification#1006 (comment)

Tooling will have to deal with it, and I'd suggest just clobbering values. It's really up to the user to define things that make sense, within the defined behavior.
Authoring tools might be able to provide warnings to users in such a case.

In our case, this is a PHP client and the brackets[] are part of standard way of passing array query params.

Example / POC:

mkdir tmp
vim index.php

index.php add the following:

<?php
print_r($_GET);

Then run the local webserver: php -S localhost:8888:

WRONG

GET http://localhost:8888/?color=blue&color=black&color=brown
Array ( [color] => brown )

GOOD

GET http://localhost:8888/?color[]=blue&color[]=black&color[]=brown
Array ( [color] => Array ( [0] => blue [1] => black [2] => brown ) )

wing328 pushed a commit that referenced this issue Aug 7, 2024
… resulting invalid URL params (param=a&param=b vs param[]=a&param[]=b) #19233 (#19236)

* [BUG] PHP Client - ObjectSerializer::buildQuery flattens array params resulting invalid URL params (param=a&param=b vs param[]=a&param[]=b) #19233

* Added tests (replaced old provider data). This looks like a breaking change

* Fix space

---------

Co-authored-by: Serban Ghita <[email protected]>
@jtreminio
Copy link
Contributor

This bug was fixed in the v7.8.0 release, should be closed?

@serbanghita
Copy link
Contributor Author

@jtreminio thanks for the reminder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants