-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Fix redact processor arraycopy bug #122640
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
Conversation
|
Pinging @elastic/es-data-management (Team:Data Management) |
|
Hi @joegallo, I've created a changelog YAML for you. |
davidkyle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| */ | ||
| String redactMatches(byte[] utf8Bytes, String redactStartToken, String redactEndToken) { | ||
| var merged = mergeOverlappingReplacements(replacementPositions); | ||
| int longestPatternName = merged.stream().mapToInt(r -> r.patternName.getBytes(StandardCharsets.UTF_8).length).max().getAsInt(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
longestPatternName probably was a name once but now its the number of bytes in the longest name. Suggest longestPatternSize
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 on renaming, but since it's the max() of the length of the patternName(s), I went with maxPatternNameLength.
There's a byte array buffer size issue that can come up sometimes (it can be undersized) -- we need to account for the size of the prefix and suffix when doing that math on the buffer size.