"/" not accepted in searchRepos 's query

[dev590t]

Description

I can't find the repository sterglee/scalalab with searchRepos

Reproduce bug

val searchRepos = gh.repos.searchRepos("sterglee/scalalab", Nil)
val response = searchRepos.unsafeRunSync()
response.result match {
  case Left(e) => println(s"Something went wrong: ${e.getMessage}")
  case Right(r) => println(r)
}

That print:

SearchReposResult(0,false,List())

Expected

But the repository exist. And I can find it with REST API

curl   -H "Accept: application/vnd.github.v3+json"   https://api.github.com/search/repositories?q=sterglee%2Fscalalab

I have also directly search sterglee/scalalab in github. github can find the repository.

searchRepos shouldn't return a empty list.

[BenFradet]

hey, did you solve the issue in the end?

[dev590t]

I have tested with gh.repos.searchRepos("47degrees/github4s", Nil), it doesn't work also.

The specification of searchRepos don't precise if / is accepted in its argument query. I'm not sure that is a bug, or is a expected behavior. So I have close the issue, and replace gh.repos.searchRepos("sterglee/scalalab", Nil) by gh.repos.searchRepos("scalalab", Nil) in my code.

But I feel it is a bug, params = Map("q" -> s"${encode(query, "utf-8")}+${searchParams.map(_.value).mkString("+")}"), this line in the implementation should allow searchRepo to accept / in query.

I reopen the issue.

[BenFradet]

curl https://api.github.com/search/repositories?q=sterglee%2Fscalalab does work so I agree there is an issue with the lib

[josiah14]

I believe I've identified root cause.

RepositoryInterpreter.scala#L77 encodes the query parameters using a java.net.URLEncoder, which converts any /s into a %2F, which is no problem so far. The issue crops up when a second encoding happens at Http4sSyntax.scala#L62 via an org.http4s.Uri encoder, which then transforms the % in the %2F into a %25, transforming the entire section into %252F. The Github API does not know how to unravel multiple layers of encoding like this, and so it most likely treats the latter 2F as literal and part of the search pattern, which results in a successful search but with zero results.

[josiah14]

java.net.URLEncoder appears to be redundant since every URL passed to the HttpClient gets encoded by Http4sSyntax.RequestBuilderOps.toUri anyway.

I think the best way to resolve this, therefore, is by stripping out the usages of java.net.URLEncoder and adding some tests to guard against a bug like this getting reintroduced in the future.

Via the changes in my branch, both of these now work:

scala> val searchRepos = gh.repos.searchRepos("47degrees/github4s", Nil)
val searchRepos: cats.effect.IO[github4s.GHResponse[github4s.domain.SearchReposResult]] = IO(...)

scala> searchRepos.unsafeRunSync()
val res0: github4s.GHResponse[github4s.domain.SearchReposResult] = GHResponse(Right(SearchReposResult(1,false,List(Repository(53343599,github4s,47degrees/github4s,User(60125571,47degrees,https://avatars.githubusercontent.com/u/60125571?v=4,https://github.com/47degrees,None,None,None,None,None,None,Some(https://api.github.com/users/47degrees/followers),Some(https://api.github.com/users/47degrees/following{/other_user}),Organization,None,None,None),false,false,false,RepoUrls(https://api.github.com/repos/47degrees/github4s,https://github.com/47degrees/github4s,git://github.com/47degrees/github4s.git,git@github.com:47degrees/github4s.git,https://github.com/47degrees/github4s.git,https://github.com/47degrees/github4s,HashMap(tags_url -> https://api.github.com/repos/...

scala> val searchRepos = gh.repos.searchRepos("repo:47degrees/github4s", Nil)
val searchRepos: cats.effect.IO[github4s.GHResponse[github4s.domain.SearchReposResult]] = IO(...)

scala> searchRepos.unsafeRunSync()
val res1: github4s.GHResponse[github4s.domain.SearchReposResult] = GHResponse(Right(SearchReposResult(1,false,List(Repository(53343599,github4s,47degrees/github4s,User(60125571,47degrees,https://avatars.githubusercontent.com/u/60125571?v=4,https://github.com/47degrees,None,None,None,None,None,None,Some(https://api.github.com/users/47degrees/followers),Some(https://api.github.com/users/47degrees/following{/other_user}),Organization,None,None,None),false,false,false,RepoUrls(https://api.github.com/repos/47degrees/github4s,https://github.com/47degrees/github4s,git://github.com/47degrees/github4s.git,git@github.com:47degrees/github4s.git,https://github.com/47degrees/github4s.git,https://github.com/47degrees/github4s,HashMap(tags_url -> https://api.github.com/repos/...

[dev590t]

I think the best way to resolve this, therefore, is by stripping out the usages of java.net.URLEncoder and adding some tests to guard against a bug like this getting reintroduced in the future.

@josiah14 , Is it possible to use the type system of Scala to guard against a bug like this?
For instance

case class NotEncodedQuery(value : String)
val queryString : NotEncodedQuery = ???
val verbatimQuery = URI(Query.unsafeFromString(Uri.encode(queryString.value)))

[josiah14]

@dev590t That's an interesting an worthwhile thought. I'm not sure there's a way to ensure the query string is not encoded though without incurring extra runtime overhead since the query string itself is provided by the library consumer. It would make sense to me to ensure we aren't multi-encoding the string within the library itself, but to assume that just because we haven't encoded it yet that the end user hasn't provided an already encoded string may be a bit presumptuous and thus difficult to really wrap into the type system in a way that seems to indicate we know for sure whether something we've received is encoded or not.

@Daenyth Do you have any input on this? I'm hoping I'm wrong and there's a reasonable way to wrap this sort of validation into the type-system without overhead (or with little-enough overhead that it's justified).

[Daenyth]

I don't think it's worth making the types more complex to fix this; we can just have a unit test in this case