fix: Client URL param encoding

strangelove-ventures · Sep 27, 2023 · e36d882 · e36d882
1 parent a7175f5
commit e36d882
Show file tree

Hide file tree

Showing 9 changed files with 37 additions and 27 deletions.
diff --git a/cosmos/rest_account_balance.go b/cosmos/rest_account_balance.go
@@ -32,7 +32,7 @@ func (c RestClient) AccountBalance(ctx context.Context, account, denom string) (
 		}
 	}
 
-	err := c.get(ctx, u.String(), &resp)
+	err := c.get(ctx, u, &resp)
 	if err != nil {
 		return AccountBalance{}, err
 	}

diff --git a/cosmos/rest_account_balance_test.go b/cosmos/rest_account_balance_test.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"io"
 	"net/http"
+	"net/url"
 	"strings"
 	"testing"
 
@@ -22,9 +23,10 @@ func TestAccountBalance(t *testing.T) {
 			account = "cosmos123"
 			denom   = "ustake"
 		)
-		httpClient.GetFn = func(ctx context.Context, path string) (*http.Response, error) {
+		httpClient.GetFn = func(ctx context.Context, path url.URL) (*http.Response, error) {
 			require.NotNil(t, ctx)
-			require.Equal(t, "/cosmos/bank/v1beta1/balances/cosmos123/by_denom?denom=ustake", path)
+			require.Equal(t, "/cosmos/bank/v1beta1/balances/cosmos123/by_denom", path.Path)
+			require.Equal(t, "denom=ustake", path.RawQuery)
 
 			const response = `{
   "balance": {
@@ -51,7 +53,7 @@ func TestAccountBalance(t *testing.T) {
 
 	t.Run("error", func(t *testing.T) {
 		var httpClient mockHTTPClient
-		httpClient.GetFn = func(ctx context.Context, path string) (*http.Response, error) {
+		httpClient.GetFn = func(ctx context.Context, _ url.URL) (*http.Response, error) {
 			return nil, errors.New("boom")
 		}
 		client := NewRestClient(&httpClient)

diff --git a/cosmos/rest_client.go b/cosmos/rest_client.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"net/url"
 )
 
 // RestClient is a client for the Cosmos REST API.
@@ -14,7 +15,7 @@ type RestClient struct {
 }
 
 type HTTPClient interface {
-	Get(ctx context.Context, path string) (*http.Response, error)
+	Get(ctx context.Context, path url.URL) (*http.Response, error)
 }
 
 func NewRestClient(c HTTPClient) *RestClient {
@@ -24,8 +25,8 @@ func NewRestClient(c HTTPClient) *RestClient {
 }
 
 // response must be a pointer to a datatype (typically a struct)
-func (c RestClient) get(ctx context.Context, url string, response any) error {
-	resp, err := c.client.Get(ctx, url)
+func (c RestClient) get(ctx context.Context, path url.URL, response any) error {
+	resp, err := c.client.Get(ctx, path)
 	if err != nil {
 		return err
 	}

diff --git a/cosmos/rest_latest_block.go b/cosmos/rest_latest_block.go
@@ -2,6 +2,7 @@ package cosmos
 
 import (
 	"context"
+	"net/url"
 	"time"
 )
 
@@ -67,6 +68,6 @@ type Block struct {
 // LatestBlock queries the latest block from the Cosmos REST API given the baseURL.
 func (c RestClient) LatestBlock(ctx context.Context) (Block, error) {
 	var latestBlock Block
-	err := c.get(ctx, "/cosmos/base/tendermint/v1beta1/blocks/latest", &latestBlock)
+	err := c.get(ctx, url.URL{Path: "/cosmos/base/tendermint/v1beta1/blocks/latest"}, &latestBlock)
 	return latestBlock, err
 }
diff --git a/cosmos/rest_latest_block_test.go b/cosmos/rest_latest_block_test.go
@@ -7,6 +7,7 @@ import (
 	"errors"
 	"io"
 	"net/http"
+	"net/url"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -16,10 +17,10 @@ import (
 var latestBlockFixture []byte
 
 type mockHTTPClient struct {
-	GetFn func(ctx context.Context, path string) (*http.Response, error)
+	GetFn func(ctx context.Context, path url.URL) (*http.Response, error)
 }
 
-func (m mockHTTPClient) Get(ctx context.Context, path string) (*http.Response, error) {
+func (m mockHTTPClient) Get(ctx context.Context, path url.URL) (*http.Response, error) {
 	if m.GetFn != nil {
 		return m.GetFn(ctx, path)
 	}
@@ -33,9 +34,9 @@ func TestClient_LatestBlock(t *testing.T) {
 
 	t.Run("happy path", func(t *testing.T) {
 		var httpClient mockHTTPClient
-		httpClient.GetFn = func(ctx context.Context, path string) (*http.Response, error) {
+		httpClient.GetFn = func(ctx context.Context, path url.URL) (*http.Response, error) {
 			require.NotNil(t, ctx)
-			require.Equal(t, "/cosmos/base/tendermint/v1beta1/blocks/latest", path)
+			require.Equal(t, "/cosmos/base/tendermint/v1beta1/blocks/latest", path.Path)
 
 			return &http.Response{
 				StatusCode: 200,
@@ -53,7 +54,7 @@ func TestClient_LatestBlock(t *testing.T) {
 
 	t.Run("error", func(t *testing.T) {
 		var httpClient mockHTTPClient
-		httpClient.GetFn = func(ctx context.Context, path string) (*http.Response, error) {
+		httpClient.GetFn = func(ctx context.Context, path url.URL) (*http.Response, error) {
 			return nil, errors.New("boom")
 		}
 		client := NewRestClient(&httpClient)

diff --git a/cosmos/rest_slashing.go b/cosmos/rest_slashing.go
@@ -2,6 +2,7 @@ package cosmos
 
 import (
 	"context"
+	"net/url"
 	"path"
 	"strconv"
 	"time"
@@ -24,7 +25,7 @@ type SigningInfo struct {
 func (c RestClient) SigningInfo(ctx context.Context, consaddress string) (SigningInfo, error) {
 	p := path.Join("/cosmos/slashing/v1beta1/signing_infos", consaddress)
 	var info SigningInfo
-	err := c.get(ctx, p, &info)
+	err := c.get(ctx, url.URL{Path: p}, &info)
 	return info, err
 }
 
@@ -47,6 +48,6 @@ func (s SlashingParams) SignedBlocksWindow() float64 {
 // Docs: https://docs.cosmos.network/swagger/#/Query/SlashingParams
 func (c RestClient) SlashingParams(ctx context.Context) (SlashingParams, error) {
 	var params SlashingParams
-	err := c.get(ctx, "/cosmos/slashing/v1beta1/params", &params)
+	err := c.get(ctx, url.URL{Path: "/cosmos/slashing/v1beta1/params"}, &params)
 	return params, err
 }
diff --git a/cosmos/rest_slashing_test.go b/cosmos/rest_slashing_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"io"
 	"net/http"
+	"net/url"
 	"strings"
 	"testing"
 	"time"
@@ -15,9 +16,9 @@ func TestRestClient_SigningStatus(t *testing.T) {
 	t.Parallel()
 
 	var httpClient mockHTTPClient
-	httpClient.GetFn = func(ctx context.Context, path string) (*http.Response, error) {
+	httpClient.GetFn = func(ctx context.Context, path url.URL) (*http.Response, error) {
 		require.NotNil(t, ctx)
-		require.Equal(t, "/cosmos/slashing/v1beta1/signing_infos/cosmosvalcons123", path)
+		require.Equal(t, "/cosmos/slashing/v1beta1/signing_infos/cosmosvalcons123", path.Path)
 
 		const fixture = `{
  "val_signing_info": {
@@ -46,9 +47,9 @@ func TestRestClient_SlashingParams(t *testing.T) {
 	t.Parallel()
 
 	var httpClient mockHTTPClient
-	httpClient.GetFn = func(ctx context.Context, path string) (*http.Response, error) {
+	httpClient.GetFn = func(ctx context.Context, path url.URL) (*http.Response, error) {
 		require.NotNil(t, ctx)
-		require.Equal(t, "/cosmos/slashing/v1beta1/params", path)
+		require.Equal(t, "/cosmos/slashing/v1beta1/params", path.Path)
 
 		const fixture = `{
   "params": {

diff --git a/metrics/fallback_client.go b/metrics/fallback_client.go
@@ -37,10 +37,13 @@ func NewFallbackClient(client *http.Client, metrics ClientMetrics, hosts []url.U
 
 const unknownErrReason = "unknown"
 
-func (c FallbackClient) Get(ctx context.Context, path string) (*http.Response, error) {
+func (c FallbackClient) Get(ctx context.Context, path url.URL) (*http.Response, error) {
 	doGet := func(host url.URL) (*http.Response, error) {
 		log := c.log.With("host", host.Hostname(), "path", path, "method", http.MethodGet)
-		host.Path = path
+
+		host.Path = path.Path
+		host.RawQuery = path.RawQuery
+
 		req, err := http.NewRequestWithContext(ctx, http.MethodGet, host.String(), nil)
 		if err != nil {
 			log.Debug("Failed to create request", "error", err)

diff --git a/metrics/fallback_client_test.go b/metrics/fallback_client_test.go
@@ -53,7 +53,7 @@ func TestFallbackClient_Get(t *testing.T) {
 			return stubResp, nil
 		}
 
-		resp, err := client.Get(ctx, "/v1/foo")
+		resp, err := client.Get(ctx, url.URL{Path: "/v1/foo"})
 		require.NoError(t, resp.Body.Close())
 
 		require.NoError(t, err)
@@ -79,7 +79,7 @@ func TestFallbackClient_Get(t *testing.T) {
 			return stubResp, nil
 		}
 
-		resp, err := client.Get(ctx, "/v1/foo")
+		resp, err := client.Get(ctx, url.URL{Path: "/v1/foo"})
 		require.NoError(t, resp.Body.Close())
 
 		require.NoError(t, err)
@@ -106,7 +106,7 @@ func TestFallbackClient_Get(t *testing.T) {
 			return stubResp, nil
 		}
 
-		resp, err := client.Get(ctx, "")
+		resp, err := client.Get(ctx, url.URL{})
 		require.NoError(t, resp.Body.Close())
 
 		require.NoError(t, err)
@@ -136,7 +136,7 @@ func TestFallbackClient_Get(t *testing.T) {
 		}
 
 		//nolint
-		_, err := client.Get(ctx, "")
+		_, err := client.Get(ctx, url.URL{})
 
 		require.Error(t, err)
 	})
@@ -163,7 +163,7 @@ func TestFallbackClient_Get(t *testing.T) {
 			}
 
 			//nolint
-			_, _ = client.Get(ctx, "")
+			_, _ = client.Get(ctx, url.URL{})
 
 			require.Equal(t, "error.example.com", metrics.GotHost.Hostname(), tt)
 			require.Equal(t, tt.WantMsg, metrics.GotErrMsg, tt)
@@ -180,7 +180,7 @@ func TestFallbackClient_Get(t *testing.T) {
 		}
 
 		//nolint
-		_, _ = client.Get(ctx, "")
+		_, _ = client.Get(ctx, url.URL{})
 
 		require.Zero(t, metrics.IncClientErrCalls)
 	})