Skip to content
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.

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

SearchService.Repositories doesn't work well #963

Closed
lizebang opened this issue Aug 3, 2018 · 2 comments
Closed

SearchService.Repositories doesn't work well #963

lizebang opened this issue Aug 3, 2018 · 2 comments
Assignees

Comments

@lizebang
Copy link

lizebang commented Aug 3, 2018

When I use SearchService.Repositories like this, there is a error.

	client := github.NewClient(nil)
	rs, _, err := client.Search.Repositories(context.Background(), "language:golang+stars:>=200+pushed:>=2018-01-01",
		&github.SearchOptions{Sort: "stars", Order: "desc", ListOptions: github.ListOptions{Page: 2, PerPage: 100}})
	......

Error:

GET https://api.github.com/search/repositories?order=desc&page=2&per_page=100&q=language%3Agolang%2Bstars%3A%3E%3D200%2Bpushed%3A%3E%3D2018-01-01&sort=stars: 422 Validation Failed [{Resource:Search Field:q Code:invalid Message:None of the search qualifiers apply to this search type.}]

The right URL is https://api.github.com/search/repositories?q=language:golang+pushed:%3E2018-01-01+stars:%3E=200&sort=stars&order=desc&page=2&per_page=100, not https://api.github.com/search/repositories?order=desc&page=2&per_page=100&q=language%3Agolang%2Bstars%3A%3E%3D200%2Bpushed%3A%3E%3D2018-01-01&sort=stars.

@gmlewis
Copy link
Collaborator

gmlewis commented Aug 4, 2018

Ah, yes, you are quite right! Thank you, @lizebang!

Looking at search.go, line 224, we set the query parameter like this: params.Set("q", parameters.Query) and then on line 228, we encode the parameters with params.Encode.

I tested this in the embedded examples playground here: https://golang.org/pkg/net/url/#Values
(result: https://play.golang.org/p/ye2rk_hq9zG ) and confirmed that Values.Encode does not first call QueryEscape.

It looks to me like we should call QueryEscape on line 224.

Let's open this up for discussion in case my analysis is incorrect since this would be a breaking change to the API. I would like to hear if maybe existing users of this package simply call QueryEscape themselves before calling any of the search functions.

Edit: My analysis above was not correct... I forgot to call Encode to see if the parameter was escaped. Here is an updated run where the colon is escaped: https://play.golang.org/p/sDrQ2mSWmAk

@lizebang is correct below. Now we need a working solution.

@lizebang
Copy link
Author

lizebang commented Aug 4, 2018

Thank you for your reply, @gmlewis

Values.Encode calls QueryEscape, but parameters.Query should not be escaped. If parameters.Query is escaped, GitHub will think this is an invalid field and return 422 Validation Failed.

Is this correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants