SUP-4172: Preserve query parameters in bk api requests #575
Conversation
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
Previously, query parameters like /builds?branch=main were being URL-encoded (?→%3F), causing 404 errors. The fix separates the query string before url.JoinPath() and reattaches it after.
Contributor
Author
|
Test: |
mcncl
reviewed
Dec 6, 2025
Contributor
mcncl
left a comment
mcncl
left a comment
There was a problem hiding this comment.
I think this would work if the URL is in an ideal state, but would fail if incorrect/unexpected encoding was used and/or missing?
For example, using the api call with /builds?branch=feature/test name&state=passed, I think would fail due to the space?
I think that would also be the case if any fragments were passed in using #? Ie, what would happen if the user sets the API call to /builds?branch=main#foo?
There is a Parse function available on the url package of stdlib which I think would do a much better job than using the strings package.
mcncl
approved these changes
Dec 11, 2025
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.
Previously, query parameters like
/builds?branch=mainwere being URL-encoded (?→%3F), causing 404 errors. The fix separates the query string beforeurl.JoinPath()and reattaches it after.Fixed at the HTTP level instead of
apicommand level (api.go), so no need for flags to pass through each url (sub)path:internal/http/client.go- Parse and preserve query parameters in URL constructioninternal/http/client_test.go- Added tests for query parameter handlingFix at the
apicommand level would have been:bk api /builds --field branch=main --field state=passedImplemented fix at the HTTP level is:
bk api "/builds?branch=main&state=passed"This approach also saves refactoring code to match kong when the
apicommand eventually gets its own migration to kong from cobra