fmtsafe: allow string(RedactableString) as safe arg#70979
fmtsafe: allow string(RedactableString) as safe arg#70979tbg wants to merge 1 commit intocockroachdb:masterfrom
Conversation
It's safe to use a RedactableString as a format argument instead of a constant string. This teaches the linter that. Release note: None
|
This change is |
knz
left a comment
There was a problem hiding this comment.
This does not seem correct. Say, I have this:
f := redact.SafeString("‹%s seecret›")
s := redact.Sprintf("abc") // s == "‹abc›"
out := redact.Sprintf(s, f) // out == "‹‹abc› seecret›"The linter is really there to preserve the invariant that the format string is a literal constant:
- either it is actually a literal constant
- or it forwarding an incoming format parameter, which recursively is provably a literal constant
If there's one thing missing, is that we should probably assert that the literal string constant does not contain redaction markers. but allowing the format to be hidden via a variable doesn't seem to be the right way to go?
Reviewable status:
complete! 0 of 0 LGTMs obtained
knz
left a comment
There was a problem hiding this comment.
(I meant fmt.Sprintf(f, s) in the example but the remainder of the argument remains the same)
Reviewable status:
|
You're right, let's solve the original problem in a different way. (And there is no rush on doing so). |
It's safe to use a RedactableString as a format argument instead
of a constant string. This teaches the linter that.
Needed for #70485
Release note: None