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

Fixed parsing bool value from query string #19028

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Teclor
Copy link

@Teclor Teclor commented Oct 29, 2024

When boolean value comes from query it is represented as 'true' or 'false string. However, it is not parsed until it is not a key for a map field. As a result, error message is thrown when boolean values are passed in GET requests (e.g. when some rest service is mapped to grpc one).
My suggestion: as long as field is defined as boolean, let's accept string 'true' or 'false' values too.

@Teclor Teclor requested a review from a team as a code owner October 29, 2024 15:49
@Teclor Teclor requested review from ericsalo and removed request for a team October 29, 2024 15:49
Copy link

google-cla bot commented Oct 29, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@anandolee
Copy link
Contributor

@Teclor Thank you for this PR, but unfortunately we can't review it until you sign the CLA.

@Teclor
Copy link
Author

Teclor commented Nov 1, 2024

@Teclor Thank you for this PR, but unfortunately we can't review it until you sign the CLA.

Hello, I signed CLA the day I made this MR, then I restarted check and now you can see it succeeded 3 days ago.
Here it is:
https://github.com/protocolbuffers/protobuf/pull/19028/checks

Also it looks passed in the check list here in MR
image

@Teclor
Copy link
Author

Teclor commented Nov 11, 2024

@anandolee could you remove the "cla: no" tag from pull request, please? Or do I have to proceed some extra steps?

@tonyliaoss tonyliaoss added 🅰️ safe for tests Mark a commit as safe to run presubmits over and removed cla: no labels Nov 19, 2024
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Nov 19, 2024
@tonyliaoss tonyliaoss self-assigned this Nov 21, 2024
@acozzette acozzette added the php label Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants