Skip to content
This repository has been archived by the owner on Sep 12, 2019. It is now read-only.

Commit

Permalink
Fix a bug in protocols.Asset.String. Add more_info field to `invali…
Browse files Browse the repository at this point in the history
…d_parameter` errors.
  • Loading branch information
bartekn committed Jun 30, 2017
1 parent 05b4fa0 commit 2c75fd5
Show file tree
Hide file tree
Showing 25 changed files with 117 additions and 95 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestRequestHandlerAuthorize(t *testing.T) {
"name": "account_id"
}
}`)
assert.Equal(t, expected, test.StringToJSONMap(responseString))
assert.Equal(t, expected, test.StringToJSONMap(responseString, "more_info"))
})
})

Expand All @@ -73,7 +73,7 @@ func TestRequestHandlerAuthorize(t *testing.T) {
"name": "asset_code"
}
}`)
assert.Equal(t, expected, test.StringToJSONMap(responseString))
assert.Equal(t, expected, test.StringToJSONMap(responseString, "more_info"))
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (rh *RequestHandler) Builder(w http.ResponseWriter, r *http.Request) {

sequenceNumber, err := strconv.ParseUint(request.SequenceNumber, 10, 64)
if err != nil {
errorResponse := protocols.NewInvalidParameterError("sequence_number", request.SequenceNumber)
errorResponse := protocols.NewInvalidParameterError("sequence_number", request.SequenceNumber, "Sequence number must be a number")
log.WithFields(errorResponse.LogData).Error(errorResponse.Error())
server.Write(w, errorResponse)
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,9 @@ func (rh *RequestHandler) Payment(w http.ResponseWriter, r *http.Request) {
}
}

_, err = keypair.Parse(destinationObject.AccountID)
if err != nil {
if !protocols.IsValidAccountID(destinationObject.AccountID) {
log.WithFields(log.Fields{"AccountId": destinationObject.AccountID}).Print("Invalid AccountId in destination")
server.Write(w, protocols.NewInvalidParameterError("destination", request.Destination))
server.Write(w, protocols.NewInvalidParameterError("destination", request.Destination, "Destination public key must start with `G`."))
return
}

Expand Down Expand Up @@ -231,7 +230,7 @@ func (rh *RequestHandler) Payment(w http.ResponseWriter, r *http.Request) {
id, err := strconv.ParseUint(memo, 10, 64)
if err != nil {
log.WithFields(log.Fields{"memo": memo}).Print("Cannot convert memo_id value to uint64")
server.Write(w, protocols.NewInvalidParameterError("memo", request.Memo))
server.Write(w, protocols.NewInvalidParameterError("memo", request.Memo, "Memo.id must be a number"))
return
}
memoMutator = b.MemoID{id}
Expand All @@ -241,7 +240,7 @@ func (rh *RequestHandler) Payment(w http.ResponseWriter, r *http.Request) {
memoBytes, err := hex.DecodeString(memo)
if err != nil || len(memoBytes) != 32 {
log.WithFields(log.Fields{"memo": memo}).Print("Cannot decode hash memo value")
server.Write(w, protocols.NewInvalidParameterError("memo", request.Memo))
server.Write(w, protocols.NewInvalidParameterError("memo", request.Memo, "Memo.hash must be 32 bytes and hex encoded."))
return
}
var b32 [32]byte
Expand All @@ -250,7 +249,7 @@ func (rh *RequestHandler) Payment(w http.ResponseWriter, r *http.Request) {
memoMutator = &b.MemoHash{hash}
default:
log.Print("Not supported memo type: ", memoType)
server.Write(w, protocols.NewInvalidParameterError("memo", request.Memo))
server.Write(w, protocols.NewInvalidParameterError("memo", request.Memo, "Memo type not supported"))
return
}

Expand Down Expand Up @@ -289,12 +288,12 @@ func (rh *RequestHandler) Payment(w http.ResponseWriter, r *http.Request) {
case tx.Err.Error() == "Asset code length is invalid":
server.Write(
w,
protocols.NewInvalidParameterError("asset_code", request.AssetCode),
protocols.NewInvalidParameterError("asset_code", request.AssetCode, "Asset code length is invalid"),
)
case strings.Contains(tx.Err.Error(), "cannot parse amount"):
server.Write(
w,
protocols.NewInvalidParameterError("amount", request.Amount),
protocols.NewInvalidParameterError("amount", request.Amount, "Cannot parse amount"),
)
default:
log.WithFields(log.Fields{"err": tx.Err}).Print("Transaction builder error")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func TestRequestHandlerPayment(t *testing.T) {
"name": "source"
}
}`)
assert.Equal(t, expected, test.StringToJSONMap(responseString))
assert.Equal(t, expected, test.StringToJSONMap(responseString, "more_info"))
})
})

Expand Down Expand Up @@ -111,7 +111,7 @@ func TestRequestHandlerPayment(t *testing.T) {
"name": "destination"
}
}`)
assert.Equal(t, expected, test.StringToJSONMap(responseString))
assert.Equal(t, expected, test.StringToJSONMap(responseString, "more_info"))
})
})

Expand Down Expand Up @@ -345,7 +345,7 @@ func TestRequestHandlerPayment(t *testing.T) {
"name": "asset_issuer"
}
}`)
assert.Equal(t, expected, test.StringToJSONMap(responseString))
assert.Equal(t, expected, test.StringToJSONMap(responseString, "more_info"))
})
})

Expand Down Expand Up @@ -395,7 +395,7 @@ func TestRequestHandlerPayment(t *testing.T) {
"name": "asset_code"
}
}`)
assert.Equal(t, expected, test.StringToJSONMap(responseString))
assert.Equal(t, expected, test.StringToJSONMap(responseString, "more_info"))
})
})

Expand Down Expand Up @@ -445,7 +445,7 @@ func TestRequestHandlerPayment(t *testing.T) {
"name": "amount"
}
}`)
assert.Equal(t, expected, test.StringToJSONMap(responseString))
assert.Equal(t, expected, test.StringToJSONMap(responseString, "more_info"))
})
})

Expand Down Expand Up @@ -565,7 +565,7 @@ func TestRequestHandlerPayment(t *testing.T) {
"name": "memo"
}
}`)
assert.Equal(t, expected, test.StringToJSONMap(responseString))
assert.Equal(t, expected, test.StringToJSONMap(responseString, "more_info"))
})

Convey("unsupported memo_type", func() {
Expand All @@ -581,7 +581,7 @@ func TestRequestHandlerPayment(t *testing.T) {
"name": "memo"
}
}`)
assert.Equal(t, expected, test.StringToJSONMap(responseString))
assert.Equal(t, expected, test.StringToJSONMap(responseString, "more_info"))
})

Convey("memo is attached to the transaction", func() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,17 @@ func (rh *RequestHandler) HandlerAuth(c web.C, w http.ResponseWriter, r *http.Re
return
}

if senderStellarToml.SigningKey == "" {
errorResponse := protocols.NewInvalidParameterError("data.sender", authData.Sender)
log.WithFields(errorResponse.LogData).Warn("No SIGNING_KEY in stellar.toml of sender")
if !protocols.IsValidAccountID(senderStellarToml.SigningKey) {
errorResponse := protocols.NewInvalidParameterError("data.sender", authData.Sender, "SIGNING_KEY in stellar.toml of sender is invalid")
log.WithFields(errorResponse.LogData).Warn("SIGNING_KEY in stellar.toml of sender is invalid")
server.Write(w, errorResponse)
return
}

// Verify signature
signatureBytes, err := base64.StdEncoding.DecodeString(authreq.Signature)
if err != nil {
errorResponse := protocols.NewInvalidParameterError("sig", authreq.Signature)
errorResponse := protocols.NewInvalidParameterError("sig", authreq.Signature, "Invalid base64 string.")
log.WithFields(errorResponse.LogData).Warn("Error decoding signature")
server.Write(w, errorResponse)
return
Expand All @@ -77,7 +77,7 @@ func (rh *RequestHandler) HandlerAuth(c web.C, w http.ResponseWriter, r *http.Re
"data": authreq.Data,
"sig": authreq.Signature,
}).Warn("Invalid signature")
errorResponse := protocols.NewInvalidParameterError("sig", authreq.Signature)
errorResponse := protocols.NewInvalidParameterError("sig", authreq.Signature, "Invalid signature.")
server.Write(w, errorResponse)
return
}
Expand All @@ -86,7 +86,7 @@ func (rh *RequestHandler) HandlerAuth(c web.C, w http.ResponseWriter, r *http.Re
var tx xdr.Transaction
_, err = xdr.Unmarshal(b64r, &tx)
if err != nil {
errorResponse := protocols.NewInvalidParameterError("data.tx", authData.Tx)
errorResponse := protocols.NewInvalidParameterError("data.tx", authData.Tx, "Error decoding Transaction XDR")
log.WithFields(log.Fields{
"err": err,
"tx": authData.Tx,
Expand All @@ -96,7 +96,7 @@ func (rh *RequestHandler) HandlerAuth(c web.C, w http.ResponseWriter, r *http.Re
}

if tx.Memo.Hash == nil {
errorResponse := protocols.NewInvalidParameterError("data.tx", authData.Tx)
errorResponse := protocols.NewInvalidParameterError("data.tx", authData.Tx, "Transaction does not contain Memo.Hash")
log.WithFields(log.Fields{"tx": authData.Tx}).Warn("Transaction does not contain Memo.Hash")
server.Write(w, errorResponse)
return
Expand All @@ -107,7 +107,7 @@ func (rh *RequestHandler) HandlerAuth(c web.C, w http.ResponseWriter, r *http.Re
memoBytes := [32]byte(*tx.Memo.Hash)

if memoPreimageHashBytes != memoBytes {
errorResponse := protocols.NewInvalidParameterError("data.tx", authData.Tx)
errorResponse := protocols.NewInvalidParameterError("data.tx", authData.Tx, "Memo preimage hash does not equal tx Memo.Hash")

h := xdr.Hash(memoPreimageHashBytes)
tx.Memo.Hash = &h
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestRequestHandlerAuth(t *testing.T) {
"code": "invalid_parameter",
"message": "Invalid parameter."
}`)
assert.Equal(t, expected, test.StringToJSONMap(responseString))
assert.Equal(t, expected, test.StringToJSONMap(responseString, "more_info"))
})

