http4s client double-encodes path segments

[kubukoz]

Path segments like foo%bar get encoded twice: once in HttpEndpoint#path and again in the endpoint's inputToRequest:

def inputToRequest(input: I): Request[F] = {
  val metadata = inputMetadataEncoder.encode(input)
  // here
  val path = httpEndpoint.path(input)
  // and here: addPath
  val uri = baseUri.addPath(path).withMultiValueQueryParams(metadata.query)
  val headers = toHeaders(metadata.headers)
  val baseRequest = Request[F](method, uri, headers = headers)
  if (inputHasBody) {
    baseRequest.withEntity(input)
  } else baseRequest
}

For context, here's addPath from http4s:

/** Splits the path segments and adds each of them to the path url-encoded.
  * A segment is delimited by /
  * @param morePath the path to add
  * @return a new uri with the segments added to the path
  */
def addPath(morePath: String): Uri =
  copy(path = morePath.split("/").foldLeft(path)((p, segment) => p.addSegment(segment)))

scala-cli reproduction:

// using scala 2.13.7
// using lib org.http4s::http4s-ember-client:0.23.7
// using lib com.disneystreaming.smithy4s::smithy4s-http4s:0.11.3
// using lib com.disneystreaming.smithy4s::smithy4s-tests:0.11.3
import cats.effect.IO
import cats.effect.IOApp
import cats.effect.kernel.Resource
import org.http4s.Response
import org.http4s.client.Client
import org.http4s.implicits._
import smithy4s.example.PizzaAdminService
import smithy4s.http4s.SimpleRestJsonBuilder

object demo extends IOApp.Simple {

  def run: IO[Unit] = {
    val client = Client[IO] { req =>
      Resource.eval(IO.println("Actual:  " + req.uri.renderString) *> IO(Response[IO]()))
    }

    SimpleRestJsonBuilder(PizzaAdminService)
      .clientResource(client, uri"http://localhost:8080")
      .use { client =>
        val expected = uri"http://localhost:8080" / "restaurant" / "hello%world" / "menu"

        IO.println("Expected: " + expected.renderString) *>
          client.getMenu("hello%world")
      }
  }.void

}

Outputs:

Expected: http://localhost:8080/restaurant/hello%25world/menu
Actual:  http://localhost:8080/restaurant/hello%2525world/menu

Additionally (this is probably a separate issue, I'm not sure), characters like colons are encoded while they shouldn't be (e.g. https://en.wikipedia.org/wiki/Template:Welcome is fine): the same code with "hello:world" as the parameter outputs:

Expected: http://localhost:8080/restaurant/hello:world/menu
Actual:  http://localhost:8080/restaurant/hello%253Aworld/menu

I'm starting to think we shouldn't be doing urlencoding at all, and path rendering should be left to the interpreters. In the case of the http4s client, we'd pass a collection of segments to baseUri.path.addSegments. I'm not 100% sure this will work with greedy labels yet.

[kubukoz]

I think the double encoding part is a regression introduced in #41, which can be blamed on me 😅 the path: String method was being built incorrectly, though. Maybe we don't need it? Maybe we can have a List[String or some newtype for Segment] instead (and as I mentioned in the original post, encode these in the interpreters)?