fix(csharp): prevent percent-encoding of '?' in HTTP path query parameters#26
Merged
birschick-bq merged 12 commits intoMar 12, 2026
Merged
Conversation
UriBuilder encodes '?' as '%3F' when a query string is embedded in the path argument (e.g. Databricks warehouse paths like /sql/1.0/warehouses/<id>?o=<orgId>). Fix by splitting path on '?' and assigning the query portion to UriBuilder.Query separately. Adds unit tests covering paths with and without query parameters, and the full-URI passthrough case. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
eric-wang-1990
commented
Mar 12, 2026
Contributor
Author
Code reviewFound 1 issue:
hiveserver2/csharp/test/Hive2/HiveServer2ConnectionGetBaseAddressTest.cs Lines 95 to 104 in 950c58e 🤖 Generated with Claude Code If this code review was useful, please react with 👍. Otherwise, react with 👎. |
…lection BindingFlags.FlattenHierarchy only surfaces protected static members when combined with BindingFlags.Public, not NonPublic. The previous flags caused GetMethod to return null and all tests to throw at runtime. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
eric-wang-1990
commented
Mar 12, 2026
Instantiating SparkHttpConnection triggered auth validation in the constructor, failing all tests. Reflect on typeof(HiveServer2Connection) directly instead — no instance needed for a static method, and the test assembly has InternalsVisibleTo access. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…s internal Mark GetBaseAddress as protected internal so the test assembly (which has InternalsVisibleTo access) can call it directly. This avoids brittle reflection that would silently fail at runtime on signature changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
birschick-bq
requested changes
Mar 12, 2026
birschick-bq
left a comment
Collaborator
There was a problem hiding this comment.
There is an UriBuilder overload that takes the query.
Collaborator
|
License text requirement for new test file ... |
Use the 5-parameter constructor that accepts the query string (extraValue) directly, instead of setting .Query after construction. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove the 'This file has been modified' preamble which is only for files derived from existing Apache-licensed sources. This file is new. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
birschick-bq
requested changes
Mar 12, 2026
…right Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
eric-wang-1990
added a commit
to adbc-drivers/databricks
that referenced
this pull request
Mar 12, 2026
UriBuilder was percent-encoding '?' as '%3F' in HTTP paths containing query parameters (e.g. /sql/1.0/warehouses/<id>?o=<orgId>). This caused connection failures for warehouse paths with org ID suffixes. Upstream fix: adbc-drivers/hiveserver2#26 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 task
github-merge-queue Bot
pushed a commit
to adbc-drivers/databricks
that referenced
this pull request
Mar 12, 2026
…ding (#341) ## Summary - Updates the `hiveserver2` submodule to the latest `main` which includes the fix for `UriBuilder` percent-encoding `?` as `%3F` in HTTP paths containing query parameters (e.g. `/sql/1.0/warehouses/<id>?o=<orgId>`) - This caused connection failures for Databricks warehouse paths that include an org ID suffix ## Upstream fix adbc-drivers/hiveserver2#26 ## Test plan - [ ] Verify connection succeeds with HTTP path containing `?o=<orgId>` query parameter 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
github-merge-queue Bot
pushed a commit
to adbc-drivers/databricks
that referenced
this pull request
Mar 12, 2026
…ding (#341) ## Summary - Updates the `hiveserver2` submodule to the latest `main` which includes the fix for `UriBuilder` percent-encoding `?` as `%3F` in HTTP paths containing query parameters (e.g. `/sql/1.0/warehouses/<id>?o=<orgId>`) - This caused connection failures for Databricks warehouse paths that include an org ID suffix ## Upstream fix adbc-drivers/hiveserver2#26 ## Test plan - [ ] Verify connection succeeds with HTTP path containing `?o=<orgId>` query parameter 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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
UriBuilderpercent-encodes?as%3Fwhen a query string is embedded in the path argument (e.g. Databricks warehouse paths like/sql/1.0/warehouses/<id>?o=<orgId>)?and assigning the query portion toUriBuilder.Queryseparately, usingSubstringfornet472/netstandard2.0compatibilityTest plan
GetBaseAddress_PathWithQueryParam_DoesNotPercentEncodeQuestionMark— verifies?is not encoded as%3Fand path/query are correctly separatedGetBaseAddress_PathWithoutQueryParam_ReturnsExpectedUri— verifies plain paths are unaffectedGetBaseAddress_FullUri_ReturnsUriDirectly— verifies full URI passthrough is unchanged🤖 Generated with Claude Code