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

release-20.2: keys: avoid creating invalid utf-8 sequences in logs #54272

Merged
merged 1 commit into from
Sep 11, 2020

Conversation

knz
Copy link
Contributor

@knz knz commented Sep 11, 2020

Backport 1/1 commits from #53973.

/cc @cockroachdb/release


Prior to this patch, the algorithm to truncate the pretty-print
representation of range start/end keys was able to produce invalid
utf-8 sequences, which were then included via context tags and log
messages pretty frequently in logs.

This had two shortcomings, one minor and one major:

- the minor shortcomings is that some text editors refuse to open
  a text file containing invalid utf-8 sequences as "text", which
  makes it extra hard to quickly navigate through files in e.g.
  a `cockroach debug zip` output.

- it could cause 3rd party consumers of log files with redaction
  markers to miss the start or end of redaction boundaries and
  thus mis-redact. (The go reader is immune to this, but we want to
  advertise the log format.)

Release justification: bug fix

Release note: None
@knz knz requested a review from otan September 11, 2020 17:51
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@otan
Copy link
Contributor

otan commented Sep 11, 2020

LGTM, but subject to release backport approval

@knz knz mentioned this pull request Sep 11, 2020
40 tasks
@knz
Copy link
Contributor Author

knz commented Sep 11, 2020

cc @nvanbenschoten TL in this area

@knz knz merged commit 3e6175c into cockroachdb:release-20.2 Sep 11, 2020
@knz knz deleted the backport20.2-53973 branch September 11, 2020 18:30
@otan
Copy link
Contributor

otan commented Sep 11, 2020

i thought you had to wait for release to sign off if it isn't a release blocker before merging

@knz
Copy link
Contributor Author

knz commented Sep 11, 2020

i thought you had to wait for release to sign off if it isn't a release blocker before merging

oops I was not aware. I'm going to review the process. Thanks for the heads up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants