Skip to content

Commit

Permalink
Don't write errors twice when ReadRESTReq() fails
Browse files Browse the repository at this point in the history
Closes: #3490
  • Loading branch information
Alessio Treglia committed Feb 4, 2019
1 parent 0e9504c commit d18d7eb
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 50 deletions.
14 changes: 8 additions & 6 deletions client/rest/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (br BaseReq) ValidateBasic(w http.ResponseWriter) bool {

/*
ReadRESTReq is a simple convenience wrapper that reads the body and
unmarshals to the req interface.
unmarshals to the req interface. Returns false if errors occurred.
Usage:
type SomeReq struct {
Expand All @@ -107,20 +107,22 @@ unmarshals to the req interface.
}
req := new(SomeReq)
err := ReadRESTReq(w, r, cdc, req)
if ok := ReadRESTReq(w, r, cdc, req); !ok {
return
}
*/
func ReadRESTReq(w http.ResponseWriter, r *http.Request, cdc *codec.Codec, req interface{}) error {
func ReadRESTReq(w http.ResponseWriter, r *http.Request, cdc *codec.Codec, req interface{}) bool {
body, err := ioutil.ReadAll(r.Body)
if err != nil {
WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return err
return false
}

err = cdc.UnmarshalJSON(body, req)
if err != nil {
WriteErrorResponse(w, http.StatusBadRequest, fmt.Sprintf("failed to decode JSON payload: %s", err))
return err
return false
}

return nil
return true
}
3 changes: 1 addition & 2 deletions x/auth/client/rest/sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ func SignTxRequestHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.Ha
return func(w http.ResponseWriter, r *http.Request) {
var m SignBody

if err := rest.ReadRESTReq(w, r, cdc, &m); err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if !rest.ReadRESTReq(w, r, cdc, &m) {
return
}

Expand Down
3 changes: 1 addition & 2 deletions x/bank/client/rest/sendtx.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ func SendRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx context.CLIC
}

var req sendReq
err = rest.ReadRESTReq(w, r, cdc, &req)
if err != nil {
if !rest.ReadRESTReq(w, r, cdc, &req) {
return
}

Expand Down
12 changes: 4 additions & 8 deletions x/distribution/client/rest/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ func withdrawDelegatorRewardsHandlerFn(cdc *codec.Codec, cliCtx context.CLIConte
queryRoute string) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
var req withdrawRewardsReq
if err := rest.ReadRESTReq(w, r, cdc, &req); err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if !rest.ReadRESTReq(w, r, cdc, &req) {
return
}

Expand Down Expand Up @@ -96,8 +95,7 @@ func withdrawDelegationRewardsHandlerFn(cdc *codec.Codec, cliCtx context.CLICont
return func(w http.ResponseWriter, r *http.Request) {
var req withdrawRewardsReq

if err := rest.ReadRESTReq(w, r, cdc, &req); err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if !rest.ReadRESTReq(w, r, cdc, &req) {
return
}

Expand Down Expand Up @@ -137,8 +135,7 @@ func setDelegatorWithdrawalAddrHandlerFn(cdc *codec.Codec, cliCtx context.CLICon
return func(w http.ResponseWriter, r *http.Request) {
var req setWithdrawalAddrReq

if err := rest.ReadRESTReq(w, r, cdc, &req); err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if !rest.ReadRESTReq(w, r, cdc, &req) {
return
}

Expand Down Expand Up @@ -173,8 +170,7 @@ func withdrawValidatorRewardsHandlerFn(cdc *codec.Codec, cliCtx context.CLIConte
return func(w http.ResponseWriter, r *http.Request) {
var req withdrawRewardsReq

if err := rest.ReadRESTReq(w, r, cdc, &req); err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if !rest.ReadRESTReq(w, r, cdc, &req) {
return
}

Expand Down
19 changes: 6 additions & 13 deletions x/gov/client/rest/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ type voteReq struct {
func postProposalHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
var req postProposalReq
err := rest.ReadRESTReq(w, r, cdc, &req)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if !rest.ReadRESTReq(w, r, cdc, &req) {
return
}

Expand All @@ -96,8 +94,7 @@ func postProposalHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.Han

// create the message
msg := gov.NewMsgSubmitProposal(req.Title, req.Description, proposalType, req.Proposer, req.InitialDeposit)
err = msg.ValidateBasic()
if err != nil {
if err := msg.ValidateBasic(); err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return
}
Expand Down Expand Up @@ -128,8 +125,7 @@ func depositHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.HandlerF
}

var req depositReq
err := rest.ReadRESTReq(w, r, cdc, &req)
if err != nil {
if !rest.ReadRESTReq(w, r, cdc, &req) {
return
}

Expand All @@ -140,8 +136,7 @@ func depositHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.HandlerF

// create the message
msg := gov.NewMsgDeposit(req.Depositor, proposalID, req.Amount)
err = msg.ValidateBasic()
if err != nil {
if err := msg.ValidateBasic(); err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return
}
Expand Down Expand Up @@ -172,8 +167,7 @@ func voteHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.HandlerFunc
}

var req voteReq
err := rest.ReadRESTReq(w, r, cdc, &req)
if err != nil {
if !rest.ReadRESTReq(w, r, cdc, &req) {
return
}

Expand All @@ -190,8 +184,7 @@ func voteHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.HandlerFunc

// create the message
msg := gov.NewMsgVote(req.Voter, proposalID, voteOption)
err = msg.ValidateBasic()
if err != nil {
if err := msg.ValidateBasic(); err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return
}
Expand Down
3 changes: 1 addition & 2 deletions x/ibc/client/rest/transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ func TransferRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx context.
}

var req transferReq
err = rest.ReadRESTReq(w, r, cdc, &req)
if err != nil {
if !rest.ReadRESTReq(w, r, cdc, &req) {
return
}

Expand Down
3 changes: 1 addition & 2 deletions x/slashing/client/rest/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ func unjailRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx context.CL
bech32validator := vars["validatorAddr"]

var req UnjailReq
err := rest.ReadRESTReq(w, r, cdc, &req)
if err != nil {
if !rest.ReadRESTReq(w, r, cdc, &req) {
return
}

Expand Down
21 changes: 6 additions & 15 deletions x/staking/client/rest/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,7 @@ func postDelegationsHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx context.
return func(w http.ResponseWriter, r *http.Request) {
var req msgDelegationsInput

err := rest.ReadRESTReq(w, r, cdc, &req)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if !rest.ReadRESTReq(w, r, cdc, &req) {
return
}

Expand All @@ -70,8 +68,7 @@ func postDelegationsHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx context.
}

msg := staking.NewMsgDelegate(req.DelegatorAddr, req.ValidatorAddr, req.Delegation)
err = msg.ValidateBasic()
if err != nil {
if err := msg.ValidateBasic(); err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return
}
Expand Down Expand Up @@ -103,9 +100,7 @@ func postRedelegationsHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx contex
return func(w http.ResponseWriter, r *http.Request) {
var req msgBeginRedelegateInput

err := rest.ReadRESTReq(w, r, cdc, &req)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if !rest.ReadRESTReq(w, r, cdc, &req) {
return
}

Expand All @@ -115,8 +110,7 @@ func postRedelegationsHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx contex
}

msg := staking.NewMsgBeginRedelegate(req.DelegatorAddr, req.ValidatorSrcAddr, req.ValidatorDstAddr, req.SharesAmount)
err = msg.ValidateBasic()
if err != nil {
if err := msg.ValidateBasic(); err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return
}
Expand Down Expand Up @@ -148,9 +142,7 @@ func postUnbondingDelegationsHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx
return func(w http.ResponseWriter, r *http.Request) {
var req msgUndelegateInput

err := rest.ReadRESTReq(w, r, cdc, &req)
if err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
if !rest.ReadRESTReq(w, r, cdc, &req) {
return
}

Expand All @@ -160,8 +152,7 @@ func postUnbondingDelegationsHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx
}

msg := staking.NewMsgUndelegate(req.DelegatorAddr, req.ValidatorAddr, req.SharesAmount)
err = msg.ValidateBasic()
if err != nil {
if err := msg.ValidateBasic(); err != nil {
rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
return
}
Expand Down

0 comments on commit d18d7eb

Please sign in to comment.