Skip to content

Repository navigation

fix(github): Percent-encode branch names and refs in URL paths #175

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

Open
joseph-sentry wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(github): Percent-encode branch names and refs in URL paths #175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions src/scm/providers/github/provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from datetime import date, datetime
from email.utils import format_datetime, parsedate_to_datetime
from typing import Any, Literal, cast
from urllib.parse import quote

import msgspec
import requests
Expand Down Expand Up @@ -432,6 +433,12 @@ def _extract_response_meta(response: requests.Response) -> ResponseMeta:
return meta


def _quote_ref(ref: str) -> str:
"""Percent-encode a branch name, ref or SHA for a URL path; ``#``, ``%`` and ``?`` are legal in
git refs but would otherwise end or corrupt the path. ``/`` stays literal, as GitHub expects."""
return quote(ref, safe="/")


class GitHubProvider:
def __init__(
self,
Expand Down Expand Up @@ -945,7 +952,7 @@ def get_branch(
request_options: RequestOptions | None = None,
) -> ActionResult[GitRef]:
response = self.get(
f"/repos/{self.repository['name']}/branches/{branch}",
f"/repos/{self.repository['name']}/branches/{_quote_ref(branch)}",
request_options=request_options,
)
return map_action(response, lambda r: GitRef(ref=r["name"], sha=r["commit"]["sha"]))
Expand All @@ -963,7 +970,7 @@ def create_branch(self, branch: BranchName, sha: SHA) -> ActionResult[GitRef]:

def update_branch(self, branch: BranchName, sha: SHA, force: bool = False) -> ActionResult[GitRef]:
response = self.patch(
f"/repos/{self.repository['name']}/git/refs/heads/{branch}",
f"/repos/{self.repository['name']}/git/refs/heads/{_quote_ref(branch)}",
data={"sha": sha, "force": force},
)
return map_action(
Expand All @@ -972,7 +979,7 @@ def update_branch(self, branch: BranchName, sha: SHA, force: bool = False) -> Ac
)

def delete_branch(self, branch: BranchName) -> None:
self.delete(f"/repos/{self.repository['name']}/git/refs/heads/{branch}")
self.delete(f"/repos/{self.repository['name']}/git/refs/heads/{_quote_ref(branch)}")

def _assert_branch_head(self, branch: BranchName, expected_head_sha: SHA) -> None:
head_sha = self.get_branch(branch)["data"]["sha"]
Expand All @@ -987,7 +994,7 @@ def get_git_ref(
request_options: RequestOptions | None = None,
) -> ActionResult[GitRef]:
response = self.get(
f"/repos/{self.repository['name']}/git/ref/{ref}",
f"/repos/{self.repository['name']}/git/ref/{_quote_ref(ref)}",
request_options=request_options,
)
return map_action(response, lambda r: GitRef(ref=r["ref"], sha=r["object"]["sha"]))
Expand Down Expand Up @@ -1163,7 +1170,7 @@ def get_commit(
request_options: RequestOptions | None = None,
) -> ActionResult[CommitWithChanges]:
response = self.get(
f"/repos/{self.repository['name']}/commits/{sha}",
f"/repos/{self.repository['name']}/commits/{_quote_ref(sha)}",
request_options=request_options,
)
return map_action(response, map_commit_with_changes)
Expand All @@ -1175,7 +1182,7 @@ def get_commit_changes(
request_options: RequestOptions | None = None,
) -> PaginatedActionResult[list[CommitFile]]:
response = self.get(
f"/repos/{self.repository['name']}/commits/{sha}",
f"/repos/{self.repository['name']}/commits/{_quote_ref(sha)}",
pagination=pagination,
request_options=request_options,
)
Expand Down Expand Up @@ -1243,7 +1250,7 @@ def compare_commits(
) -> PaginatedActionResult[CommitComparison]:
"""Compare two commits. ``include_behind`` is a no-op: GitHub always returns ``behind_by``."""
response = self.get(
f"/repos/{self.repository['name']}/compare/{start_sha}...{end_sha}",
f"/repos/{self.repository['name']}/compare/{_quote_ref(start_sha)}...{_quote_ref(end_sha)}",
pagination=pagination,
request_options=request_options,
)
Expand Down Expand Up @@ -1897,7 +1904,7 @@ def list_check_runs_for_ref(
params["filter"] = timestamp_filter

response = self.get(
f"/repos/{self.repository['name']}/commits/{ref}/check-runs",
f"/repos/{self.repository['name']}/commits/{_quote_ref(ref)}/check-runs",
params=params,
pagination=pagination,
request_options=request_options,
Expand Down Expand Up @@ -1951,7 +1958,7 @@ def get_archive_link(
request_options: RequestOptions | None = None,
) -> ActionResult[ArchiveLink]:
response = self.get(
f"/repos/{self.repository['name']}/{GITHUB_ARCHIVE_FORMAT_MAP[archive_format]}/{ref}",
f"/repos/{self.repository['name']}/{GITHUB_ARCHIVE_FORMAT_MAP[archive_format]}/{_quote_ref(ref)}",
request_options=request_options,
allow_redirects=False,
)
Expand All @@ -1972,7 +1979,7 @@ def download_archive(
request_options: RequestOptions | None = None,
) -> requests.Response:
return self.get(
f"/repos/{self.repository['name']}/{GITHUB_ARCHIVE_FORMAT_MAP[archive_format]}/{ref}",
f"/repos/{self.repository['name']}/{GITHUB_ARCHIVE_FORMAT_MAP[archive_format]}/{_quote_ref(ref)}",
request_options=request_options,
)

Expand Down
31 changes: 31 additions & 0 deletions tests/unit/provider/test_github.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,14 @@ def expected_workflow_job(raw: dict[str, Any]) -> dict[str, Any]:
"raw": BRANCH_RAW,
"expected_data": expected_git_ref_from_branch(BRANCH_RAW),
},
{
"name": "get_branch",
"operation": "get",
"kwargs": {"branch": "feature/#12-50%-fix"},
"path": "/repos/test-org/test-repo/branches/feature/%2312-50%25-fix",
"raw": BRANCH_RAW,
"expected_data": expected_git_ref_from_branch(BRANCH_RAW),
},
{
"name": "create_branch",
"operation": "post",
Expand All @@ -1022,6 +1030,15 @@ def expected_workflow_job(raw: dict[str, Any]) -> dict[str, Any]:
"raw": GIT_REF_RAW,
"expected_data": expected_git_ref(GIT_REF_RAW),
},
{
"name": "update_branch",
"operation": "patch",
"kwargs": {"branch": "feature/#12-50%-fix", "sha": "abc123", "force": True},
"path": "/repos/test-org/test-repo/git/refs/heads/feature/%2312-50%25-fix",
"data": {"sha": "abc123", "force": True},
"raw": GIT_REF_RAW,
"expected_data": expected_git_ref(GIT_REF_RAW),
},
{
"name": "get_git_ref",
"operation": "get",
Expand All @@ -1030,6 +1047,14 @@ def expected_workflow_job(raw: dict[str, Any]) -> dict[str, Any]:
"raw": GIT_REF_RAW,
"expected_data": expected_git_ref_full(GIT_REF_RAW),
},
{
"name": "get_git_ref",
"operation": "get",
"kwargs": {"ref": "heads/feature/#12"},
"path": "/repos/test-org/test-repo/git/ref/heads/feature/%2312",
"raw": GIT_REF_RAW,
"expected_data": expected_git_ref_full(GIT_REF_RAW),
},
{
"name": "create_git_blob",
"operation": "post",
Expand Down Expand Up @@ -1377,6 +1402,12 @@ def expected_workflow_job(raw: dict[str, Any]) -> dict[str, Any]:
"kwargs": {"branch": "feature"},
"path": "/repos/test-org/test-repo/git/refs/heads/feature",
},
{
"name": "delete_branch",
"operation": "delete",
"kwargs": {"branch": "feature/#12-50%-fix"},
"path": "/repos/test-org/test-repo/git/refs/heads/feature/%2312-50%25-fix",
},
{
"name": "delete_issue_comment",
"operation": "delete",
Expand Down
Loading