Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

services/horizon/internal: Fix index out of range for requests with empty named url params. #1973

Merged
merged 5 commits into from
Nov 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion services/horizon/internal/actions/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}

Expand Down
19 changes: 18 additions & 1 deletion services/horizon/internal/actions/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()

Expand Down
10 changes: 8 additions & 2 deletions services/horizon/internal/actions/offer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
11 changes: 11 additions & 0 deletions services/horizon/internal/actions/offer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down