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.

Sign up for GitHub

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

Jump to bottom

URL encode key value pairs in x-goog-request-params #12232

Closed
10 tasks done
scotthart opened this issue Jul 26, 2023 · 3 comments
Closed
10 tasks done

URL encode key value pairs in x-goog-request-params #12232

scotthart opened this issue Jul 26, 2023 · 3 comments
Assignees
@alevenberg
Labels
cpp: operator Good things for the operator to fix priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@scotthart
Copy link
Member

scotthart commented Jul 26, 2023 •
edited by alevenberg
Loading

Per https://google.aip.dev/client-libraries/4222

This mostly affects the metadata decorators.

@dbolduc 's understanding is that we encode the keys and values and leave the & and = joiners as they are.

We need to:

  • URL encode the keys for implicit and explicit routing at code generation time
  • URL encode the values for...
    • explicit routing in GAPICs + legacy bigtable + testing_util::ValidateMetadataFixture
    • explicit routing -> simple case (that doesn't need a RoutingMatcher)
    • implicit routing in GAPICs + testing_util::ValidateMetadataFixture ?
    • storage's client side streaming APIs
    • handwritten pubsub client side streaming APIs
    • handwritten minimal IAM stub
    • maybe handwritten bigquery v2, although no requests seem to require routing. 🤷 (none of the requests set the params)
    • legacy spanner admin, if we care to (We don't care to keep spanner legacy admin classes up to date, since it is legacy and now generated)
@scotthart scotthart added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Jul 26, 2023
@dbolduc dbolduc added the cpp: operator Good things for the operator to fix label Jul 26, 2023
@dbolduc dbolduc self-assigned this Aug 22, 2023
This was referenced Aug 22, 2023
Merged
Merged
Merged
@coryan
Copy link
Contributor

coryan commented Aug 23, 2023

Do we need to do anything for REST-based clients?

@dbolduc
Copy link
Member

dbolduc commented Aug 23, 2023

Do we need to do anything for REST-based clients?

Yeah. Hopefully the work is identical to the gRPC GAPICs

@dbolduc dbolduc mentioned this issue Aug 24, 2023
Merged
@alevenberg alevenberg self-assigned this Aug 29, 2023
@dbolduc dbolduc removed their assignment Aug 29, 2023
This was referenced Aug 29, 2023
Merged
Merged
Closed
Merged
Merged
Closed
This was referenced Sep 6, 2023
Merged
Closed
@alevenberg
Copy link
Member

Looked for x-goog-request-params in the handwritten bigquery: google/cloud/bigquery/v2/minimal

And none of the 4 decorators (dataset, job, project, or table) set any headers on the context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alevenberg alevenberg

Labels
cpp: operator Good things for the operator to fix priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@coryan @scotthart @dbolduc @alevenberg