[BUG] PHP sends boolean params as "1" or "" instead of "true" or "false"

[jacobweber]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• What's the version of OpenAPI Generator used?
• Have you search for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Bounty to sponsor the fix (example)

Description

When params are of type "boolean", they should be sent as "true" or "false", but the PHP generator sends them as "1" or "". But it correctly documents the parameters as @param bool. So if you send boolean parameters, it ends up doing something like (string) true, which evaluates to 1.

openapi-generator version

4.0.0-SNAPSHOT

OpenAPI declaration file content or url

https://gist.github.com/jacobweber/e83874fbf1c0ee6ede79100a7507e11f

Command line used for generation

java -jar openapi-generator.jar generate -i <url> -g php -o /path/to/output

Steps to reproduce

1. Build the above spec.
2. Run composer install from the output dir.
3. Run the "Getting Started" example code in README.md:

$admin = True; // bool | Include admin users?
try {
    $result = $apiInstance->usersGet($admin);
    print_r($result);
} catch (Exception $e) {
    echo 'Exception when calling DefaultApi->usersGet: ', $e->getMessage(), PHP_EOL;
}

4. It calls GET http://localhost/api/users?admin=1 instead of GET http://localhost/api/users?admin=true.

Related issues/PRs

N/A

Suggest a fix

Detect booleans and convert to "true" or "false".

[wing328]

@jacobweber thanks for reporting the issue. I think https://github.com/OpenAPITools/openapi-generator/blob/master/samples/client/petstore-security-test/php/lib/ObjectSerializer.php#L51 si a good starting point to fix it.

[jacobweber]

@wing328 Thanks. But isn't that function just used for serializing model objects? I think in this case I should change toString, which is called by toQueryValue etc. to serialize form/query/path parameters.

I'm not really sure what this function should do, though. I'm sure there are more types besides dates and booleans that need to be serialized. It looks like OpenAPI 3.0 can handle complex serialized parameters using the schema or content properties. Should those be handled as well?

[ackintosh]

Thanks for this issue!

'1' / '0' or 'true' / 'false'

I think both of these are valid as boolean representation. It depends on the way that parse the query.

FYI: http_build_query() converts boolean to '1' / '0' :

var_dump(\http_build_query([
    'true' => true,
    'false' => false,
]));
// string(14) "true=1&false=0"

(Adding a generator option or configuration class option to convert boolean as 'true' / 'false' may good for users.)

'' as representation of false is invalid

It may due to build_query() in guzzle/psr7.

[jacobweber]

@ackintosh But I don’t think those are valid in OpenAPI 2.0. The spec refers to https://tools.ietf.org/html/draft-zyp-json-schema-04#section-3.5, which says that booleans are JSON boolean values. JSON only uses true/false for booleans.

[ackintosh]

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#data-types

Data Types

Primitive data types in the Swagger Specification are based on the types supported by the JSON-Schema Draft 4. Models are described using the Schema Object which is a subset of JSON Schema Draft 4.

From my understanding, that defines how to describe the specification of API. That doesn't define the representation of a value on HTTP communications.

If we follow JSON representation, string values in HTTP query parameter should be surrounded with " like below:

• true_string="true"&false_string="false"&other_string="other"
• true_boolean=true&false_boolean=false&other_string="other"

[jacobweber]

When they refer to "primitive data types", I'm pretty sure they're talking about the values you actually send in an HTTP request, not just the format of the spec. Otherwise they wouldn't specify things like the format for dates and integers. And they say things like "Query parameters only support primitive types."

You're right that they don't make it very clear, at least for booleans. However, in one of the examples here, they do use GET/users/{id}?metadata=true where metadata is a boolean parameter.

It would seem like an incomplete spec if it used a data type but didn't specify how to send it. Even if it doesn't, all the other generators that I've tried send true/false, and so does Swagger-UI. So it's probably best to be consistent.

[wing328]

@jacobweber @ackintosh you both have valid points.

To move this forward, I suggest we go with #2257 and if there are demands to send boolean values as 1/0 in the query string or other parameters, then we can make an option to cater both cases.

(I did work on 1 or 2 issues before in which the users are expecting the client to send true/false in the query string)

[etremblay]

@wing328 The fix introduced by #2257 has been removed by #3984.

ObjectSerializer::toQueryValue() has been removed, I guess to improve collection serialization, but the boolean case is still broken.

[andy-educake]

We've just come across this issue with booleans, as @etremblay says, it seems that removing ObjectSerializer::toQueryValue() has broken using booleans in the query string. Can we put it back? Do we need a new ticket for this? I had a look but couldn't see any tests for the generated code that we might use to demonstrate the bug.

[stevenbuehner]

Yes, I can confirm, hat the boolean issue in a query is still / again in place. Is there anything going on to fix this issue?