in_forward: fix username parsing

[edsiper]

Fix parsing of username from the config value and ensure consistent error handling.

---

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

  • Corrected username buffer sizing to prevent potential overflows.
  • Ensured proper cleanup on allocation failures to avoid memory leaks.
  • Adjusted control flow so resources are released only after successful setup, improving stability.

• Refactor

  • Streamlined resource management during user setup for the forward input plugin, enhancing reliability under error conditions.

[coderabbitai]

Walkthrough

Adjusts setup_users in plugins/in_forward/fw.c: changes username length handling, adds proper cleanup of allocated username on password allocation failure, and moves split release to occur only after both allocations succeed, aligning error paths with resource management order.

Changes

Cohort / File(s)	Summary
Forward input user setup & error handling plugins/in_forward/fw.c	Updated username length to use sentry->len; added flb_sds_destroy(user->name) on password allocation failure; deferred split release until both username and password allocations succeed; clarified comments and aligned cleanup order.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller
  participant setup_users
  participant Alloc as Allocator
  participant Split as SplitCtx

  Caller->>setup_users: setup_users(split)
  setup_users->>Split: parse sentry
  setup_users->>Alloc: allocate username (len = sentry.len)
  alt username alloc fails
    setup_users-->>Caller: return error (no allocations to free)
  else username ok
    setup_users->>Alloc: allocate password
    alt password alloc fails
      setup_users->>Alloc: destroy username
      setup_users-->>Caller: return error (split not released here)
    else password ok
      setup_users->>Split: release split (after both allocations)
      setup_users-->>Caller: return success
    end
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

backport to v4.0.x

Suggested reviewers

• koleini
• fujimotos

Poem

A nibble of bytes, a hop through C,
I tidy the splits, so leaks can’t be.
Username snug, password tight,
Free on the left, then free on the right.
Thump-thump goes QA—approved with glee.
(_/)
( •_•)✨
/>🍃 Memory clean as can be!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title “in_forward: fix username parsing” concisely and accurately summarizes the primary change by indicating that the in_forward plugin’s username parsing logic was corrected, matching the adjustments to allocation size and error handling described in the diff.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch in_forward-username-parsing

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2af2017 and 21634a1.

📒 Files selected for processing (1)

• plugins/in_forward/fw.c (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

plugins/in_forward/fw.c (3)

src/flb_sds.c (2)

• flb_sds_create_len (58-76)
• flb_sds_destroy (389-399)

include/fluent-bit/flb_mem.h (1)

• flb_free (126-128)

src/flb_utils.c (1)

• flb_utils_split_free (477-489)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)

• GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
• GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
• GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
• GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
• GitHub Check: PR - fuzzing test
• GitHub Check: pr-compile-without-cxx (3.31.6)
• GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
• GitHub Check: pr-compile-centos-7
• GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
• GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
• GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)

🔇 Additional comments (3)

plugins/in_forward/fw.c (3)

219-219: LGTM! Correct fix for username length allocation.

The change from sentry->len + 1 to sentry->len is correct. The flb_sds_create_len function already null-terminates the string internally (as seen in the relevant code snippet: s[len] = '\0'), so the extra +1 was unnecessary and could cause issues. This also makes the username allocation consistent with the password allocation on line 230.

---

233-233: LGTM! Critical fix for memory leak in error path.

The addition of flb_sds_destroy(user->name) is necessary to prevent a memory leak when password allocation fails. At this point, user->name was successfully allocated (line 219) and assigned (line 226), so it must be destroyed before freeing the user struct. The delete_users(ctx) call only cleans up users already in the list, not the current user being processed.

---

240-241: LGTM! Helpful clarification of control flow.

The updated comment accurately reflects that the split is released only after both username and password allocations succeed. This clarifies the resource management order and aligns with the improved error handling where split is freed in all error paths.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[edsiper]

Windows CI issue not related.