fix: accept case-insensitive HTTPS schemes in Thrift connections - #953
Open
Shubham-Padkonde wants to merge 2 commits into
Open
Shubham-Padkonde wants to merge 2 commits into
Shubham-Padkonde wants to merge 2 commits into
Conversation
Signed-off-by: Shubham Padkonde <shubhampadkonde12@gmail.com>
vuanhphung
added
engineer-bot
vuanhphung
reviewed
Sep 30, 2026
| host=server_hostname.rstrip("/"), port=port, path=http_path.lstrip("/") | ||
| ) | ||
| if not uri.startswith("https://"): | ||
| if not uri.lower().startswith("https://"): |
Collaborator
There was a problem hiding this comment.
Should we reuse the existing normalize_host_with_protocol helper in common/url_utils.py?
Signed-off-by: Shubham Padkonde <shubhampadkonde12@gmail.com>
This branch has not been 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.
What type of PR is this?
Description
An explicit
HTTPS://or mixed-caseHtTpS://prefix inserver_hostnameis currently prefixed again withhttps://. The Thrift transport then parseshttpsas the hostname instead of the workspace hostname.Check the HTTPS prefix case-insensitively while preserving the original URI. Added transport-level regression coverage for uppercase and mixed-case schemes, with lowercase and scheme-less controls, checking the resulting hostname, port, and path.
This addresses the Thrift backend portion of #950. The separately reported Rust kernel behavior is outside this change.
How is this tested?
Windows / Python 3.13:
python -m pytest tests/unit/test_thrift_backend.py -q: 71 passed, 87 subtests passed.python -m pytest tests/unit -m "not realkernel" -q: 1,018 passed, 5 skipped, 1 deselected, 359 subtests passed (11 warnings).git diff --checkpass.No live Databricks workspace or optional Rust kernel tests were run.
Related Tickets & Documents
Related to #950 (Thrift portion).
Prepared with Codex assistance. The contributor personally reviewed and signed off the commit.