Skip to content

Commit 5f64fee

Browse files
chughpiyushchughpiyush
authored
Escape base and head in repos_commits.go (google#1774)
Fixes google#1642.
1 parent c2c4a6d commit 5f64fee

2 files changed

Lines changed: 194 additions & 125 deletions

File tree

‎github/repos_commits.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"bytes"
1010
"context"
1111
"fmt"
12+
"net/url"
1213
"time"
1314
)
1415

@@ -224,7 +225,10 @@ func (s *RepositoriesService) GetCommitSHA1(ctx context.Context, owner, repo, re
224225
//
225226
// GitHub API docs: https://docs.github.com/en/free-pro-team@latest/rest/reference/repos/#compare-two-commits
226227
func (s *RepositoriesService) CompareCommits(ctx context.Context, owner, repo string, base, head string) (*CommitsComparison, *Response, error) {
227-
u := fmt.Sprintf("repos/%v/%v/compare/%v...%v", owner, repo, base, head)
228+
escapedBase := url.QueryEscape(base)
229+
escapedHead := url.QueryEscape(head)
230+
231+
u := fmt.Sprintf("repos/%v/%v/compare/%v...%v", owner, repo, escapedBase, escapedHead)
228232

229233
req, err := s.client.NewRequest("GET", u, nil)
230234
if err != nil {
@@ -248,7 +252,11 @@ func (s *RepositoriesService) CompareCommits(ctx context.Context, owner, repo st
248252
//
249253
// GitHub API docs: https://docs.github.com/en/free-pro-team@latest/rest/reference/repos/#compare-two-commits
250254
func (s *RepositoriesService) CompareCommitsRaw(ctx context.Context, owner, repo, base, head string, opts RawOptions) (string, *Response, error) {
251-
u := fmt.Sprintf("repos/%v/%v/compare/%v...%v", owner, repo, base, head)
255+
escapedBase := url.QueryEscape(base)
256+
escapedHead := url.QueryEscape(head)
257+
258+
u := fmt.Sprintf("repos/%v/%v/compare/%v...%v", owner, repo, escapedBase, escapedHead)
259+
252260
req, err := s.client.NewRequest("GET", u, nil)
253261
if err != nil {
254262
return "", nil, err

‎github/repos_commits_test.go‎

Lines changed: 184 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"context"
1010
"fmt"
1111
"net/http"
12+
"net/url"
1213
"reflect"
1314
"strings"
1415
"testing"
@@ -366,12 +367,28 @@ func TestRepositoriesService_TrailingPercent_GetCommitSHA1(t *testing.T) {
366367
}
367368

368369
func TestRepositoriesService_CompareCommits(t *testing.T) {
369-
client, mux, _, teardown := setup()
370-
defer teardown()
370+
testCases := []struct {
371+
base string
372+
head string
373+
}{
374+
{base: "b", head: "h"},
375+
{base: "123base", head: "head123"},
376+
{base: "`~!@#$%^&*()_+-=[]\\{}|;':\",./<>?/*-+123base", head: "head123`~!@#$%^&*()_+-=[]\\{}|;':\",./<>?/*-+"},
377+
}
371378

372-
mux.HandleFunc("/repos/o/r/compare/b...h", func(w http.ResponseWriter, r *http.Request) {
373-
testMethod(t, r, "GET")
374-
fmt.Fprintf(w, `{
379+
for _, sample := range testCases {
380+
client, mux, _, teardown := setup()
381+
382+
base := sample.base
383+
head := sample.head
384+
escapedBase := url.QueryEscape(base)
385+
escapedHead := url.QueryEscape(head)
386+
387+
pattern := fmt.Sprintf("/repos/o/r/compare/%v...%v", base, head)
388+
389+
mux.HandleFunc(pattern, func(w http.ResponseWriter, r *http.Request) {
390+
testMethod(t, r, "GET")
391+
fmt.Fprintf(w, `{
375392
"base_commit": {
376393
"sha": "s",
377394
"commit": {
@@ -398,46 +415,28 @@ func TestRepositoriesService_CompareCommits(t *testing.T) {
398415
}
399416
],
400417
"files": [ { "filename": "f" } ],
401-
"html_url": "https://github.com/o/r/compare/b...h",
418+
"html_url": "https://github.com/o/r/compare/%[1]v...%[2]v",
402419
"permalink_url": "https://github.com/o/r/compare/o:bbcd538c8e72b8c175046e27cc8f907076331401...o:0328041d1152db8ae77652d1618a02e57f745f17",
403-
"diff_url": "https://github.com/o/r/compare/b...h.diff",
404-
"patch_url": "https://github.com/o/r/compare/b...h.patch",
405-
"url": "https://api.github.com/repos/o/r/compare/b...h"
406-
}`)
407-
})
408-
409-
ctx := context.Background()
410-
got, _, err := client.Repositories.CompareCommits(ctx, "o", "r", "b", "h")
411-
if err != nil {
412-
t.Errorf("Repositories.CompareCommits returned error: %v", err)
413-
}
420+
"diff_url": "https://github.com/o/r/compare/%[1]v...%[2]v.diff",
421+
"patch_url": "https://github.com/o/r/compare/%[1]v...%[2]v.patch",
422+
"url": "https://api.github.com/repos/o/r/compare/%[1]v...%[2]v"
423+
}`, escapedBase, escapedHead)
424+
})
425+
426+
ctx := context.Background()
427+
got, _, err := client.Repositories.CompareCommits(ctx, "o", "r", base, head)
428+
if err != nil {
429+
t.Errorf("Repositories.CompareCommits returned error: %v", err)
430+
}
414431

415-
want := &CommitsComparison{
416-
BaseCommit: &RepositoryCommit{
417-
SHA: String("s"),
418-
Commit: &Commit{
419-
Author: &CommitAuthor{Name: String("n")},
420-
Committer: &CommitAuthor{Name: String("n")},
421-
Message: String("m"),
422-
Tree: &Tree{SHA: String("t")},
423-
},
424-
Author: &User{Login: String("l")},
425-
Committer: &User{Login: String("l")},
426-
Parents: []*Commit{
427-
{
428-
SHA: String("s"),
429-
},
430-
},
431-
},
432-
Status: String("s"),
433-
AheadBy: Int(1),
434-
BehindBy: Int(2),
435-
TotalCommits: Int(1),
436-
Commits: []*RepositoryCommit{
437-
{
432+
want := &CommitsComparison{
433+
BaseCommit: &RepositoryCommit{
438434
SHA: String("s"),
439435
Commit: &Commit{
440-
Author: &CommitAuthor{Name: String("n")},
436+
Author: &CommitAuthor{Name: String("n")},
437+
Committer: &CommitAuthor{Name: String("n")},
438+
Message: String("m"),
439+
Tree: &Tree{SHA: String("t")},
441440
},
442441
Author: &User{Login: String("l")},
443442
Committer: &User{Login: String("l")},
@@ -447,109 +446,171 @@ func TestRepositoriesService_CompareCommits(t *testing.T) {
447446
},
448447
},
449448
},
450-
},
451-
Files: []*CommitFile{
452-
{
453-
Filename: String("f"),
449+
Status: String("s"),
450+
AheadBy: Int(1),
451+
BehindBy: Int(2),
452+
TotalCommits: Int(1),
453+
Commits: []*RepositoryCommit{
454+
{
455+
SHA: String("s"),
456+
Commit: &Commit{
457+
Author: &CommitAuthor{Name: String("n")},
458+
},
459+
Author: &User{Login: String("l")},
460+
Committer: &User{Login: String("l")},
461+
Parents: []*Commit{
462+
{
463+
SHA: String("s"),
464+
},
465+
},
466+
},
454467
},
455-
},
456-
HTMLURL: String("https://github.com/o/r/compare/b...h"),
457-
PermalinkURL: String("https://github.com/o/r/compare/o:bbcd538c8e72b8c175046e27cc8f907076331401...o:0328041d1152db8ae77652d1618a02e57f745f17"),
458-
DiffURL: String("https://github.com/o/r/compare/b...h.diff"),
459-
PatchURL: String("https://github.com/o/r/compare/b...h.patch"),
460-
URL: String("https://api.github.com/repos/o/r/compare/b...h"),
461-
}
468+
Files: []*CommitFile{
469+
{
470+
Filename: String("f"),
471+
},
472+
},
473+
HTMLURL: String(fmt.Sprintf("https://github.com/o/r/compare/%v...%v", escapedBase, escapedHead)),
474+
PermalinkURL: String("https://github.com/o/r/compare/o:bbcd538c8e72b8c175046e27cc8f907076331401...o:0328041d1152db8ae77652d1618a02e57f745f17"),
475+
DiffURL: String(fmt.Sprintf("https://github.com/o/r/compare/%v...%v.diff", escapedBase, escapedHead)),
476+
PatchURL: String(fmt.Sprintf("https://github.com/o/r/compare/%v...%v.patch", escapedBase, escapedHead)),
477+
URL: String(fmt.Sprintf("https://api.github.com/repos/o/r/compare/%v...%v", escapedBase, escapedHead)),
478+
}
462479

463-
if !reflect.DeepEqual(got, want) {
464-
t.Errorf("Repositories.CompareCommits returned \n%+v, want \n%+v", got, want)
465-
}
480+
if !reflect.DeepEqual(got, want) {
481+
t.Errorf("Repositories.CompareCommits returned \n%+v, want \n%+v", got, want)
482+
}
466483

467-
const methodName = "CompareCommits"
468-
testBadOptions(t, methodName, func() (err error) {
469-
_, _, err = client.Repositories.CompareCommits(ctx, "\n", "\n", "\n", "\n")
470-
return err
471-
})
484+
const methodName = "CompareCommits"
485+
testBadOptions(t, methodName, func() (err error) {
486+
_, _, err = client.Repositories.CompareCommits(ctx, "\n", "\n", "\n", "\n")
487+
return err
488+
})
472489

473-
testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
474-
got, resp, err := client.Repositories.CompareCommits(ctx, "o", "r", "b", "h")
475-
if got != nil {
476-
t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got)
477-
}
478-
return resp, err
479-
})
490+
testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
491+
got, resp, err := client.Repositories.CompareCommits(ctx, "o", "r", base, head)
492+
if got != nil {
493+
t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got)
494+
}
495+
return resp, err
496+
})
497+
498+
teardown()
499+
}
480500
}
481501

482502
func TestRepositoriesService_CompareCommitsRaw_diff(t *testing.T) {
483-
client, mux, _, teardown := setup()
484-
defer teardown()
503+
testCases := []struct {
504+
base string
505+
head string
506+
}{
507+
{base: "b", head: "h"},
508+
{base: "123base", head: "head123"},
509+
{base: "`~!@#$%^&*()_+-=[]\\{}|;':\",./<>?/*-+123base", head: "head123`~!@#$%^&*()_+-=[]\\{}|;':\",./<>?/*-+"},
510+
}
511+
512+
for _, sample := range testCases {
513+
client, mux, _, teardown := setup()
514+
515+
base := sample.base
516+
head := sample.head
517+
pattern := fmt.Sprintf("/repos/o/r/compare/%v...%v", base, head)
518+
const rawStr = "@@diff content"
519+
520+
mux.HandleFunc(pattern, func(w http.ResponseWriter, r *http.Request) {
521+
testMethod(t, r, "GET")
522+
testHeader(t, r, "Accept", mediaTypeV3Diff)
523+
fmt.Fprint(w, rawStr)
524+
})
525+
526+
ctx := context.Background()
527+
got, _, err := client.Repositories.CompareCommitsRaw(ctx, "o", "r", base, head, RawOptions{Type: Diff})
528+
if err != nil {
529+
t.Fatalf("Repositories.GetCommitRaw returned error: %v", err)
530+
}
531+
want := rawStr
532+
if got != want {
533+
t.Errorf("Repositories.GetCommitRaw returned %s want %s", got, want)
534+
}
485535

486-
const rawStr = "@@diff content"
536+
const methodName = "CompareCommitsRaw"
537+
testBadOptions(t, methodName, func() (err error) {
538+
_, _, err = client.Repositories.CompareCommitsRaw(ctx, "\n", "\n", "\n", "\n", RawOptions{Type: Diff})
539+
return err
540+
})
487541

488-
mux.HandleFunc("/repos/o/r/compare/b...h", func(w http.ResponseWriter, r *http.Request) {
489-
testMethod(t, r, "GET")
490-
testHeader(t, r, "Accept", mediaTypeV3Diff)
491-
fmt.Fprint(w, rawStr)
492-
})
542+
testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
543+
got, resp, err := client.Repositories.CompareCommitsRaw(ctx, "o", "r", base, head, RawOptions{Type: Diff})
544+
if got != "" {
545+
t.Errorf("testNewRequestAndDoFailure %v = %#v, want ''", methodName, got)
546+
}
547+
return resp, err
548+
})
493549

494-
ctx := context.Background()
495-
got, _, err := client.Repositories.CompareCommitsRaw(ctx, "o", "r", "b", "h", RawOptions{Type: Diff})
496-
if err != nil {
497-
t.Fatalf("Repositories.GetCommitRaw returned error: %v", err)
550+
teardown()
498551
}
499-
want := rawStr
500-
if got != want {
501-
t.Errorf("Repositories.GetCommitRaw returned %s want %s", got, want)
502-
}
503-
504-
const methodName = "CompareCommitsRaw"
505-
testBadOptions(t, methodName, func() (err error) {
506-
_, _, err = client.Repositories.CompareCommitsRaw(ctx, "\n", "\n", "\n", "\n", RawOptions{Type: Diff})
507-
return err
508-
})
509-
510-
testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
511-
got, resp, err := client.Repositories.CompareCommitsRaw(ctx, "o", "r", "b", "h", RawOptions{Type: Diff})
512-
if got != "" {
513-
t.Errorf("testNewRequestAndDoFailure %v = %#v, want ''", methodName, got)
514-
}
515-
return resp, err
516-
})
517552
}
518553

519554
func TestRepositoriesService_CompareCommitsRaw_patch(t *testing.T) {
520-
client, mux, _, teardown := setup()
521-
defer teardown()
522-
523-
const rawStr = "@@patch content"
524-
525-
mux.HandleFunc("/repos/o/r/compare/b...h", func(w http.ResponseWriter, r *http.Request) {
526-
testMethod(t, r, "GET")
527-
testHeader(t, r, "Accept", mediaTypeV3Patch)
528-
fmt.Fprint(w, rawStr)
529-
})
555+
testCases := []struct {
556+
base string
557+
head string
558+
}{
559+
{base: "b", head: "h"},
560+
{base: "123base", head: "head123"},
561+
{base: "`~!@#$%^&*()_+-=[]\\{}|;':\",./<>?/*-+123base", head: "head123`~!@#$%^&*()_+-=[]\\{}|;':\",./<>?/*-+"},
562+
}
563+
564+
for _, sample := range testCases {
565+
client, mux, _, teardown := setup()
566+
567+
base := sample.base
568+
head := sample.head
569+
pattern := fmt.Sprintf("/repos/o/r/compare/%v...%v", base, head)
570+
const rawStr = "@@patch content"
571+
572+
mux.HandleFunc(pattern, func(w http.ResponseWriter, r *http.Request) {
573+
testMethod(t, r, "GET")
574+
testHeader(t, r, "Accept", mediaTypeV3Patch)
575+
fmt.Fprint(w, rawStr)
576+
})
577+
578+
ctx := context.Background()
579+
got, _, err := client.Repositories.CompareCommitsRaw(ctx, "o", "r", base, head, RawOptions{Type: Patch})
580+
if err != nil {
581+
t.Fatalf("Repositories.GetCommitRaw returned error: %v", err)
582+
}
583+
want := rawStr
584+
if got != want {
585+
t.Errorf("Repositories.GetCommitRaw returned %s want %s", got, want)
586+
}
530587

531-
ctx := context.Background()
532-
got, _, err := client.Repositories.CompareCommitsRaw(ctx, "o", "r", "b", "h", RawOptions{Type: Patch})
533-
if err != nil {
534-
t.Fatalf("Repositories.GetCommitRaw returned error: %v", err)
535-
}
536-
want := rawStr
537-
if got != want {
538-
t.Errorf("Repositories.GetCommitRaw returned %s want %s", got, want)
588+
teardown()
539589
}
540590
}
541591

542592
func TestRepositoriesService_CompareCommitsRaw_invalid(t *testing.T) {
543-
client, _, _, teardown := setup()
544-
defer teardown()
545-
546593
ctx := context.Background()
547-
_, _, err := client.Repositories.CompareCommitsRaw(ctx, "o", "r", "s", "h", RawOptions{100})
548-
if err == nil {
549-
t.Fatal("Repositories.GetCommitRaw should return error")
594+
595+
testCases := []struct {
596+
base string
597+
head string
598+
}{
599+
{base: "b", head: "h"},
600+
{base: "123base", head: "head123"},
601+
{base: "`~!@#$%^&*()_+-=[]\\{}|;':\",./<>?/*-+123base", head: "head123`~!@#$%^&*()_+-=[]\\{}|;':\",./<>?/*-+"},
550602
}
551-
if !strings.Contains(err.Error(), "unsupported raw type") {
552-
t.Error("Repositories.GetCommitRaw should return unsupported raw type error")
603+
604+
for _, sample := range testCases {
605+
client, _, _, teardown := setup()
606+
_, _, err := client.Repositories.CompareCommitsRaw(ctx, "o", "r", sample.base, sample.head, RawOptions{100})
607+
if err == nil {
608+
t.Fatal("Repositories.GetCommitRaw should return error")
609+
}
610+
if !strings.Contains(err.Error(), "unsupported raw type") {
611+
t.Error("Repositories.GetCommitRaw should return unsupported raw type error")
612+
}
613+
teardown()
553614
}
554615
}
555616

0 commit comments

Comments
 (0)