Convey("When data is invalid", func() {
Expand All @@ -100,7 +100,7 @@ func TestRequestHandlerAuth(t *testing.T) {
"code": "invalid_parameter",
"message": "Invalid parameter."
}`)
assert.Equal(t, expected, test.StringToJSONMap(responseString))
assert.Equal(t, expected, test.StringToJSONMap(responseString, "more_info"))
})

Convey("When sender's stellar.toml does not contain signing key", func() {
Expand Down Expand Up @@ -150,7 +150,7 @@ func TestRequestHandlerAuth(t *testing.T) {
"name": "data.sender"
}
}`)
assert.Equal(t, expected, test.StringToJSONMap(responseString))
assert.Equal(t, expected, test.StringToJSONMap(responseString, "more_info"))
})

Convey("When signature is invalid", func() {
Expand Down Expand Up @@ -213,7 +213,7 @@ func TestRequestHandlerAuth(t *testing.T) {
"name": "sig"
}
}`)
assert.Equal(t, expected, test.StringToJSONMap(responseString))
assert.Equal(t, expected, test.StringToJSONMap(responseString, "more_info"))
})

Convey("When all params are valid", func() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func TestRequestHandlerSend(t *testing.T) {
"name": "source"
}
}`)
assert.Equal(t, expected, test.StringToJSONMap(responseString))
assert.Equal(t, expected, test.StringToJSONMap(responseString, "more_info"))
})

