Skip to content

in_forward: fix stale byte accounting when multiple frames share a buffer#12049

Open
zshuang0316 wants to merge 1 commit into
fluent:masterfrom
zshuang0316:in-forward-frame-desync
Open

in_forward: fix stale byte accounting when multiple frames share a buffer#12049
zshuang0316 wants to merge 1 commit into
fluent:masterfrom
zshuang0316:in-forward-frame-desync

Conversation

@zshuang0316

@zshuang0316 zshuang0316 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

In fw_prot_process, the inner frame loop tracks how many bytes each decoded object consumed in all_used, then uses it to trim conn->buf once the buffer is drained. The first object is parsed with msgpack_unpacker_next_with_size(unp, &result, &bytes), but the loop tail advanced to the next object with the plain msgpack_unpacker_next(unp, &result), which does not refresh bytes.

When several frames arrive in a single buffer — common with compressed payloads, which are small — every iteration after the first adds the previous object's stale bytes to all_used. The resulting total is wrong, so conn->buf is trimmed by the wrong offset and the remaining frames in the buffer desync.

The fix uses msgpack_unpacker_next_with_size() at the loop tail as well, so bytes is refreshed for every object, matching the initial parse.


Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • Example configuration file for the change
  • Debug log output from testing the change
  • Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • [N/A] Run local packaging test showing all targets (including any new ones) build.
  • [N/A] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • [N/A] Documentation required for this feature

Backporting

  • Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Summary by CodeRabbit

  • Bug Fixes
    • Improved message parsing so incoming data is counted more accurately during processing.
    • Helped ensure multi-packet or multi-object streams are handled reliably without affecting byte tracking.

…ffer

The frame loop advances to the next object with the plain
msgpack_unpacker_next(), which does not refresh 'bytes'. When several
frames arrive in a single buffer (common with compressed payloads, which
are small), iterations after the first add the previous object's stale
size to 'all_used'. That wrong total trims conn->buf by the wrong offset
and desyncs the remaining frames. Use msgpack_unpacker_next_with_size()
so 'bytes' is refreshed for every object, matching the initial parse.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: zshuang0316 <zshuang0316@163.com>
@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b3577383-8ef4-4aa1-85ab-54aeaf0c8171

📥 Commits

Reviewing files that changed from the base of the PR and between a1e05fc and caa1918.

📒 Files selected for processing (1)
  • plugins/in_forward/fw_prot.c

📝 Walkthrough

Walkthrough

A single-line fix in the forward input plugin's msgpack parsing loop switches from msgpack_unpacker_next to msgpack_unpacker_next_with_size, ensuring the decoded object size populates the bytes variable used for byte accounting.

Changes

Msgpack Unpacking Fix

Layer / File(s) Summary
Switch to size-returning unpacker call
plugins/in_forward/fw_prot.c
Replaces the msgpack unpacking call with a variant that returns the object size, fixing the bytes/all_used accounting in the parsing loop.

Estimated code review effort: 1 (Trivial) | ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main fix in in_forward: stale byte accounting for shared-buffer frames.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zshuang0316

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: caa1918b6c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant