Address review feedback

stellar · Jun 1, 2020 · 262c1ff · 262c1ff
1 parent c065c6f
commit 262c1ff
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 12 deletions.
diff --git a/services/horizon/internal/actions/offer.go b/services/horizon/internal/actions/offer.go
@@ -2,15 +2,13 @@ package actions
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 
 	"github.com/stellar/go/protocols/horizon"
 	"github.com/stellar/go/services/horizon/internal/db2/history"
 	"github.com/stellar/go/services/horizon/internal/resourceadapter"
 	"github.com/stellar/go/support/errors"
 	"github.com/stellar/go/support/render/hal"
-	"github.com/stellar/go/support/render/problem"
 )
 
 // GetOfferByID is the action handler for the /offers/{id} endpoint
@@ -94,15 +92,11 @@ func (handler GetOffersHandler) GetResourcePage(
 
 	selling, err := qp.Selling()
 	if err != nil {
-		p := problem.BadRequest
-		p.Extras = map[string]interface{}{"reason": fmt.Sprintf("bad selling asset: %s", err.Error())}
-		return nil, p
+		return nil, err
 	}
 	buying, err := qp.Buying()
 	if err != nil {
-		p := problem.BadRequest
-		p.Extras = map[string]interface{}{"reason": fmt.Sprintf("bad buying asset: %s", err.Error())}
-		return nil, p
+		return nil, err
 	}
 
 	query := history.OffersQuery{

diff --git a/services/horizon/internal/actions/offer_test.go b/services/horizon/internal/actions/offer_test.go
@@ -393,6 +393,31 @@ func TestGetOffersHandler(t *testing.T) {
 			tt.Assert.Equal(asset, offer.Buying)
 		}
 	})
+
+	t.Run("Wrong buying query parameter", func(t *testing.T) {
+		asset := horizon.Asset{}
+		eurAsset.Extract(&asset.Type, &asset.Code, &asset.Issuer)
+
+		_, err := handler.GetResourcePage(
+			httptest.NewRecorder(),
+			makeRequest(
+				t,
+				map[string]string{
+					"buying": `native\\u0026cursor=\\u0026limit=10\\u0026order=asc\\u0026selling=BTC:GAEDZ7BHMDYEMU6IJT3CTTGDUSLZWS5CQWZHGP4XUOIDG5ISH3AFAEK2`,
+				},
+				map[string]string{},
+				q.Session,
+			),
+		)
+		tt.Assert.Error(err)
+		p, ok := err.(*problem.P)
+		if tt.Assert.True(ok) {
+			tt.Assert.Equal(400, p.Status)
+			tt.Assert.NotNil(p.Extras)
+			tt.Assert.Equal(p.Extras["invalid_field"], "buying")
+			tt.Assert.Equal(p.Extras["reason"], "Asset code length is invalid")
+		}
+	})
 }
 
 func TestGetAccountOffersHandler(t *testing.T) {

diff --git a/services/horizon/internal/actions/query_params.go b/services/horizon/internal/actions/query_params.go
@@ -70,7 +70,10 @@ func (q SellingBuyingAssetQueryParams) Selling() (*xdr.Asset, error) {
 			}
 			asset, err := xdr.NewCreditAsset(parts[0], parts[1])
 			if err != nil {
-				return nil, err
+				return nil, problem.MakeInvalidFieldProblem(
+					"selling",
+					err,
+				)
 			}
 			return &asset, err
 		}
@@ -87,7 +90,9 @@ func (q SellingBuyingAssetQueryParams) Selling() (*xdr.Asset, error) {
 	)
 
 	if err != nil {
-		return nil, err
+		p := problem.BadRequest
+		p.Extras = map[string]interface{}{"reason": fmt.Sprintf("bad selling asset: %s", err.Error())}
+		return nil, p
 	}
 
 	return &selling, nil
@@ -110,7 +115,10 @@ func (q SellingBuyingAssetQueryParams) Buying() (*xdr.Asset, error) {
 			}
 			asset, err := xdr.NewCreditAsset(parts[0], parts[1])
 			if err != nil {
-				return nil, err
+				return nil, problem.MakeInvalidFieldProblem(
+					"buying",
+					err,
+				)
 			}
 			return &asset, err
 		}
@@ -127,7 +135,9 @@ func (q SellingBuyingAssetQueryParams) Buying() (*xdr.Asset, error) {
 	)
 
 	if err != nil {
-		return nil, err
+		p := problem.BadRequest
+		p.Extras = map[string]interface{}{"reason": fmt.Sprintf("bad buying asset: %s", err.Error())}
+		return nil, p
 	}
 
 	return &buying, nil