http_server: accept empty HTTP header values

[kimonus]

Supersedes #12175, which GitHub auto-closed when the temporary base branch
for #12212 was removed after merge.

Fixes #12174.

Summary

The parser fix was merged upstream in monkey/monkey#444 and bundled in
Fluent Bit with Monkey 1.8.9 by #12212. This PR contains only the remaining
Fluent Bit-owned changes:

• propagate Monkey HTTP/1 parser errors so invalid requests are closed instead
  of being reset and left pending;
• keep protocol autodetection pending for an incomplete HTTP/2 preface and
  pass all accumulated preface bytes to nghttp2;
• add runtime coverage for accepted empty generic headers and rejected empty
  semantic headers;
• add real-server in_http coverage for the original POST/body report,
  bounded Content-Length parsing, and split HTTP/1 and HTTP/2 requests.

No files under lib/monkey are changed.

Behavior

Before this change, an invalid HTTP/1 request could leave a keep-alive client
without a response, and a split HTTP/2 preface could be routed to the HTTP/1
parser. After this change, invalid requests are rejected and incomplete HTTP/2
autodetection waits for the rest of the preface. Empty generic headers remain
accepted.

Testing

Ubuntu 24.04 Docker, GCC 13.3:

cmake -S . -B build-pr -DFLB_TESTS_RUNTIME=On -DFLB_TESTS_INTERNAL=On
cmake --build build-pr -j8

Result: full configure and build passed.

ctest --test-dir build-pr \
  -R '^(flb-rt-in_http|in_http_tls_expect.sh|flb-it-http_server)$' \
  --output-on-failure

Result: focused CTest passed 3/3.

ctest --test-dir build-pr --output-on-failure

Result: 204/205 tests passed. The sole failure was the unrelated legacy
flb-rt-out_td, which depends on a hard-coded /tmp/td.conf and leaves its
pre-created td.0 output unconfigured when the repository sample config is
mounted.

Targeted split-boundary regressions:

tests/integration/.venv/bin/python -m pytest -q \
  tests/integration/scenarios/in_http/tests/test_in_http_001.py::test_in_http_accepts_split_http2_preface \
  tests/integration/scenarios/in_http/tests/test_in_http_001.py::test_in_http_accepts_http1_request_split_before_autodetect_boundary

Result: 2/2 passed normally and 2/2 passed with
VALGRIND=1 VALGRIND_STRICT=1.

Complete affected integration scope:

tests/integration/.venv/bin/python -m pytest -q \
  tests/integration/scenarios/in_http/tests/test_in_http_001.py \
  tests/integration/scenarios/in_http_max_connections/tests/test_in_http_max_connections_001.py

Result: 37/37 passed normally and 37/37 passed with
VALGRIND=1 VALGRIND_STRICT=1.

The exact six-scenario Input HTTP pause and lifecycle tests selection also
passed in both modes: 27 passed and 167 deselected normally, then 27 passed and
167 deselected with strict Valgrind.

The CI-style full-PR commit-prefix checker and git diff --check pass against
the current master branch.

PR template evidence

• Example configuration: N/A; the regression uses existing in_http
  integration configurations without changing user configuration.
• Debug output: the focused CTest and real-server integration runs start the
  newly built Fluent Bit v5.1.0 binary and exercise actual HTTP listeners.
• Valgrind: the complete affected integration scope and the exact CI selection
  both passed with strict Valgrind handling.

Representative clean Valgrind output:

HEAP SUMMARY:
    in use at exit: 0 bytes in 0 blocks
  total heap usage: 14,898 allocs, 14,898 frees, 3,052,433 bytes allocated
All heap blocks were freed -- no leaks are possible
ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Compatibility

No configuration, packaging, documentation, or bundled-library changes are
included. The runtime socket regression uses Fluent Bit's portable socket
types. Upstream Windows and macOS workflows are pending maintainer approval
for this fork PR.

Documentation

N/A. This fixes protocol parsing and error handling without changing public
configuration or documented user-facing interfaces. The default
docs-required label can be removed if the maintainers agree.

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of malformed HTTP/1 requests by returning an appropriate server error.
  • Correctly detects HTTP/2 prefaces split across multiple network reads.
  • Rejects empty or whitespace-only Content-Length headers without forwarding payloads.
  • Rejects empty Upgrade headers without producing unintended output.
  • Accepts valid requests with empty generic, Connection, and Transfer-Encoding headers.

• Tests

  • Added regression coverage for split protocol requests, header validation, payload forwarding, and response handling.

[coderabbitai]

📝 Walkthrough

Walkthrough

The HTTP server now preserves split HTTP/2 prefaces and reports HTTP/1 parser errors. Runtime and integration tests cover empty headers, invalid Content-Length values, response statuses, payload forwarding, and rejection behavior.

Changes

HTTP protocol regressions

Layer / File(s)	Summary
Split HTTP/2 preface detection src/http_server/flb_http_server.c, tests/integration/scenarios/in_http/tests/test_in_http_001.py	Protocol detection waits for incomplete HTTP/2 prefaces and passes accumulated bytes to HTTP/2 ingestion. Tests cover split HTTP/2 prefaces and HTTP/1 requests split before autodetection.
HTTP/1 parser error handling src/http_server/flb_http_server_http1.c, tests/runtime/in_http.c	flb_http1_server_session_ingest marks parser errors on the stream and returns HTTP_SERVER_PROVIDER_ERROR. Runtime coverage validates response handling for malformed requests.
Empty-header regression coverage tests/runtime/in_http.c, tests/integration/scenarios/in_http/tests/test_in_http_001.py	Raw-socket tests verify accepted empty headers, rejected empty or whitespace-only values, response statuses, payload forwarding, and callback behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related issues

