Skip to content

Commit

Permalink
refactor: remove redacted message (backport #11960) (#12002)
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] committed May 20, 2022
1 parent cf17549 commit a72d9fa
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 95 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Improvements

* (x/feegrant) [\#11813](https://github.com/cosmos/cosmos-sdk/pull/11813) Fix pagination total count in `AllowancesByGranter` query.
* (errors) [\#12002](https://github.com/cosmos/cosmos-sdk/pull/12002) Removed 'redacted' error message from defaultErrEncoder.

### Bug Fixes

Expand Down
21 changes: 2 additions & 19 deletions types/errors/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,15 @@ func debugErrEncoder(err error) string {
return fmt.Sprintf("%+v", err)
}

// The defaultErrEncoder applies Redact on the error before encoding it with its internal error message.
func defaultErrEncoder(err error) string {
return Redact(err).Error()
return err.Error()
}

type coder interface {
ABCICode() uint32
}

// abciCode test if given error contains an ABCI code and returns the value of
// abciCode tests if given error contains an ABCI code and returns the value of
// it if available. This function is testing for the causer interface as well
// and unwraps the error.
func abciCode(err error) uint32 {
Expand Down Expand Up @@ -190,19 +189,3 @@ func errIsNil(err error) bool {
}
return false
}

var errPanicWithMsg = Wrapf(ErrPanic, "error message redacted to hide potential sensitive info. Use the '--trace' flag if you are running a node to see the full stack trace error")

// Redact replaces an error that is not initialized as an ABCI Error with a
// generic internal error instance. If the error is an ABCI Error, that error is
// simply returned.
func Redact(err error) error {
if ErrPanic.Is(err) {
return errPanicWithMsg
}
if abciCode(err) == internalABCICode {
return errInternal
}

return err
}
91 changes: 15 additions & 76 deletions types/errors/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,27 +57,13 @@ func (s *abciTestSuite) TestABCInfo() {
wantCode: 0,
wantSpace: "",
},
"stdlib is generic message": {
err: io.EOF,
debug: false,
wantLog: "internal",
wantCode: 1,
wantSpace: UndefinedCodespace,
},
"stdlib returns error message in debug mode": {
err: io.EOF,
debug: true,
wantLog: "EOF",
wantCode: 1,
wantSpace: UndefinedCodespace,
},
"wrapped stdlib is only a generic message": {
err: Wrap(io.EOF, "cannot read file"),
debug: false,
wantLog: "internal",
wantCode: 1,
wantSpace: UndefinedCodespace,
},
// This is hard to test because of attached stacktrace. This
// case is tested in an another test.
//"wrapped stdlib is a full message in debug mode": {
Expand All @@ -103,10 +89,12 @@ func (s *abciTestSuite) TestABCInfo() {
}

for testName, tc := range cases {
space, code, log := ABCIInfo(tc.err, tc.debug)
s.Require().Equal(tc.wantSpace, space, testName)
s.Require().Equal(tc.wantCode, code, testName)
s.Require().Equal(tc.wantLog, log, testName)
s.T().Run(testName, func(t *testing.T) {
space, code, log := ABCIInfo(tc.err, tc.debug)
s.Require().Equal(tc.wantSpace, space, testName)
s.Require().Equal(tc.wantCode, code, testName)
s.Require().Equal(tc.wantLog, log, testName)
})
}
}

Expand Down Expand Up @@ -135,25 +123,20 @@ func (s *abciTestSuite) TestABCIInfoStacktrace() {
wantStacktrace: true,
wantErrMsg: "wrapped: stdlib",
},
"wrapped stdlib error in non-debug mode does not have stacktrace": {
err: Wrap(fmt.Errorf("stdlib"), "wrapped"),
debug: false,
wantStacktrace: false,
wantErrMsg: "internal",
},
}

const thisTestSrc = "github.com/cosmos/cosmos-sdk/types/errors.(*abciTestSuite).TestABCIInfoStacktrace"

for testName, tc := range cases {
_, _, log := ABCIInfo(tc.err, tc.debug)
if !tc.wantStacktrace {
s.Require().Equal(tc.wantErrMsg, log, testName)
continue
}

s.Require().True(strings.Contains(log, thisTestSrc), testName)
s.Require().True(strings.Contains(log, tc.wantErrMsg), testName)
s.T().Run(testName, func(t *testing.T) {
_, _, log := ABCIInfo(tc.err, tc.debug)
if !tc.wantStacktrace {
s.Require().Equal(tc.wantErrMsg, log, testName)
} else {
s.Require().True(strings.Contains(log, thisTestSrc), testName)
s.Require().True(strings.Contains(log, tc.wantErrMsg), testName)
}
})
}
}

Expand All @@ -163,46 +146,6 @@ func (s *abciTestSuite) TestABCIInfoHidesStacktrace() {
s.Require().Equal("wrapped: unauthorized", log)
}

func (s *abciTestSuite) TestRedact() {
cases := map[string]struct {
err error
untouched bool // if true we expect the same error after redact
changed error // if untouched == false, expect this error
}{
"panic looses message": {
err: Wrap(ErrPanic, "some secret stack trace"),
changed: errPanicWithMsg,
},
"sdk errors untouched": {
err: Wrap(ErrUnauthorized, "cannot drop db"),
untouched: true,
},
"pass though custom errors with ABCI code": {
err: customErr{},
untouched: true,
},
"redact stdlib error": {
err: fmt.Errorf("stdlib error"),
changed: errInternal,
},
}

for name, tc := range cases {
spec := tc
redacted := Redact(spec.err)
if spec.untouched {
s.Require().Equal(spec.err, redacted, name)
continue
}

// see if we got the expected redact
s.Require().Equal(spec.changed, redacted, name)
// make sure the ABCI code did not change
s.Require().Equal(abciCode(spec.err), abciCode(redacted), name)

}
}

func (s *abciTestSuite) TestABCIInfoSerializeErr() {
var (
// Create errors with stacktrace for equal comparison.
Expand Down Expand Up @@ -231,10 +174,6 @@ func (s *abciTestSuite) TestABCIInfoSerializeErr() {
debug: true,
exp: fmt.Sprintf("%+v", myErrDecode),
},
"redact in default encoder": {
src: myPanic,
exp: "error message redacted to hide potential sensitive info. Use the '--trace' flag if you are running a node to see the full stack trace error: panic",
},
"do not redact in debug encoder": {
src: myPanic,
debug: true,
Expand Down

0 comments on commit a72d9fa

Please sign in to comment.