Skip to content

Commit

Permalink
barriers: use a redactable string as message payload
Browse files Browse the repository at this point in the history 
This changes the implementation of barriers to use a redactable string
as message payload. This makes it possible to include more details
(all the safe parts of the message payload) when printing out
the barrier cause.

In order to ensure safety when interacting with previous versions of
the library without this logic, we must be careful to wrap/escape the
message string received when decoding a network error. To force this
additional wrapping, we rename the error type so that it can get a
separate decode function.
  • Loading branch information
@knz
knz committed Mar 9, 2022
1 parent a73e1b6 commit 80eef64
Show file tree
Hide file tree
Showing 13 changed files with 499 additions and 474 deletions.
65 changes: 45 additions & 20 deletions barriers/barriers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func Handled(err error) error {
if err == nil {
return nil
}
return HandledWithMessage(err, err.Error())
return HandledWithSafeMessage(err, redact.Sprint(err))
}

// HandledWithMessage is like Handled except the message is overridden.
Expand All @@ -47,7 +47,14 @@ func HandledWithMessage(err error, msg string) error {
if err == nil {
return nil
}
return &barrierError{maskedErr: err, msg: msg}
return HandledWithSafeMessage(err, redact.Sprint(msg))
}

// HandledWithSafeMessage is like Handled except the message is overridden.
// This can be used e.g. to hide message details or to prevent
// downstream code to make assertions on the message's contents.
func HandledWithSafeMessage(err error, msg redact.RedactableString) error {
return &barrierErr{maskedErr: err, smsg: msg}
}

// HandledWithMessagef is like HandledWithMessagef except the message
Expand All @@ -56,34 +63,34 @@ func HandledWithMessagef(err error, format string, args ...interface{}) error {
if err == nil {
return nil
}
return &barrierError{maskedErr: err, msg: fmt.Sprintf(format, args...)}
return &barrierErr{maskedErr: err, smsg: redact.Sprintf(format, args...)}
}

