diff --git a/services/horizon/internal/actions/helpers.go b/services/horizon/internal/actions/helpers.go index 295ec506ea..c708b6ff35 100644 --- a/services/horizon/internal/actions/helpers.go +++ b/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) } } diff --git a/services/horizon/internal/actions/helpers_test.go b/services/horizon/internal/actions/helpers_test.go index d9beffde95..e173b393c6 100644 --- a/services/horizon/internal/actions/helpers_test.go +++ b/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() diff --git a/services/horizon/internal/actions/offer.go b/services/horizon/internal/actions/offer.go index 7b30efce58..748209cc7a 100644 --- a/services/horizon/internal/actions/offer.go +++ b/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 diff --git a/services/horizon/internal/actions/offer_test.go b/services/horizon/internal/actions/offer_test.go index 533a217ea9..dfbac000f6 100644 --- a/services/horizon/internal/actions/offer_test.go +++ b/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 {