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

refactor: remove redacted message (backport #11960) #12002

Merged
merged 4 commits into from
May 20, 2022
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
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