Use PathEscape on search query instead of QueryEscape (#966)

Fixes #963.
google · Aug 8, 2018 · 8c40ff4 · 8c40ff4
1 parent fbd7fb9
commit 8c40ff4
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 5 deletions.
diff --git a/github/search.go b/github/search.go
@@ -8,7 +8,9 @@ package github
 import (
 	"context"
 	"fmt"
+	"net/url"
 	"strconv"
+	"strings"
 
 	qs "github.com/google/go-querystring/query"
 )
@@ -221,11 +223,15 @@ func (s *SearchService) search(ctx context.Context, searchType string, parameter
 	if err != nil {
 		return nil, err
 	}
-	params.Set("q", parameters.Query)
+	q := strings.Replace(parameters.Query, " ", "+", -1)
 	if parameters.RepositoryID != nil {
 		params.Set("repository_id", strconv.FormatInt(*parameters.RepositoryID, 10))
 	}
-	u := fmt.Sprintf("search/%s?%s", searchType, params.Encode())
+	query := "q=" + url.PathEscape(q)
+	if v := params.Encode(); v != "" {
+		query = query + "&" + v
+	}
+	u := fmt.Sprintf("search/%s?%s", searchType, query)
 
 	req, err := s.client.NewRequest("GET", u, nil)
 	if err != nil {

diff --git a/github/search_test.go b/github/search_test.go
@@ -111,25 +111,71 @@ func TestSearchService_Issues(t *testing.T) {
 	}
 }
 
-func TestSearchService_Issues_withQualifiers(t *testing.T) {
+func TestSearchService_Issues_withQualifiersNoOpts(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()
 
+	const q = "gopher is:issue label:bug language:go pushed:>=2018-01-01 stars:>=200"
+
+	var requestURI string
 	mux.HandleFunc("/search/issues", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
 		testFormValues(t, r, values{
-			"q": "gopher is:issue label:bug language:go",
+			"q": q,
 		})
+		requestURI = r.RequestURI
 
 		fmt.Fprint(w, `{"total_count": 4, "incomplete_results": true, "items": [{"number":1},{"number":2}]}`)
 	})
 
 	opts := &SearchOptions{}
-	result, _, err := client.Search.Issues(context.Background(), "gopher is:issue label:bug language:go", opts)
+	result, _, err := client.Search.Issues(context.Background(), q, opts)
 	if err != nil {
 		t.Errorf("Search.Issues returned error: %v", err)
 	}
 
+	if want := "/api-v3/search/issues?q=gopher+is:issue+label:bug+language:go+pushed:%3E=2018-01-01+stars:%3E=200"; requestURI != want {
+		t.Fatalf("URI encoding failed: got %v, want %v", requestURI, want)
+	}
+
+	want := &IssuesSearchResult{
+		Total:             Int(4),
+		IncompleteResults: Bool(true),
+		Issues:            []Issue{{Number: Int(1)}, {Number: Int(2)}},
+	}
+	if !reflect.DeepEqual(result, want) {
+		t.Errorf("Search.Issues returned %+v, want %+v", result, want)
+	}
+}
+
+func TestSearchService_Issues_withQualifiersAndOpts(t *testing.T) {
+	client, mux, _, teardown := setup()
+	defer teardown()
+
+	const q = "gopher is:issue label:bug language:go pushed:>=2018-01-01 stars:>=200"
+
+	var requestURI string
+	mux.HandleFunc("/search/issues", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		testFormValues(t, r, values{
+			"q":    q,
+			"sort": "forks",
+		})
+		requestURI = r.RequestURI
+
+		fmt.Fprint(w, `{"total_count": 4, "incomplete_results": true, "items": [{"number":1},{"number":2}]}`)
+	})
+
+	opts := &SearchOptions{Sort: "forks"}
+	result, _, err := client.Search.Issues(context.Background(), q, opts)
+	if err != nil {
+		t.Errorf("Search.Issues returned error: %v", err)
+	}
+
+	if want := "/api-v3/search/issues?q=gopher+is:issue+label:bug+language:go+pushed:%3E=2018-01-01+stars:%3E=200&sort=forks"; requestURI != want {
+		t.Fatalf("URI encoding failed: got %v, want %v", requestURI, want)
+	}
+
 	want := &IssuesSearchResult{
 		Total:             Int(4),
 		IncompleteResults: Bool(true),