services/horizon/internal: Fix index out of range for requests with e…

…mpty named url params. (#1973)

* Add regression for #1965.

* Return early if there are no values for URLParams.

* Add accountID validation on GetAccountOffersHandler

* Use query structs for GetAccountOffersHandler.

* Use GetURLParam in GetParams.

services/horizon/internal/actions/helpers.go

@@ -607,6 +607,14 @@ func (base *Base) GetTimeMillis(name string) (timeMillis time.Millis) {
 // Ref: https://github.com/go-chi/chi/blob/d132b31857e5922a2cc7963f4fcfd8f46b3f2e97/context.go#L69
 func GetURLParam(r *http.Request, key string) (string, bool) {
 	rctx := chi.RouteContext(r.Context())
+
+	// Return immediately if keys does not match Values
+	// This can happen when a named param is not specified.
+	// This is a bug in chi: https://github.com/go-chi/chi/issues/426
+	if len(rctx.URLParams.Keys) != len(rctx.URLParams.Values) {
+		return "", false
+	}
+
 	for k := len(rctx.URLParams.Keys) - 1; k >= 0; k-- {
 		if rctx.URLParams.Keys[k] == key {
 			return rctx.URLParams.Values[k], true
@@ -690,7 +698,8 @@ func GetParams(dst interface{}, r *http.Request) error {
 				)
 			}
 
-			query.Set(key, rctx.URLParam(key))
+			param, _ := GetURLParam(r, key)
+			query.Set(key, param)
 		}
 	}
 

services/horizon/internal/actions/helpers_test.go

@@ -426,7 +426,7 @@ func TestPath(t *testing.T) {
 	tt.Assert.Equal("/foo-bar/blah", action.Path())
 }
 
-func TestGetURLParam(t *testing.T) {
+func TestBaseGetURLParam(t *testing.T) {
 	tt := test.Start(t)
 	defer tt.Finish()
 	action := makeTestAction()
@@ -446,6 +446,23 @@ func TestGetURLParam(t *testing.T) {
 	tt.Assert.Equal(false, ok)
 }
 
+func TestGetURLParam(t *testing.T) {
+	tt := test.Start(t)
+	defer tt.Finish()
+	r := makeAction("/accounts/{account_id}/operations?limit=100", nil).R
+
+	// simulates a request where the named param is not passed.
+	// Regression for https://github.com/stellar/go/issues/1965
+	rctx := chi.RouteContext(r.Context())
+	rctx.URLParams.Keys = []string{
+		"account_id",
+	}
+
+	val, ok := GetURLParam(r, "account_id")
+	tt.Assert.Empty(val)
+	tt.Assert.False(ok)
+}
+
 func TestGetAssets(t *testing.T) {
 	rctx := chi.NewRouteContext()
 

services/horizon/internal/actions/offer.go

@@ -110,6 +110,11 @@ func (handler GetOffersHandler) GetResourcePage(
 	return offers, nil
 }
 
+// AccountOffersQuery query struct for offers end-point
+type AccountOffersQuery struct {
+	AccountID string `schema:"account_id" valid:"accountID,required"`
+}
+
 // GetAccountOffersHandler is the action handler for the
 // `/accounts/{account_id}/offers` endpoint when using experimental ingestion.
 type GetAccountOffersHandler struct {
@@ -121,14 +126,15 @@ func (handler GetAccountOffersHandler) parseOffersQuery(r *http.Request) (histor
 		return history.OffersQuery{}, err
 	}
 
-	seller, err := GetString(r, "account_id")
+	qp := AccountOffersQuery{}
+	err = GetParams(&qp, r)
 	if err != nil {
 		return history.OffersQuery{}, err
 	}
 
 	query := history.OffersQuery{
 		PageQuery: pq,
-		SellerID:  seller,
+		SellerID:  qp.AccountID,
 	}
 
 	return query, nil

services/horizon/internal/actions/offer_test.go

@@ -393,6 +393,17 @@ func TestGetAccountOffersHandler(t *testing.T) {
 	for _, offer := range offers {
 		tt.Assert.Equal(issuer.Address(), offer.Seller)
 	}
+
+	records, err = handler.GetResourcePage(
+		httptest.NewRecorder(),
+		makeRequest(
+			t,
+			map[string]string{},
+			map[string]string{},
+			q.Session,
+		),
+	)
+	tt.Assert.Error(err)
 }
 
 func pageableToOffers(t *testing.T, page []hal.Pageable) []horizon.Offer {