You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
in_tail: avoid libc basename() in file name compare - #12782
flb_tail_target_file_name_cmp called libc basename() twice. On macOS (and some other libcs) basename() uses a process-wide static buffer, so a threaded in_tail input can overwrite that buffer between basename() and strdup(), or during strcmp(), and then treat an unchanged file as rotated. That produces duplicate records.
This replaces basename() with a thread-safe scan for the last path separator (/ and, on Windows, \) on the original strings, so no shared buffer or extra copies are involved. Windows still uses _stricmp.
Apple's basename() uses a process-wide static buffer, so a threaded
in_tail input can overwrite the pointer mid-compare and report a
false rotation. Compare the suffix after the last path separator on
the original strings instead.
Fixesfluent#12780
Signed-off-by: kevin9327 <5299031+kevin9327@users.noreply.github.com>
No actionable comments were generated in the recent review.
Walkthrough
The tail file name comparison now uses a helper that extracts path basenames without allocating memory. It recognizes platform-specific separators and retains case-insensitive comparison on Windows and case-sensitive comparison elsewhere.
Changes: Tail path name comparison
Layer / File(s)
Summary
Extract and compare path basenames plugins/in_tail/tail_file.h
flb_tail_path_basename returns the portion after the final recognized separator, or an empty string for a null path. flb_tail_target_file_name_cmp compares the extracted components directly and no longer allocates or frees strings, or returns -1 for allocation failures.
This change removes a thread-safety race in tail file name comparison that could cause unchanged files to be treated as rotated and have their records duplicated. No actionable merge-blocking risk was found.
Pre-merge checks: 4
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title "in_tail: avoid libc basename() in file name compare" matches the main change. The PR replaces libc basename() in flb_tail_target_file_name_cmp with a scan for the last path separator, which…
Linked Issues check
Issue #12780 requires that unchanged files not be reported as rotated when a threaded in_tail input runs the name compare concurrently with other tail inputs. The root cause is the shared static buffe…
Out of Scope Changes check
The change set, as shown by the whole-PR summary and the tail_file.h diff against the merge base, touches only plugins/in_tail/tail_file.h. All edits are in flb_tail_target_file_name_cmp and its new h…
Full details: Title check
Explanation
The title "in_tail: avoid libc basename() in file name compare" matches the main change. The PR replaces libc basename() in flb_tail_target_file_name_cmp with a scan for the last path separator, which the raw summary confirms via the new flb_tail_path_basename helper and the rewritten comparison. The title names the plugin, the libc call removed, and the affected function's purpose, so it is specific and concise.
Full details: Linked Issues check
Explanation
Issue #12780 requires that unchanged files not be reported as rotated when a threaded in_tail input runs the name compare concurrently with other tail inputs. The root cause is the shared static buffer returned by libc basename(). The diff to plugins/in_tail/tail_file.h removes all libc basename() calls from flb_tail_target_file_name_cmp(). It adds flb_tail_path_basename(), which returns a pointer into the original string after the last '/' (and '' on Windows). The comparison then uses strcmp() on non-Windows and _stricmp() on Windows. No shared state remains, so the race described in the issue no longer exists on any platform. The issue explicitly lists a portable helper that avoids libc basename() as an acceptable alternative to basename_r(). The PR does not add an automated regression test, and the PR description states no test run. The issue does not require one. Runtime behavior on macOS is not verified in the available evidence. Minor semantic difference: libc basename('dir/') returns 'dir', while the new helper returns ''. This does not affect regular file paths.
Full details: Out of Scope Changes check
Explanation
The change set, as shown by the whole-PR summary and the tail_file.h diff against the merge base, touches only plugins/in_tail/tail_file.h. All edits are in flb_tail_target_file_name_cmp and its new helper flb_tail_path_basename. This directly implements the fix for #12780. The removal of flb_strdup() and flb_free() calls and of the -1 allocation-failure returns follows from the removed allocations. This is a necessary part of the fix, not unrelated work. No unrelated changes were found.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: adff7107-9232-46b8-b504-162062576281
Commits
Reviewing files that changed from the base of the PR and between f5a3cd2 and 505de56.
Files selected for processing: 1
plugins/in_tail/tail_file.h
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
flb_tail_target_file_name_cmpcalled libcbasename()twice. On macOS (and some other libcs)basename()uses a process-wide static buffer, so a threadedin_tailinput can overwrite that buffer betweenbasename()andstrdup(), or duringstrcmp(), and then treat an unchanged file as rotated. That produces duplicate records.This replaces
basename()with a thread-safe scan for the last path separator (/and, on Windows,\) on the original strings, so no shared buffer or extra copies are involved. Windows still uses_stricmp.Fixes #12780
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:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
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