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(plugins) gRPC gateway uri para bool handling #9180

Merged
merged 7 commits into from
Aug 3, 2022

Conversation

StarlightIbuki
Copy link
Contributor

gRPC gateway should handle the "false" argument as false for boolean parameters.

This change is to be consistent with grpc-gateway, where by http://localhost:8090/v1/example/echo?stringv=false&boolean=false&number=1 you get:

{
	"stringv": "false",
	"boolean": false,
	"number": 1
}

as echo.

Before this fix, any value passed to a boolean parameter is treated as a string and thus encoded as true.

Fix FTI-3335

gRPC gateway should handle "false" argument as false for boolean parameters

fix FTI-3335
@StarlightIbuki
Copy link
Contributor Author

For who wants to review:
fixture/grpc/targetservice/* is auto-generated.
targetservice.proto and grpc-target.go are modified for testing the behavior fixed by this PR.

@fffonion fffonion merged commit 6cd2676 into master Aug 3, 2022
@fffonion fffonion deleted the fix/grpc_gateway_false_uri_arg branch August 3, 2022 05:42
StarlightIbuki added a commit that referenced this pull request Aug 9, 2022
gRPC gateway should handle the "false" argument as false for boolean parameters.

This change is to be consistent with [grpc-gateway](https://github.com/grpc-ecosystem/grpc-gateway), where by `http://localhost:8090/v1/example/echo?stringv=false&boolean=false&number=1` you get:
```json
{
	"stringv": "false",
	"boolean": false,
	"number": 1
}
```
as echo.

Before this fix, any value passed to a boolean parameter is treated as a string and thus encoded as `true`.

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

Successfully merging this pull request may close these issues.

3 participants