[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?

[wing328]

I don't think anyone is working on a fix for this issue.

We welcome a PR from anyone in the community to start with.

[stevenbuehner]

I found out, that the issue may have been fixed in guzzle guzzle/psr7#264 and was relased a view days ago with guzzle/psr7 2.0.0.

But this whould mean, that openapi-generator needs to use the new version of guzzle/psr with slightly changed function names. Is this possible in the near future? Othterwise a workaround would be needed - which I could provide.

[wing328]

We definitely want to upgrade guzzle to the latest stable version. I think there's a PR for that but some tests failed (which may be caused by the slightly changed functions names that you've pointed out).

If it's not a lot of work, I would suggest filing a PR to upgrade guzzle to the latest stable version to start with and we'll start from there (fixing tests, etc)

[mysticaltech]

Folks, I am still experiencing this in 6.0.1, the day is coming as true/false, but read as 1 / "". What do I do?

[harryqt]

@mysticaltech Same issue, did you find any patch?

[mysticaltech]

@Dibbyo456 I solved it, but sadly I do not remember how.

[harryqt]

I ended up replacing boolean values to string.