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

mapstr: fix M.Clone behaviour for arrays of M #262

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

efd6
Copy link
Contributor

@efd6 efd6 commented Dec 18, 2024

What does this PR do?

The current implementation does not clone in branches of slices in a mapstr.M, resulting in cases where the clone will share values with the original and allowing unexpected mutation of the original. This change fixes that by adding slices type handling. Note that this will not fix cases of unexpected sharing where the node value type is not a mapstr.M or map[string]any, so structs that are included with pointer fields will still show this effect.

Why is it important?

The current implementation results in data corruption when logging mapstr.M values with redacted fields.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works

Author's Checklist

  • [ ]

Related issues

The current implementation does not clone in branches of slices in a
mapstr.M, resulting in cases where the clone will share values with the
original and allowing unexpected mutation of the original. This change
fixes that by adding slices type handling. Note that this will not fix
cases of unexpected sharing where the node value type is not a mapstr.M
or map[string]any, so structs that are included with pointer fields will
still show this effect.
@efd6 efd6 added bug Something isn't working Team:Security-External Integrations Label for the Security External Integrations team labels Dec 18, 2024
@efd6 efd6 self-assigned this Dec 18, 2024
@efd6
Copy link
Contributor Author

efd6 commented Dec 18, 2024

/test

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @efd6

@efd6 efd6 marked this pull request as ready for review December 18, 2024 01:53
@efd6 efd6 requested a review from a team as a code owner December 18, 2024 01:53
@efd6 efd6 requested review from mauri870 and faec and removed request for a team December 18, 2024 01:53
@ycombinator ycombinator added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team Team:Security-External Integrations Label for the Security External Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants