From 3c50bed6df1033d9ddd7880140c46281c689d657 Mon Sep 17 00:00:00 2001
From: davidmcleish <88298217+davidmcleish@users.noreply.github.com>
Date: Wed, 21 Dec 2022 04:26:36 +1100
Subject: [PATCH] Use PathEscape instead of QueryEscape (#49)

---
 http.go      | 6 +++---
 http_test.go | 9 +++++++++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/http.go b/http.go
index 3754b14..9a5a6d2 100644
--- a/http.go
+++ b/http.go
@@ -271,8 +271,8 @@ func (h *httpGetter) makeRequest(ctx context.Context, m string, in request, b io
 	u := fmt.Sprintf(
 		"%v%v/%v",
 		h.baseURL,
-		url.QueryEscape(in.GetGroup()),
-		url.QueryEscape(in.GetKey()),
+		url.PathEscape(in.GetGroup()),
+		url.PathEscape(in.GetKey()),
 	)
 	req, err := http.NewRequestWithContext(ctx, m, u, b)
 	if err != nil {
@@ -300,7 +300,7 @@ func (h *httpGetter) Get(ctx context.Context, in *pb.GetRequest, out *pb.GetResp
 	defer res.Body.Close()
 	if res.StatusCode != http.StatusOK {
 		msg, _ := ioutil.ReadAll(io.LimitReader(res.Body, 1024*1024)) // Limit reading the error body to max 1 MiB
-		return fmt.Errorf("server returned: %v, %v", res.Status, msg)
+		return fmt.Errorf("server returned: %v, %v", res.Status, string(msg))
 	}
 	b := bufferPool.Get().(*bytes.Buffer)
 	b.Reset()
diff --git a/http_test.go b/http_test.go
index 86b6d34..c7b7c20 100644
--- a/http_test.go
+++ b/http_test.go
@@ -174,6 +174,15 @@ func TestHTTPPool(t *testing.T) {
 	if !bytes.Equal(setValue, getValue.ByteSlice()) {
 		t.Fatal(errors.New(fmt.Sprintf("incorrect value retrieved after set: %s", getValue)))
 	}
+
+	// Key with non-URL characters to test URL encoding roundtrip
+	key = "a b/c,d"
+	if err := g.Get(ctx, key, StringSink(&value)); err != nil {
+		t.Fatal(err)
+	}
+	if suffix := ":" + key; !strings.HasSuffix(value, suffix) {
+		t.Errorf("Get(%q) = %q, want value ending in %q", key, value, suffix)
+	}
 }
 
 func testKeys(n int) (keys []string) {