Skip to content

Conversation

edsiper
Copy link
Member

@edsiper edsiper commented Sep 22, 2025

Fixes #10905

The config format read_blob() implementation in Windows, was incorrectly receiving a struct parser_state * instead of a struct file_state * to the read_config function. This PR fix the wrong parameter.


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 handling of files matched by glob patterns in YAML configuration, ensuring they inherit the correct parent context for path resolution.
    • Improves reliability when loading multiple config fragments via globs, reducing unexpected include/path errors and making behavior more predictable on non-Windows systems.

Copy link

coderabbitai bot commented Sep 22, 2025

Walkthrough

Updates read_glob’s non-Windows path handling to call read_config with state->file as the parent file context instead of the state object. No public APIs changed.

Changes

Cohort / File(s) Summary
YAML include/glob handling
src/config_format/flb_cf_yaml.c
In read_glob (non-Windows), changed read_config invocation from read_config(conf, ctx, state, buf) to read_config(conf, ctx, state->file, buf), altering the parent file context passed during include processing.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User as User
  participant Parser as YAML Parser
  participant Glob as Glob Resolver
  participant Loader as read_config

  User->>Parser: Load YAML with includes
  Parser->>Glob: Resolve patterns (non-Windows)
  loop For each matched path
    Glob->>Loader: read_config(conf, ctx, state->file, matched_path)
    note right of Loader: Parent context set to state->file<br/>(previously was state)
  end
  Loader-->>Parser: Include content merged
  Parser-->>User: Configuration loaded
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop through files with whiskers bright,
Swap a parent in the moonlit night.
Paths now point to where they should,
Globs aligned, the forest’s good.
Thump-thump—includes all neatly filed,
A tidy burrow, rabbit-smiled. 🐇✨

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 (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "config_format: yaml: fix state parameter for read_blob in Windows" succinctly states the primary intent—correcting the state pointer passed to the YAML reader on Windows—and aligns with the PR objectives of passing a struct file_state* to read_config. It is concise, specific to the subsystem and function-level fix, and suitable for history scanning. If the actual change targets read_glob rather than read_blob, consider renaming the title to match the precise function name for added accuracy.
Linked Issues Check ✅ Passed The change corrects the argument passed to read_config by using state->file (a file_state pointer) instead of the parser state pointer, which directly addresses the Windows wildcard include crash described in issue [#10905] by ensuring the correct parent file context during glob/include processing. This maps to the linked issue's objective of making globbing behavior on Windows match Linux and preventing the memory access violation. The provided summary shows the fix is localized to the relevant call site in flb_cf_yaml.c, though final verification requires running the Windows reproduction or CI tests.
Out of Scope Changes Check ✅ Passed Based on the provided summary, the change is limited to src/config_format/flb_cf_yaml.c and only adjusts the parameter passed to read_config; no exported/public declarations or unrelated files were modified, so I detect no out-of-scope changes. This assessment is constrained to the supplied summary and should be rechecked against the full diff to confirm no other edits were introduced.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch config_format-yaml-windows-glob

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 and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/config_format/flb_cf_yaml.c (1)

617-620: Propagate recursive errors in Windows read_glob.

If a nested glob expansion fails, the current code ignores the return value. Bubble up errors to avoid partial includes and harder-to-debug states.

Apply:

-        if (strchr(p1, '*')) {
-            read_glob(conf, ctx, state, buf); /* recursive */
-            continue;
-        }
+        if (strchr(p1, '*')) {
+            int rc = read_glob(conf, ctx, state, buf); /* recursive */
+            if (rc < 0) {
+                FindClose(hnd);
+                return -1;
+            }
+            continue;
+        }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 386e373 and 0804b66.

📒 Files selected for processing (1)
  • src/config_format/flb_cf_yaml.c (1 hunks)
⏰ 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). (29)
  • 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: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
  • 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_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=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=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_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 (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=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 (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=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=On, 3.31.6, gcc, g++)
  • 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-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, gcc, g++, ubuntu-24.04, clang-14)
  • GitHub Check: pr-compile-centos-7
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
  • GitHub Check: PR - fuzzing test
🔇 Additional comments (1)
src/config_format/flb_cf_yaml.c (1)

626-629: Fix is correct: pass file_state to read_config on Windows glob.*

Matches the non-Windows path and prevents the access violation by avoiding passing a parser_state* where a file_state* is required; this should address #10905's crash on Windows when using "" in includes. Automated scans errored (rg PCRE2 compile error; python script syntax error). Re-run a robust search for read_config(...) call sites that pass 'state' (not 'state->file') or manually verify; validate on Windows with includes ['..\conf_inputs_.yaml'] containing 2+ matching files.

@edsiper edsiper merged commit 720c52b into master Sep 22, 2025
65 checks passed
@edsiper edsiper deleted the config_format-yaml-windows-glob branch September 22, 2025 15:17
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.

"*" wildcard in "include" section filenames triggers memory access violation on Windows machines.
1 participant