Skip to content

fix: validate dump messages start message id#50349

Open
proobker wants to merge 1 commit into
milvus-io:masterfrom
proobker:codex/fix-dump-messages-invalid-id
Open

fix: validate dump messages start message id#50349
proobker wants to merge 1 commit into
milvus-io:masterfrom
proobker:codex/fix-dump-messages-invalid-id

Conversation

@proobker

@proobker proobker commented Jun 6, 2026

Copy link
Copy Markdown

Fixes #50341

DumpMessages previously used MustUnmarshalMessageID on client-provided start_message_id bytes. A malformed message ID could panic and crash the process.

This change uses the non-panicking UnmarshalMessageID path and returns an invalid parameter error instead.

Added regression coverage for malformed Pulsar start_message_id bytes.

Signed-off-by: proobker <rabimenuka1@gmail.com>
@sre-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: proobker
To complete the pull request process, please assign tedxu after the PR has been reviewed.
You can assign the PR to them by writing /assign @tedxu in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot added the size/S Denotes a PR that changes 10-29 lines. label Jun 6, 2026
@mergify mergify Bot added dco-passed DCO check passed. kind/bug Issues or changes related a bug labels Jun 6, 2026
@sre-ci-robot

Copy link
Copy Markdown
Contributor

[ci-v2-notice]
Notice: New ci-v2 system is enabled for this PR.

To rerun ci-v2 checks, comment with:

  • /ci-rerun-code-check // for ci-v2/code-check
  • /ci-rerun-code-check-macos // for Code Checker MacOS (GitHub Actions)
  • /ci-rerun-build // for ci-v2/build
  • /ci-rerun-build-all // for ci-v2/build-all (multi-arch builds)
  • /ci-rerun-buildenv // for ci-v2/build-env (build milvus-env builder images)
  • /ci-rerun-ut-integration // for ci-v2/ut-integration, will rerun ci-v2/build
  • /ci-rerun-ut-go // for ci-v2/ut-go, will rerun ci-v2/build
  • /ci-rerun-ut-cpp // for ci-v2/ut-cpp
  • /ci-rerun-ut // for all ci-v2/ut-integration, ci-v2/ut-go, ci-v2/ut-cpp, will rerun ci-v2/build
  • /ci-rerun-e2e-default // for ci-v2/e2e-default
  • /ci-rerun-e2e-amd // for ci-v2/e2e-amd (e2e pool dispatcher)
  • /ci-rerun-build-ut-cov // for ci-v2/build-ut-cov (build + unit tests in one pipeline)
  • /ci-rerun-gosdk // for ci-v2/go-sdk (Go SDK E2E tests, ARM)

If you have any questions or requests, please contact @zhikunyao.

@sre-ci-robot

Copy link
Copy Markdown
Contributor

✅ CI Loop Results ddeb61b

Stage Result Duration Tests
✅ Build SUCCESS 14.5min -
✅ Code-Check SUCCESS 8.4min -
✅ UT-GO SUCCESS 23.5min 1012 passed
✅ UT-Integration SUCCESS 25.7min 46 passed
✅ UT-CPP-Cov SUCCESS 54.0min 7851 passed

Total: 82min | Pipeline | Artifacts

Overall Coverage: 71.2%
Diff Coverage: Go 100.0% (4 hit, 0 miss, 4 measurable lines, 0 unmeasured)
Diff Coverage HTML: view changed lines
Total Patch Coverage: 100.0% (4/4 measurable lines, 0 unmeasured)

@codecov

codecov Bot commented Jun 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.99%. Comparing base (93a76fe) to head (ddeb61b).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #50349      +/-   ##
==========================================
- Coverage   79.01%   78.99%   -0.03%     
==========================================
  Files        2236     2236              
  Lines      396046   396046              
==========================================
- Hits       312925   312841      -84     
- Misses      73607    73668      +61     
- Partials     9514     9537      +23     
Components Coverage Δ
Client 80.59% <ø> (ø)
Core 86.09% <ø> (ø)
Go 76.79% <100.00%> (-0.04%) ⬇️
Files with missing lines Coverage Δ
internal/proxy/impl.go 71.38% <100.00%> (ø)

... and 32 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sre-ci-robot sre-ci-robot added low-code-coverage add test-label from zhikun, diff coverage > 80% and removed low-code-coverage add test-label from zhikun, diff coverage > 80% labels Jun 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-passed DCO check passed. kind/bug Issues or changes related a bug size/S Denotes a PR that changes 10-29 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: DumpMessages panics and crashes the whole process on a malformed start_message_id

3 participants