Convey("When params are valid", func() {
Expand Down
8 changes: 3 additions & 5 deletions src/github.com/stellar/gateway/protocols/bridge/authorize.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

"github.com/stellar/gateway/bridge/config"
"github.com/stellar/gateway/protocols"
"github.com/stellar/go/keypair"
)

var (
Expand Down Expand Up @@ -45,9 +44,8 @@ func (request *AuthorizeRequest) Validate(allowedAssets []config.Asset, issuingA
return err
}

_, err = keypair.Parse(request.AccountID)
if err != nil {
return protocols.NewInvalidParameterError("account_id", request.AccountID)
if !protocols.IsValidAccountID(request.AccountID) {
return protocols.NewInvalidParameterError("account_id", request.AccountID, "Account ID must start with `G`.")
}

// Is asset allowed?
Expand All @@ -60,7 +58,7 @@ func (request *AuthorizeRequest) Validate(allowedAssets []config.Asset, issuingA
}

if !allowed {
return protocols.NewInvalidParameterError("asset_code", request.AssetCode)
return protocols.NewInvalidParameterError("asset_code", request.AssetCode, "Asset code not allowed.")
}

return nil
Expand Down
10 changes: 5 additions & 5 deletions src/github.com/stellar/gateway/protocols/bridge/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,11 @@ func (r BuilderRequest) Process() error {
err = json.Unmarshal(operation.RawBody, &manageData)
operationBody = manageData
default:
return protocols.NewInvalidParameterError("operations["+strconv.Itoa(i)+"][type]", string(operation.Type))
return protocols.NewInvalidParameterError("operations["+strconv.Itoa(i)+"][type]", string(operation.Type), "Invalid operation type.")
}

if err != nil {
return protocols.NewInvalidParameterError("operations["+strconv.Itoa(i)+"][body]", "", map[string]interface{}{"err": err})
return protocols.NewInvalidParameterError("operations["+strconv.Itoa(i)+"][body]", "", "Operation is invalid.", map[string]interface{}{"err": err})
}

r.Operations[i].Body = operationBody
Expand All @@ -113,12 +113,12 @@ func (r BuilderRequest) Process() error {
// Validate validates if the request is correct.
func (r BuilderRequest) Validate() error {
if !protocols.IsValidAccountID(r.Source) {
return protocols.NewInvalidParameterError("source", r.Source)
return protocols.NewInvalidParameterError("source", r.Source, "Source parameter must start with `G`.")
}

for i, signer := range r.Signers {
if !protocols.IsValidAccountID(signer) {
return protocols.NewInvalidParameterError("signers["+strconv.Itoa(i)+"]", signer)
if !protocols.IsValidSecret(signer) {
return protocols.NewInvalidParameterError("signers["+strconv.Itoa(i)+"]", signer, "Signer must start with `S`.")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ func (op AccountMergeOperationBody) ToTransactionMutator() b.TransactionMutator
// Validate validates if operation body is valid.
func (op AccountMergeOperationBody) Validate() error {
if !protocols.IsValidAccountID(op.Destination) {
return protocols.NewInvalidParameterError("destination", op.Destination)
return protocols.NewInvalidParameterError("destination", op.Destination, "Destination must start with `G`.")
}

if op.Source != nil && !protocols.IsValidAccountID(*op.Source) {
return protocols.NewInvalidParameterError("source", *op.Source)
return protocols.NewInvalidParameterError("source", *op.Source, "Source must start with `G`.")
}

return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ func (op AllowTrustOperationBody) ToTransactionMutator() b.TransactionMutator {
// Validate validates if operation body is valid.
func (op AllowTrustOperationBody) Validate() error {
if !protocols.IsValidAssetCode(op.AssetCode) {
return protocols.NewInvalidParameterError("asset_code", op.AssetCode)
return protocols.NewInvalidParameterError("asset_code", op.AssetCode, "Asset code is invalid")
}

if !protocols.IsValidAccountID(op.Trustor) {
return protocols.NewInvalidParameterError("trustor", op.Trustor)
return protocols.NewInvalidParameterError("trustor", op.Trustor, "Trustor must be a public key (starting with `G`).")
}

if op.Source != nil && !protocols.IsValidAccountID(*op.Source) {
return protocols.NewInvalidParameterError("source", *op.Source)
return protocols.NewInvalidParameterError("source", *op.Source, "Source must be a public key (starting with `G`).")
}

return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,17 @@ func (op ChangeTrustOperationBody) ToTransactionMutator() b.TransactionMutator {
// Validate validates if operation body is valid.
func (op ChangeTrustOperationBody) Validate() error {
if !op.Asset.Validate() {
return protocols.NewInvalidParameterError("asset", op.Asset.String())
return protocols.NewInvalidParameterError("asset", op.Asset.String(), "Asset is invalid.")
}

if op.Limit != nil {
if !protocols.IsValidAmount(*op.Limit) {
return protocols.NewInvalidParameterError("limit", *op.Limit)
return protocols.NewInvalidParameterError("limit", *op.Limit, "Limit is not a valid amount.")
}
}

if op.Source != nil && !protocols.IsValidAccountID(*op.Source) {
return protocols.NewInvalidParameterError("source", *op.Source)
return protocols.NewInvalidParameterError("source", *op.Source, "Source must be a public key (starting with `G`).")
}

return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ func (op CreateAccountOperationBody) ToTransactionMutator() b.TransactionMutator
// Validate validates if operation body is valid.
func (op CreateAccountOperationBody) Validate() error {
if !protocols.IsValidAccountID(op.Destination) {
return protocols.NewInvalidParameterError("destination", op.Destination)
return protocols.NewInvalidParameterError("destination", op.Destination, "Destination must be a public key (starting with `G`)")
}

if !protocols.IsValidAmount(op.StartingBalance) {
return protocols.NewInvalidParameterError("starting_balance", op.StartingBalance)
return protocols.NewInvalidParameterError("starting_balance", op.StartingBalance, "Not a valid amount.")
}

if op.Source != nil && !protocols.IsValidAccountID(*op.Source) {
return protocols.NewInvalidParameterError("source", *op.Source)
return protocols.NewInvalidParameterError("source", *op.Source, "Source must be a public key (starting with `G`)")
}

return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func (op InflationOperationBody) ToTransactionMutator() b.TransactionMutator {
// Validate validates if operation body is valid.
func (op InflationOperationBody) Validate() error {
if op.Source != nil && !protocols.IsValidAccountID(*op.Source) {
return protocols.NewInvalidParameterError("source", *op.Source)
return protocols.NewInvalidParameterError("source", *op.Source, "Source must be a public key (starting with `G`).")
}

return nil
Expand Down
Loading

0 comments on commit 2c75fd5

Please sign in to comment.