// barrierError is a leaf error type. It encapsulates a chain of
// barrierErr is a leaf error type. It encapsulates a chain of
// original causes, but these causes are hidden so that they inhibit
// matching via Is() and the Cause()/Unwrap() recursions.
type barrierError struct {
type barrierErr struct {
// Message for the barrier itself.
// In the common case, the message from the masked error
// is used as-is (see Handled() above) however it is
// useful to cache it here since the masked error may
// have a long chain of wrappers and its Error() call
// may be expensive.
msg string
smsg redact.RedactableString
// Masked error chain.
maskedErr error
}

var _ error = (*barrierError)(nil)
var _ errbase.SafeDetailer = (*barrierError)(nil)
var _ errbase.SafeFormatter = (*barrierError)(nil)
var _ fmt.Formatter = (*barrierError)(nil)
var _ error = (*barrierErr)(nil)
var _ errbase.SafeDetailer = (*barrierErr)(nil)
var _ errbase.SafeFormatter = (*barrierErr)(nil)
var _ fmt.Formatter = (*barrierErr)(nil)

// barrierError is an error.
func (e *barrierError) Error() string { return e.msg }
// barrierErr is an error.
func (e *barrierErr) Error() string { return e.smsg.StripMarkers() }

// SafeDetails reports the PII-free details from the masked error.
func (e *barrierError) SafeDetails() []string {
func (e *barrierErr) SafeDetails() []string {
var details []string
for err := e.maskedErr; err != nil; err = errbase.UnwrapOnce(err) {
sd := errbase.GetSafeDetails(err)
Expand All @@ -94,10 +101,10 @@ func (e *barrierError) SafeDetails() []string {
}

// Printing a barrier reveals the details.
func (e *barrierError) Format(s fmt.State, verb rune) { errbase.FormatError(e, s, verb) }
func (e *barrierErr) Format(s fmt.State, verb rune) { errbase.FormatError(e, s, verb) }

func (e *barrierError) SafeFormatError(p errbase.Printer) (next error) {
p.Print(e.msg)
func (e *barrierErr) SafeFormatError(p errbase.Printer) (next error) {
p.Print(e.smsg)
if p.Detail() {
p.Printf("-- cause hidden behind barrier\n%+v", e.maskedErr)
}
Expand All @@ -108,19 +115,37 @@ func (e *barrierError) SafeFormatError(p errbase.Printer) (next error) {
func encodeBarrier(
ctx context.Context, err error,
) (msg string, details []string, payload proto.Message) {
e := err.(*barrierError)
e := err.(*barrierErr)
enc := errbase.EncodeError(ctx, e.maskedErr)
return e.msg, e.SafeDetails(), &enc
return string(e.smsg), e.SafeDetails(), &enc
}

// A barrier error is decoded exactly.
func decodeBarrier(ctx context.Context, msg string, _ []string, payload proto.Message) error {
enc := payload.(*errbase.EncodedError)
return &barrierError{msg: msg, maskedErr: errbase.DecodeError(ctx, *enc)}
return &barrierErr{smsg: redact.RedactableString(msg), maskedErr: errbase.DecodeError(ctx, *enc)}
}

// Previous versions of barrier errors.
func decodeBarrierPrev(ctx context.Context, msg string, _ []string, payload proto.Message) error {
enc := payload.(*errbase.EncodedError)
return &barrierErr{smsg: redact.Sprint(msg), maskedErr: errbase.DecodeError(ctx, *enc)}
}

// barrierError is the "old" type nane of barrierErr. We use a new
// name now to ensure a different decode function is used when
// importing barriers from the previous structure, where the
// message is not redactable.
type barrierError struct {
msg string
maskedErr error
}

func (b *barrierError) Error() string { return "" }

func init() {
tn := errbase.GetTypeKey((*barrierError)(nil))
errbase.RegisterLeafDecoder(errbase.GetTypeKey((*barrierError)(nil)), decodeBarrierPrev)
tn := errbase.GetTypeKey((*barrierErr)(nil))
errbase.RegisterLeafDecoder(tn, decodeBarrier)
errbase.RegisterLeafEncoder(tn, encodeBarrier)
}
14 changes: 7 additions & 7 deletions barriers/barriers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ woo
| woo
| (1) woo
| Error types: (1) *errors.errorString
Error types: (1) *barriers.barrierError`},
Error types: (1) *barriers.barrierErr`},

{"handled + handled", barriers.Handled(barriers.Handled(goErr.New("woo"))), woo, `
woo
Expand All @@ -130,8 +130,8 @@ woo
| | woo
| | (1) woo
| | Error types: (1) *errors.errorString
| Error types: (1) *barriers.barrierError
Error types: (1) *barriers.barrierError`},
| Error types: (1) *barriers.barrierErr
Error types: (1) *barriers.barrierErr`},

{"handledmsg", barriers.HandledWithMessage(goErr.New("woo"), "waa"), "waa", `
waa
Expand All @@ -140,7 +140,7 @@ waa
| woo
| (1) woo
| Error types: (1) *errors.errorString
Error types: (1) *barriers.barrierError`},
Error types: (1) *barriers.barrierErr`},

{"handledmsg + handledmsg", barriers.HandledWithMessage(
barriers.HandledWithMessage(
Expand All @@ -154,8 +154,8 @@ wuu
| | woo
| | (1) woo
| | Error types: (1) *errors.errorString
| Error types: (1) *barriers.barrierError
Error types: (1) *barriers.barrierError`},
| Error types: (1) *barriers.barrierErr
Error types: (1) *barriers.barrierErr`},

{"handled + wrapper",
barriers.Handled(
Expand All @@ -172,7 +172,7 @@ waa: woo
| | multi-line wrapper payload
| Wraps: (2) woo
| Error types: (1) *barriers_test.werrFmt (2) *errors.errorString
Error types: (1) *barriers.barrierError`},
Error types: (1) *barriers.barrierErr`},
}

for _, test := range testCases {
Expand Down
6 changes: 3 additions & 3 deletions errutil/message_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ Wraps: (4) wuu: woo
| | multi-line payload
| Wraps: (2) woo
| Error types: (1) *errutil_test.werrFmt (2) *errors.errorString
Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *errutil.withPrefix (4) *barriers.barrierError`,
Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *errutil.withPrefix (4) *barriers.barrierErr`,
},

{"assert + wrap empty",
Expand All @@ -148,7 +148,7 @@ Wraps: (3) wuu: woo
| | multi-line payload
| Wraps: (2) woo
| Error types: (1) *errutil_test.werrFmt (2) *errors.errorString
Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *barriers.barrierError`,
Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *barriers.barrierErr`,
},

{"assert + wrap empty+arg",
Expand All @@ -173,7 +173,7 @@ Wraps: (4) wuu: woo
| | multi-line payload
| Wraps: (2) woo
| Error types: (1) *errutil_test.werrFmt (2) *errors.errorString
Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *errutil.withPrefix (4) *barriers.barrierError`,
Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *errutil.withPrefix (4) *barriers.barrierErr`,
},
}

Expand Down
Loading

0 comments on commit 80eef64

Please sign in to comment.