Skip to content

fix(report): prevent corrupted utf8 when computing process name#269

Merged
cmacknz merged 2 commits intoelastic:mainfrom
mauri870:process-name-utf8-aware
Nov 18, 2025
Merged

fix(report): prevent corrupted utf8 when computing process name#269
cmacknz merged 2 commits intoelastic:mainfrom
mauri870:process-name-utf8-aware

Conversation

@mauri870
Copy link
Copy Markdown
Member

@mauri870 mauri870 commented Nov 17, 2025

What does this PR do?

The processName function is not UTF-8 aware. It simply cuts the string to a fixed length without checking whether the index falls in the middle of a multibyte sequence, which results in corrupted UTF-8. Fix it by truncating the string using a UTF-8 safe approach.

Why is it important?

Prevents SetupMetricsOptions from erroring out on the call to procStats.Init() when trying to apply a regex on a corrupted utf8 string here. See comment on linked issue for details.

How to test this PR locally?

# should fail on main
git checkout main -- ./report/setup.go
go test -run ^TestProcessName$ ./report -v -count=1

# and work on this PR
git stash
go test -run ^TestProcessName$ ./report -v -count=1

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
  • I have added an entry in CHANGELOG.md

Related issues

@mauri870 mauri870 self-assigned this Nov 17, 2025
@mauri870 mauri870 requested a review from a team as a code owner November 17, 2025 12:24
@mauri870 mauri870 added bug Something isn't working Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team labels Nov 17, 2025
@mauri870 mauri870 requested review from andrzej-stencel and belimawr and removed request for a team November 17, 2025 12:24
@pierrehilbert
Copy link
Copy Markdown

@mauri870 Your test is logically failing on Windows. Should we exclude the test from running there?

@mauri870 mauri870 force-pushed the process-name-utf8-aware branch from 99a6522 to f96b89c Compare November 17, 2025 12:49
@mauri870
Copy link
Copy Markdown
Member Author

@mauri870 Your test is logically failing on Windows. Should we exclude the test from running there?

Thanks, I adjusted the test logic since we don't perform any truncation on Windows.

@mauri870
Copy link
Copy Markdown
Member Author

/test

@mauri870
Copy link
Copy Markdown
Member Author

Ugh, lots of flaky tests. I'll create issues for them.

@mauri870
Copy link
Copy Markdown
Member Author

@kruskall Added you as a reviewer because I think your input will be valuable. Thanks!

@mauri870
Copy link
Copy Markdown
Member Author

/test

Copy link
Copy Markdown
Member

@kruskall kruskall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mauri870 mauri870 enabled auto-merge (squash) November 18, 2025 18:41
@mauri870
Copy link
Copy Markdown
Member Author

/test

@cmacknz cmacknz disabled auto-merge November 18, 2025 19:34
@cmacknz cmacknz merged commit 8da89ce into elastic:main Nov 18, 2025
4 of 5 checks passed
@cmacknz
Copy link
Copy Markdown
Member

cmacknz commented Nov 18, 2025

Force merged past known flaky test #270

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants