Skip to content

Commit

Permalink
do not encode / in ref names
Browse files Browse the repository at this point in the history
fixes google#1432

Signed-off-by: Hanno Hecker <hanno@zalando.de>
  • Loading branch information
vetinari committed Feb 17, 2020
1 parent 2e3e74f commit 4989112
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 12 deletions.
5 changes: 2 additions & 3 deletions github/checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package github
import (
"context"
"fmt"
"net/url"
)

// ChecksService provides access to the Checks API in the
Expand Down Expand Up @@ -258,7 +257,7 @@ type ListCheckRunsResults struct {
//
// GitHub API docs: https://developer.github.com/v3/checks/runs/#list-check-runs-for-a-specific-ref
func (s *ChecksService) ListCheckRunsForRef(ctx context.Context, owner, repo, ref string, opts *ListCheckRunsOptions) (*ListCheckRunsResults, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/commits/%v/check-runs", owner, repo, url.QueryEscape(ref))
u := fmt.Sprintf("repos/%v/%v/commits/%v/check-runs", owner, repo, refURLEscape(ref))
u, err := addOptions(u, opts)
if err != nil {
return nil, nil, err
Expand Down Expand Up @@ -324,7 +323,7 @@ type ListCheckSuiteResults struct {
//
// GitHub API docs: https://developer.github.com/v3/checks/suites/#list-check-suites-for-a-specific-ref
func (s *ChecksService) ListCheckSuitesForRef(ctx context.Context, owner, repo, ref string, opts *ListCheckSuiteOptions) (*ListCheckSuiteResults, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/commits/%v/check-suites", owner, repo, url.QueryEscape(ref))
u := fmt.Sprintf("repos/%v/%v/commits/%v/check-suites", owner, repo, refURLEscape(ref))
u, err := addOptions(u, opts)
if err != nil {
return nil, nil, err
Expand Down
14 changes: 11 additions & 3 deletions github/git_refs.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ type updateRefRequest struct {
// GitHub API docs: https://developer.github.com/v3/git/refs/#get-a-reference
func (s *GitService) GetRef(ctx context.Context, owner string, repo string, ref string) (*Reference, *Response, error) {
ref = strings.TrimPrefix(ref, "refs/")
u := fmt.Sprintf("repos/%v/%v/git/refs/%v", owner, repo, url.QueryEscape(ref))
u := fmt.Sprintf("repos/%v/%v/git/refs/%v", owner, repo, refURLEscape(ref))
req, err := s.client.NewRequest("GET", u, nil)
if err != nil {
return nil, nil, err
Expand All @@ -79,6 +79,14 @@ func (s *GitService) GetRef(ctx context.Context, owner string, repo string, ref
return r, resp, nil
}

func refURLEscape(ref string) string {
parts := strings.Split(ref, "/")
for i, s := range parts {
parts[i] = url.QueryEscape(s)
}
return strings.Join(parts, "/")
}

// GetRefs fetches a slice of Reference objects for a given Git ref.
// If there is an exact match, only that ref is returned.
// If there is no exact match, GitHub returns all refs that start with ref.
Expand All @@ -92,7 +100,7 @@ func (s *GitService) GetRef(ctx context.Context, owner string, repo string, ref
// GitHub API docs: https://developer.github.com/v3/git/refs/#get-a-reference
func (s *GitService) GetRefs(ctx context.Context, owner string, repo string, ref string) ([]*Reference, *Response, error) {
ref = strings.TrimPrefix(ref, "refs/")
u := fmt.Sprintf("repos/%v/%v/git/refs/%v", owner, repo, url.QueryEscape(ref))
u := fmt.Sprintf("repos/%v/%v/git/refs/%v", owner, repo, refURLEscape(ref))
req, err := s.client.NewRequest("GET", u, nil)
if err != nil {
return nil, nil, err
Expand Down Expand Up @@ -212,7 +220,7 @@ func (s *GitService) UpdateRef(ctx context.Context, owner string, repo string, r
// GitHub API docs: https://developer.github.com/v3/git/refs/#delete-a-reference
func (s *GitService) DeleteRef(ctx context.Context, owner string, repo string, ref string) (*Response, error) {
ref = strings.TrimPrefix(ref, "refs/")
u := fmt.Sprintf("repos/%v/%v/git/refs/%v", owner, repo, url.QueryEscape(ref))
u := fmt.Sprintf("repos/%v/%v/git/refs/%v", owner, repo, refURLEscape(ref))
req, err := s.client.NewRequest("DELETE", u, nil)
if err != nil {
return nil, err
Expand Down
3 changes: 1 addition & 2 deletions github/repos_commits.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"bytes"
"context"
"fmt"
"net/url"
"time"
)

Expand Down Expand Up @@ -198,7 +197,7 @@ func (s *RepositoriesService) GetCommitRaw(ctx context.Context, owner string, re
//
// GitHub API docs: https://developer.github.com/v3/repos/commits/#get-the-sha-1-of-a-commit-reference
func (s *RepositoriesService) GetCommitSHA1(ctx context.Context, owner, repo, ref, lastSHA string) (string, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/commits/%v", owner, repo, url.QueryEscape(ref))
u := fmt.Sprintf("repos/%v/%v/commits/%v", owner, repo, refURLEscape(ref))

req, err := s.client.NewRequest("GET", u, nil)
if err != nil {
Expand Down
7 changes: 3 additions & 4 deletions github/repos_statuses.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package github
import (
"context"
"fmt"
"net/url"
"time"
)

Expand Down Expand Up @@ -46,7 +45,7 @@ func (r RepoStatus) String() string {
//
// GitHub API docs: https://developer.github.com/v3/repos/statuses/#list-statuses-for-a-specific-ref
func (s *RepositoriesService) ListStatuses(ctx context.Context, owner, repo, ref string, opts *ListOptions) ([]*RepoStatus, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/commits/%v/statuses", owner, repo, url.QueryEscape(ref))
u := fmt.Sprintf("repos/%v/%v/commits/%v/statuses", owner, repo, refURLEscape(ref))
u, err := addOptions(u, opts)
if err != nil {
return nil, nil, err
Expand All @@ -71,7 +70,7 @@ func (s *RepositoriesService) ListStatuses(ctx context.Context, owner, repo, ref
//
// GitHub API docs: https://developer.github.com/v3/repos/statuses/#create-a-status
func (s *RepositoriesService) CreateStatus(ctx context.Context, owner, repo, ref string, status *RepoStatus) (*RepoStatus, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/statuses/%v", owner, repo, url.QueryEscape(ref))
u := fmt.Sprintf("repos/%v/%v/statuses/%v", owner, repo, refURLEscape(ref))
req, err := s.client.NewRequest("POST", u, status)
if err != nil {
return nil, nil, err
Expand Down Expand Up @@ -110,7 +109,7 @@ func (s CombinedStatus) String() string {
//
// GitHub API docs: https://developer.github.com/v3/repos/statuses/#get-the-combined-status-for-a-specific-ref
func (s *RepositoriesService) GetCombinedStatus(ctx context.Context, owner, repo, ref string, opts *ListOptions) (*CombinedStatus, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/commits/%v/status", owner, repo, url.QueryEscape(ref))
u := fmt.Sprintf("repos/%v/%v/commits/%v/status", owner, repo, refURLEscape(ref))
u, err := addOptions(u, opts)
if err != nil {
return nil, nil, err
Expand Down

0 comments on commit 4989112

Please # to comment.