• fluent/fluent-bit#12174: Covers the same HTTP/1 parser-error handling and empty-header regression behavior.

Possibly related PRs

• fluent/fluent-bit#12175: Directly relates to HTTP/1 parser-error handling and empty-header regression tests.

Suggested reviewers: cosmo0920, edsiper

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant HTTPServer
  participant HTTP2Session
  Client->>HTTPServer: Send split HTTP/2 preface
  HTTPServer->>HTTPServer: Wait for remaining preface bytes
  HTTPServer->>HTTP2Session: Ingest accumulated preface
  HTTP2Session-->>Client: Return settings response

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: accepting empty HTTP header values in the HTTP server.

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/runtime/in_http.c`:
- Around line 638-641: Extend the rejection assertion in tests/runtime/in_http.c
around get_output_num() to wait for the same full dispatch and flush observation
interval used by the accepted raw-request path before comparing output counts.
In tests/integration/scenarios/in_http/tests/test_in_http_001.py lines 246-255,
add a bounded no-output observation covering at least one configured output
flush interval before asserting data_storage["payloads"] is empty.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 045d6dc3-3460-4597-b750-46f65f7466f6

📥 Commits

Reviewing files that changed from the base of the PR and between 9fab354 and beab797.

📒 Files selected for processing (3)

• src/http_server/flb_http_server_http1.c
• tests/integration/scenarios/in_http/tests/test_in_http_001.py
• tests/runtime/in_http.c

[kimonus]

Thanks for the CI signal. I reproduced the failing max-connections case and found an interaction in protocol autodetection: the undecided version is -1, so the old <= HTTP/1.1 check sent an incomplete HTTP/2 preface to the HTTP/1 parser. Now that parser errors are propagated correctly, that closed the connection before the idle timeout could evict it.

I updated the branch so that:

• protocol autodetection waits while a split HTTP/2 preface is still incomplete;
• after detecting HTTP/2, all bytes accumulated during detection are passed to nghttp2;
• a regression test sends a valid HTTP/2 preface across separate reads and verifies the server SETTINGS response.

New commits:

• 608dd5b1d http_server: wait for complete HTTP/2 preface
• 5eeaf9dd6 tests: integration: cover split HTTP/2 preface

Verification against the rebuilt binary:

• complete in_http + in_http_max_connections: 36 passed normally;
• the same complete set with VALGRIND=1 VALGRIND_STRICT=1: 36 passed;
• exact six-scenario workflow selection: 27 passed, 166 deselected;
• the same workflow selection under strict Valgrind: 27 passed, 166 deselected;
• full configured CTest: 204/205 passed, including flb-rt-in_http and flb-it-http_server. The sole failure was the unrelated legacy flb-rt-out_td, which depends on a hard-coded /tmp/td.conf and still leaves its pre-created td.0 output unconfigured when the repository sample config is mounted;
• full PR-range commit-prefix lint and git diff --check: passed.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/http_server/flb_http_server.c`:
- Around line 1402-1406: In the HTTP protocol autodetection flow, update the
HTTP/1 path after parser initialization to restore buffer and length from
session->incoming_data, matching the existing HTTP/2 handling, so all
accumulated bytes are replayed to the parser. Add a regression test that splits
an HTTP/1 request before the fourth byte and verify the request is parsed
successfully, including the relevant boundary size.

In `@tests/integration/scenarios/in_http/tests/test_in_http_001.py`:
- Around line 97-101: The test around the split preface writes must verify that
the server has observed and processed preface[:-2] before sending the remainder.
Replace the timing-only sleep in the test_in_http_001 scenario with a
server-visible synchronization mechanism or an explicit check that the
connection remains open after the first sendall, then send preface[-2:] plus
settings_frame.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f2701275-8c46-4711-ad8c-81cd4d726f88

📥 Commits

Reviewing files that changed from the base of the PR and between 4b801a0 and 5eeaf9d.

📒 Files selected for processing (2)

• src/http_server/flb_http_server.c
• tests/integration/scenarios/in_http/tests/test_in_http_001.py

[kimonus]

@edsiper @cosmo0920 CodeRabbit has completed its latest review with no actionable comments, and all review threads are resolved.

I updated the PR description with the final scope and verification evidence. The rebuilt binary passed the focused CTest targets 3/3; the complete affected integration scope passed 37/37 normally and 37/37 under strict Valgrind; and the exact six-scenario CI selection passed 27/27 in both modes. The full configured CTest run passed 204/205, with only the unrelated legacy flb-rt-out_td hard-coded /tmp/td.conf issue noted in the description.

The fork workflows currently show action_required pending maintainer approval. Documentation is N/A because this changes no public configuration or documented interface, so the default docs-required label can be removed if you agree.

When convenient, could you please approve the workflows and review this PR for the v5.1 milestone? Thank you.

[kimonus]

Thanks for merging this, Eduardo. I noticed that the post-merge HTTP lifecycle workflow passed 26/27 tests but failed the partial HTTP/2 preface max-connections case under strict Valgrind. The same selection passed on the PR and in our repeated local strict-Valgrind runs, so it appears timing-sensitive. I’m happy to investigate and prepare a small follow-up if you think it would be useful.