fix(pii): Don't redact already redacted text [INGEST-109] #1177
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If one relay redacts the whole text chunk and forwards it to the next relay they don't have to re-redact the text again, so we can early exit when there's no text to redact. This is causing an assertion to failin cases where the whole text has already been redacted. Without this patch, the method correctly returns the chunks, but there's unnecessary work done and the last assertion (`replacement_chunks.is_empty()`) fails. This patch makes that method to exit as early as possible removing that unnecessary work, including not even reaching the failing assertion.
jjbayer
reviewed
Feb 8, 2022
| @@ -945,3 +954,27 @@ fn test_ip_address_hashing_does_not_overwrite_id() { | |||
|
|
|||
| assert_eq!(user.id.value().unwrap().as_str(), "123"); | |||
| } | |||
|
|
|||
| #[test] | |||
| fn test_replace_replaced_text() { | |||
There was a problem hiding this comment.
nit: It might be nice to have an additional test with an actual payload example (something like this):
relay/relay-general/src/pii/convert.rs
Lines 1136 to 1146 in 358014e
jjbayer
approved these changes
Feb 8, 2022
|
Closing in favor of #1180. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If one relay redacts the whole text chunk and forwards it to the next relay
they don't have to re-redact the text again, so we can early exit when there's
no text to redact. This is causing an assertion to fail cases where the whole
text has already been redacted.
Without this patch, the method correctly returns the chunks, but there's
unnecessary work done, and the last assertion (
replacement_chunks.is_empty())fails. This patch makes that method exit as early as possible removing that
unnecessary work, including not even reaching the failing assertion.
Fixes #1095.