Escape string path parameters by segment type - #1811
Closed
renaudhartert-db wants to merge 2 commits into
Closed
renaudhartert-db wants to merge 2 commits into
renaudhartert-db wants to merge 2 commits into
Conversation
Signed-off-by: Renaud Hartert <renaud.hartert@databricks.com>
Signed-off-by: Renaud Hartert <renaud.hartert@databricks.com>
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 15 days if no further activity occurs. If this PR is still relevant, please leave a comment or push new changes to keep it open. Thank you for your contributions. |
|
This pull request has been automatically closed because it remained stale for 15 days with no further activity. Feel free to reopen it if you'd like to continue working on it. |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Percent-encodes generated string path parameters according to their OpenAPI segment semantics. Ordinary values occupy one URL segment, while explicit and patterned multi-segment resource names preserve
/separators and escape each segment independently.Why
Generated clients previously interpolated most string path parameters directly into request paths. Reserved characters such as
/,#,?, and spaces could therefore change the URL structure or route requests to the wrong endpoint.Treating every string as a single segment is also incorrect: hierarchical resource names such as
projects/p1andprojects/p1/branches/b1rely on/separators to match their routes. This PR uses the generator's segment metadata, including resource patterns fixed by universe#2437239, to distinguish those cases.What changed
Interface changes
httpclient.EncodeSingleSegmentPathParameter(string) stringfor values that must remain within one URL path segment.httpclient.EncodeMultiSegmentPathParameterbehavior.Behavioral changes
/characters become%2F.{parent=projects/*}preserve/as hierarchy separators while escaping each segment.Internal changes
How is this tested?
make fmtmake test(1,493 tests passed)make lintbazel test //openapi/genkit/apimodel:apimodel_test //openapi/genkit/rendering/gobeta:gobeta_test --tool_tag=ai-agent --test_output=errors --noshow_progress --noshow_loading_progress*bindings remain single-segment.