fix: properly handle IPv6 addresses in HTTP Host headers #11181
Conversation
WalkthroughAdds bracket-aware IPv6 validation and parsing to URL split routines, updates Host header composition to use runtime TLS status and bracket unbracketed IPv6 addresses, adds Windows IPv6 includes, and expands tests for IPv6/IPv4 Host header and URL parsing variations. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Test/Caller
participant URL as flb_utils_url_split*
participant Val as validate_ipv6_brackets()
participant Conn as flb_http_client / Connection
participant Host as Host Header Composer
T->>URL: parse URL (may contain IPv6)
URL->>Val: validate bracket usage if IPv6-like
alt invalid brackets
Val-->>URL: error
URL-->>T: return error (free resources)
else valid or not IPv6
Val-->>URL: bracket info
URL-->>T: proto, host, port, uri (bracket-aware)
T->>Conn: open connection (check runtime TLS flag)
Conn->>Host: compose Host header (uses flb_stream_get_flag_status)
alt unbracketed IPv6
Host-->>Conn: format as [address] or [address]:port per TLS rules
else IPv4/domain/already-bracketed
Host-->>Conn: format host[:port] (omit default TLS port when applicable)
end
Conn-->>T: connection + Host header ready
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/internal/http_client.c (1)
469-493: Good: Centralized Host header validation with cleanup.The
check_host_headerhelper centralizes Host header verification logic and handles cleanup properly when checks fail. Theextra_cleanupparameter allows proper resource deallocation for TLS test scenarios.Minor suggestion: Consider using a more specific type for
extra_cleanupinstead ofvoid *to improve type safety.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/flb_http_client.c(2 hunks)src/flb_utils.c(8 hunks)tests/internal/http_client.c(2 hunks)tests/internal/utils.c(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:24:55.855Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:52-56
Timestamp: 2025-08-29T06:24:55.855Z
Learning: ZSTD compression is always available in Fluent Bit and does not require conditional compilation guards. Unlike Arrow/Parquet which use #ifdef FLB_HAVE_ARROW guards, ZSTD is built unconditionally with flb_zstd.c included directly in src/CMakeLists.txt and a bundled ZSTD library at lib/zstd-1.5.7/.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:24:26.170Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:39-42
Timestamp: 2025-08-29T06:24:26.170Z
Learning: In Fluent Bit, ZSTD compression support is enabled by default and does not require conditional compilation guards (like #ifdef FLB_HAVE_ZSTD) around ZSTD-related code declarations and implementations.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components, ZSTD support is always available and doesn't need build-time conditionals.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:24:44.797Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:26-26
Timestamp: 2025-08-29T06:24:44.797Z
Learning: In Fluent Bit, ZSTD support is always available and enabled by default. The build system automatically detects and uses either the system libzstd library or builds the bundled ZSTD version. Unlike other optional dependencies like Arrow which use conditional compilation guards (e.g., FLB_HAVE_ARROW), ZSTD does not require conditional includes or build flags.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-09-08T11:21:33.975Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 10851
File: include/fluent-bit/flb_simd.h:60-66
Timestamp: 2025-09-08T11:21:33.975Z
Learning: Fluent Bit currently only supports MSVC compiler on Windows, so additional compiler compatibility guards may be unnecessary for Windows-specific code paths.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit, the correct CMake flag for using system librdkafka is `FLB_PREFER_SYSTEM_LIB_KAFKA=ON`.
Applied to files:
src/flb_http_client.c
🧬 Code graph analysis (3)
src/flb_http_client.c (1)
src/flb_sds.c (2)
flb_sds_copy(260-281)flb_sds_printf(336-387)
src/flb_utils.c (2)
src/flb_sds.c (2)
flb_sds_create_len(58-76)flb_sds_create(78-90)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
tests/internal/http_client.c (4)
src/flb_http_client.c (3)
flb_http_get_header(1015-1032)flb_http_client_destroy(1701-1708)flb_http_client(827-872)include/fluent-bit/flb_mem.h (2)
flb_free(126-128)flb_calloc(84-96)src/flb_upstream.c (2)
flb_upstream_destroy(656-698)flb_upstream_create(290-363)src/flb_sds.c (1)
flb_sds_destroy(389-399)
🔇 Additional comments (7)
src/flb_utils.c (4)
1441-1462: LGTM!The IPv6 bracket validation function correctly handles the primary error cases: missing closing bracket, empty brackets, and closing bracket without opening bracket. The logic is straightforward and focused.
1494-1504: Good: Bracket-aware port parsing prevents false positives.The logic correctly uses the bracket position to determine where to search for the port separator. This prevents incorrectly treating colons within an IPv6 address as port separators.
1536-1547: Essential: NULL checks prevent use-after-free.The added NULL checks after
flb_copy_hostcalls are critical for handling malformed input gracefully. Without these checks, a NULL host pointer would propagate to output parameters, potentially causing crashes.
1568-1580: Improved: Comprehensive error cleanup prevents memory leaks.The enhanced error handling properly frees all allocated resources (host, port, uri) when URL parsing fails. This ensures no memory leaks occur on error paths.
tests/internal/utils.c (1)
39-56: Excellent: Comprehensive IPv6 test coverage.The added test cases thoroughly cover IPv6 URL parsing scenarios including:
- Basic bracketed IPv6 addresses with and without ports
- Full and compressed IPv6 formats
- IPv4-mapped IPv6 addresses
- Edge cases with malformed brackets (missing opening, missing closing, empty brackets)
- Query strings containing brackets
This test suite will help catch regressions in IPv6 URL parsing logic.
src/flb_http_client.c (1)
621-638: Good: Improved Host header construction with IPv6 support.The changes correctly handle IPv6 addresses in Host headers:
- Uses runtime TLS status check (
flb_stream_get_flag_status) for more reliable detection- Properly detects unbracketed IPv6 addresses using
inet_pton- Wraps IPv6 addresses with brackets when combined with port numbers
- Preserves existing behavior for IPv4, domain names, and already-bracketed addresses
Note:
inet_ptonbehavior with zone IDs (e.g., "fe80::1%eth0") varies by platform. The test suite should verify this edge case works correctly.tests/internal/http_client.c (1)
557-663: Excellent: Comprehensive Host header test coverage.The test suite thoroughly covers Host header formatting across multiple scenarios:
- IPv6 addresses (unbracketed, bracketed, various formats, zone IDs)
- IPv4 addresses and domain names
- TLS vs non-TLS with default and non-standard ports
- Edge cases like port 443 without TLS
This extensive coverage ensures the Host header logic works correctly across different configurations and will catch regressions.
ShelbyZ
force-pushed
the
ipv6-changes
branch
from
a979de3 to
bcdd0dd
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/flb_http_client.c (1)
585-645: IPv6-aware Host header construction and TLS check look correct; consider bracketing IPv6 also for TLS+443The updated
add_host_and_content_length()logic looks solid overall:
- Uses
flb_stream_get_flag_status(&u->base, FLB_IO_TLS)to decide TLS, which is more accurate than relying onc->flags.- For non‑TLS or non‑443, it correctly detects unbracketed IPv6 via
inet_pton(AF_INET6, ...)and emitsHost: [addr]:port, while keeping existing%s:%ibehavior for IPv4, domain names, or already-bracketed IPv6.- Error handling and SDS usage (
flb_sds_create_size,flb_sds_copy,flb_sds_printf) are safe, with proper cleanup on failure.One refinement you might consider: in the TLS+443 branch you currently copy
out_hostas‑is, so an unbracketed IPv6 literal would still produceHost: ::1(no brackets). If you want Host to be fully RFC‑compliant for IPv6 even when the port is omitted, you could reuse the sameinet_pton(AF_INET6, ...)check there and emit[addr]instead ofaddr. For example:- /* Check if connection uses TLS and port is 443 (HTTPS default) */ - if (flb_stream_get_flag_status(&u->base, FLB_IO_TLS) && out_port == 443) { - /* HTTPS on default port 443 - omit port from Host header */ - tmp = flb_sds_copy(host, out_host, strlen(out_host)); - } - else { + /* Check if connection uses TLS and port is 443 (HTTPS default) */ + if (flb_stream_get_flag_status(&u->base, FLB_IO_TLS) && out_port == 443) { + struct in6_addr addr; + + /* For IPv6, still emit brackets even when omitting the port */ + if (out_host && out_host[0] != '[' && + inet_pton(AF_INET6, out_host, &addr) == 1) { + tmp = flb_sds_printf(&host, "[%s]", out_host); + } + else { + tmp = flb_sds_copy(host, out_host, strlen(out_host)); + } + } + else { struct in6_addr addr; /* Check if out_host is an unbracketed IPv6 address */ if (out_host && out_host[0] != '[' && inet_pton(AF_INET6, out_host, &addr) == 1) { /* IPv6 address needs brackets when combined with port */ tmp = flb_sds_printf(&host, "[%s]:%i", out_host, out_port); } else { /* IPv4 address, domain name, or already bracketed IPv6 */ tmp = flb_sds_printf(&host, "%s:%i", out_host, out_port); } }This keeps your new behavior but tightens IPv6 Host header formatting in the HTTPS default‑port case as well.
src/flb_utils.c (1)
1464-1583: Bracket-aware URL splitting and cleanup improvements are solid; minor behavior note on ‘]’ in pathsThe updates to
flb_utils_url_split()andflb_utils_url_split_sds()look good:
- They now:
- Validate IPv6 brackets up front via
validate_ipv6_brackets().- Use the returned
bracketposition to search for the port:only after the closing], otherwise from the start of the host, and ensure any candidate:comes before the first/.- Delegate host extraction to
flb_copy_host()/flb_utils_copy_host_sds(), so IPv6 literals are stored without brackets while other hosts are unchanged.- Allocate
protocol,host,port, anduricarefully and free all of them on every error path (both string and SDS variants), eliminating previous leak risks.The only behavioral nuance to be aware of is that
validate_ipv6_brackets()operates on the entire remainder of the URL starting atp, so a URL that contains a]character in the path or query (with no opening[) will now be rejected, whereas it may have been accepted before. If you later decide URLs likehttp://example.com/a]bshould be allowed, you could tighten the validator to only search for brackets up to the first/(or passsepinto it); otherwise this stricter validation is reasonable for most configurations.Also applies to: 1585-1729
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/flb_http_client.c(2 hunks)src/flb_network.c(1 hunks)src/flb_utils.c(8 hunks)tests/internal/http_client.c(2 hunks)tests/internal/utils.c(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/internal/utils.c
- tests/internal/http_client.c
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-09-08T11:21:33.975Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 10851
File: include/fluent-bit/flb_simd.h:60-66
Timestamp: 2025-09-08T11:21:33.975Z
Learning: Fluent Bit currently only supports MSVC compiler on Windows, so additional compiler compatibility guards may be unnecessary for Windows-specific code paths.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:24:26.170Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:39-42
Timestamp: 2025-08-29T06:24:26.170Z
Learning: In Fluent Bit, ZSTD compression support is enabled by default and does not require conditional compilation guards (like #ifdef FLB_HAVE_ZSTD) around ZSTD-related code declarations and implementations.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:24:55.855Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:52-56
Timestamp: 2025-08-29T06:24:55.855Z
Learning: ZSTD compression is always available in Fluent Bit and does not require conditional compilation guards. Unlike Arrow/Parquet which use #ifdef FLB_HAVE_ARROW guards, ZSTD is built unconditionally with flb_zstd.c included directly in src/CMakeLists.txt and a bundled ZSTD library at lib/zstd-1.5.7/.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:24:44.797Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:26-26
Timestamp: 2025-08-29T06:24:44.797Z
Learning: In Fluent Bit, ZSTD support is always available and enabled by default. The build system automatically detects and uses either the system libzstd library or builds the bundled ZSTD version. Unlike other optional dependencies like Arrow which use conditional compilation guards (e.g., FLB_HAVE_ARROW), ZSTD does not require conditional includes or build flags.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components, ZSTD support is always available and doesn't need build-time conditionals.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit, the correct CMake flag for using system librdkafka is `FLB_PREFER_SYSTEM_LIB_KAFKA=ON`.
Applied to files:
src/flb_http_client.c
🧬 Code graph analysis (2)
src/flb_utils.c (2)
src/flb_sds.c (2)
flb_sds_create_len(58-76)flb_sds_create(78-90)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
src/flb_http_client.c (1)
src/flb_sds.c (2)
flb_sds_copy(260-281)flb_sds_printf(336-387)
🔇 Additional comments (3)
src/flb_network.c (1)
30-36: Windows IPv6 header include looks correctAdding
<ws2tcpip.h>alongside<winsock2.h>underFLB_SYSTEM_WINDOWSmatches the existing use ofinet_pton/struct in6_addrin this file and aligns with the HTTP client changes; no further changes needed.src/flb_http_client.c (1)
36-43: Windows networking + compatibility includes are appropriateGuarding
<winsock2.h>and<ws2tcpip.h>withFLB_SYSTEM_WINDOWShere, and addingflb_compat.h, is consistent withflb_network.cand ensuresinet_pton/struct in6_addrare available on Windows without extra compiler-condition hacks (which matches the MSVC-only support model). Based on learnings.src/flb_utils.c (1)
1428-1462: IPv6 SDS host copy fix and bracket validator look correctThe
flb_utils_copy_host_sds()length change topos_end - pos_init - 2(IPv6) andpos_end - pos_init(non-IPv6) fixes the earlier off‑by‑one for non‑zeropos_initwhile preserving the existing behavior for the current callers (pos_init == 0). The newvalidate_ipv6_brackets()helper cleanly rejects malformed IPv6 host syntax (missing/empty brackets or stray]without[) and exposes the closing bracket position for later parsing, without introducing leaks or side effects.
ShelbyZ
force-pushed
the
ipv6-changes
branch
from
bcdd0dd to
347df83
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/flb_http_client.c (1)
626-643: Host header composition correctly handles TLS defaults and IPv6 bracketingThe updated
add_host_and_content_length():
- Uses
flb_stream_get_flag_status(&u->base, FLB_IO_TLS)plusout_port == 443to decide when to omit the port (HTTPS default), which is more robust than relying onc->flags.- For non‑default ports, detects unbracketed IPv6 literals via
AF_INET6and formatsHostas[%s]:%i, while leaving IPv4, domain names, and already‑bracketed IPv6 as%s:%i.This preserves existing behavior for IPv4 and domains, fixes the broken IPv6+port Host header, and keeps memory handling around the temporary SDS buffer correct. As a possible future enhancement (not required for this fix), you could consider also bracketing IPv6 literals when the port is omitted (e.g.,
Host: [::1]) for stricter RFC alignment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/flb_http_client.c(2 hunks)src/flb_network.c(1 hunks)src/flb_utils.c(8 hunks)tests/internal/http_client.c(2 hunks)tests/internal/utils.c(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/flb_network.c
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-09-08T11:21:33.975Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 10851
File: include/fluent-bit/flb_simd.h:60-66
Timestamp: 2025-09-08T11:21:33.975Z
Learning: Fluent Bit currently only supports MSVC compiler on Windows, so additional compiler compatibility guards may be unnecessary for Windows-specific code paths.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:24:26.170Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:39-42
Timestamp: 2025-08-29T06:24:26.170Z
Learning: In Fluent Bit, ZSTD compression support is enabled by default and does not require conditional compilation guards (like #ifdef FLB_HAVE_ZSTD) around ZSTD-related code declarations and implementations.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:24:55.855Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:52-56
Timestamp: 2025-08-29T06:24:55.855Z
Learning: ZSTD compression is always available in Fluent Bit and does not require conditional compilation guards. Unlike Arrow/Parquet which use #ifdef FLB_HAVE_ARROW guards, ZSTD is built unconditionally with flb_zstd.c included directly in src/CMakeLists.txt and a bundled ZSTD library at lib/zstd-1.5.7/.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:24:44.797Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:26-26
Timestamp: 2025-08-29T06:24:44.797Z
Learning: In Fluent Bit, ZSTD support is always available and enabled by default. The build system automatically detects and uses either the system libzstd library or builds the bundled ZSTD version. Unlike other optional dependencies like Arrow which use conditional compilation guards (e.g., FLB_HAVE_ARROW), ZSTD does not require conditional includes or build flags.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components, ZSTD support is always available and doesn't need build-time conditionals.
Applied to files:
src/flb_http_client.c
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit, the correct CMake flag for using system librdkafka is `FLB_PREFER_SYSTEM_LIB_KAFKA=ON`.
Applied to files:
src/flb_http_client.c
🧬 Code graph analysis (3)
tests/internal/http_client.c (4)
src/flb_http_client.c (3)
flb_http_get_header(1020-1037)flb_http_client_destroy(1706-1713)flb_http_client(832-877)include/fluent-bit/flb_mem.h (2)
flb_free(126-128)flb_calloc(84-96)src/flb_upstream.c (2)
flb_upstream_destroy(656-698)flb_upstream_create(290-363)src/flb_sds.c (1)
flb_sds_destroy(389-399)
src/flb_utils.c (2)
src/flb_sds.c (2)
flb_sds_create_len(58-76)flb_sds_create(78-90)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
src/flb_http_client.c (1)
src/flb_sds.c (2)
flb_sds_copy(260-281)flb_sds_printf(336-387)
🔇 Additional comments (5)
tests/internal/utils.c (1)
39-55: IPv6 URL split test coverage looks correct and thoroughThe new
url_checksentries nicely cover bracketed IPv6 (with/without ports, paths, IPv4‑embedded, and query strings) plus malformed bracket edge cases, and the expected host/port/uri values match the newflb_utils_url_split[_sds]semantics (host without brackets, defaults for 80/443, and-1on bad bracket syntax). No issues from a parsing or expectations standpoint.src/flb_http_client.c (1)
36-43: Windows networking includes and flb_compat usage are appropriateThe new
FLB_SYSTEM_WINDOWSblock for<winsock2.h>/<ws2tcpip.h>together with including<fluent-bit/flb_compat.h>at the top ensures the IPv6‑related types/functions used later in this file are available on Windows without affecting non‑Windows builds. This matches the project’s existing pattern for platform‑specific networking code.tests/internal/http_client.c (1)
495-555: Host header tests comprehensively validate IPv4/IPv6 and TLS behaviorThe new helpers (
test_host_header_format,test_tls_host_header_format) and associated tests exercise:
- IPv6 literals in both bracketed and unbracketed form (including compressed, full, IPv4‑mapped, all‑zeros, loopback).
- IPv4 addresses and domain names.
- TLS vs non‑TLS behavior for port 443 and non‑standard TLS ports.
- Zone‑ID variants (
fe80::1%eth0,[fe80::1%eth0]).- Explicit ports 80 and 443 for non‑TLS.
These cases mirror the logic in
add_host_and_content_length()and should prevent regressions in Host header formatting across the main combinations.Also applies to: 557-663, 673-683
src/flb_utils.c (2)
1428-1438: Corrected substring lengths inflb_utils_copy_host_sdsThe updated length calculations:
pos_end - pos_init - 2for bracketed IPv6 ([... ]) andpos_end - pos_initfor non‑bracketed hostsare now consistent with the starting pointer offset and correctly handle cases where
pos_init != 0. This fixes the previously reported off‑by‑one risk without changing behavior for the existing call sites that passpos_init == 0.
1464-1583: Bracket-aware URL splitting and cleanup logic look soundIn both
flb_utils_url_split()andflb_utils_url_split_sds():
- IPv6 hosts are first validated via
validate_ipv6_brackets, and when brackets are present you now search for the port separator':'starting at the closing']', avoiding confusion with IPv6 colons.flb_copy_host/flb_utils_copy_host_sdsare used to strip IPv6 brackets and return host literals like"::1"and"2001:db8::1", matching the new unit tests intests/internal/utils.c.- When the host substring is bracketed but malformed (missing
], empty[], or stray']'with no'['), both functions correctly fail and run the new error‑path cleanup.- Error paths now free any partially allocated
protocol,host,port, anduri(usingflb_free/flb_sds_destroyas appropriate), preventing leaks on invalid URLs or allocation failures.- The SDS variant continues to validate that the port consists only of digits before succeeding, which is consistent with its previous behavior.
Overall, the new parsing and cleanup behavior aligns with the added IPv6 URL tests and should be robust for both IPv4/hostname and bracketed IPv6 URLs.
Also applies to: 1585-1729
ShelbyZ
force-pushed
the
ipv6-changes
branch
from
347df83 to
b0a448a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/internal/http_client.c (2)
495-555: Clarifyflb_http_clientflags usage in TLS helperIn
test_tls_host_header_format, the last argument toflb_http_clientisFLB_IO_TLS, which is an I/O/upstream flag, while this parameter is conceptually for HTTP client flags (e.g., protocol/version options). TLS detection is already driven offu_conn_tls->upstreamviaflb_stream_get_flag_status, so passingFLB_IO_TLShere is unnecessary and could become confusing or brittle if bit spaces ever overlap.Consider passing
0for the flags argument and relying solely on the upstream flags to indicate TLS:- struct flb_http_client *c = flb_http_client(u_conn_tls, FLB_HTTP_GET, "/", - NULL, 0, host, port, NULL, FLB_IO_TLS); + struct flb_http_client *c = flb_http_client(u_conn_tls, FLB_HTTP_GET, "/", + NULL, 0, host, port, NULL, 0);The rest of the helper’s allocation and cleanup paths look sound.
557-668: Host header test coverage is strong; consider one more TLS+IPv6 non‑default port caseThe new tests exercise:
- IPv6 unbracketed vs bracketed hosts (
::1,[::1]) with ports,- IPv4 and domain hosts,
- TLS with default and non‑default ports,
- Multiple IPv6 formats and zone IDs (bracketed and unbracketed),
- IPv6+TLS default port (
[::1]without port in Host).This is a good spread and aligns with the new Host header rules. As a small extension, you might add a TLS + IPv6 + non‑standard port test (e.g.,
::1/ 8443) to mirrortest_https_non_standard_port_host_headerand lock in the expected[::1]:8443behavior under TLS with a non‑443 port. Optional, but would round out the matrix.Also applies to: 678-689
src/flb_utils.c (1)
1441-1474: IPv6 bracket validation now correctly scoped to the host portion
validate_ipv6_bracketslimits its search for']'to the substring before the first'/', and distinguishes between bracketed and non‑bracketed hosts. This prevents false negatives for URLs that legitimately contain']'in the path or query while still rejecting malformed host parts such as[]or hosts with a stray']'but no leading'['. The optionalout_bracketpointer is also a clean way to hand the closing bracket location back to callers.One minor stylistic point: since
pisconst char *, makingout_bracketaconst char **(andbracket/closingconst char *) would improve const‑correctness, but that’s purely optional and internal.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/flb_http_client.c(2 hunks)src/flb_network.c(1 hunks)src/flb_utils.c(8 hunks)tests/internal/http_client.c(2 hunks)tests/internal/utils.c(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/internal/utils.c
- src/flb_http_client.c
- src/flb_network.c
🧰 Additional context used
🧬 Code graph analysis (2)
tests/internal/http_client.c (4)
src/flb_http_client.c (3)
flb_http_get_header(1029-1046)flb_http_client_destroy(1715-1722)flb_http_client(841-886)src/flb_upstream.c (2)
flb_upstream_destroy(656-698)flb_upstream_create(290-363)include/fluent-bit/flb_mem.h (2)
flb_free(126-128)flb_calloc(84-96)src/flb_sds.c (1)
flb_sds_destroy(389-399)
src/flb_utils.c (2)
src/flb_sds.c (2)
flb_sds_create_len(58-76)flb_sds_create(78-90)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
🔇 Additional comments (3)
tests/internal/http_client.c (1)
469-493: check_host_header cleanup now avoids use-after-free and looks correctThe revised error path destroys the HTTP client, then (optionally) destroys the upstream via
extra_cleanup->upstreambefore freeing the connection, and finally tears down the test context and exits. This ordering removes the previous use-after-free risk while keeping behavior confined to the failure path. No issues from a correctness or lifetime perspective.src/flb_utils.c (2)
1428-1438: Corrected substring length inflb_utils_copy_host_sdsThe new length computation for IPv6 (
pos_end - pos_init - 2) and the non‑IPv6 branch (pos_end - pos_init) now both respectpos_initas an offset, fixing the earlier off‑by‑one risk whenpos_init != 0while preserving current call sites that usepos_init == 0. This makes the helper safe for future callers that might pass non‑zero offsets.
1486-1517: Bracket‑aware URL splitting and cleanup paths look correctThe changes to both
flb_utils_url_splitandflb_utils_url_split_sdsintegratevalidate_ipv6_bracketsand use itsbracketout‑parameter to:
- Decide whether to search for the port separator
':'starting at the closing']'(IPv6 literal) or from the start of the host (IPv4/domain).- Ensure that any
':'chosen as a port separator occurs before the first'/', so colons in the path don’t affect parsing.- Route bracket mismatches or invalid host extractions (
flb_copy_host/flb_utils_copy_host_sdsreturningNULL) into a common error path.The new null‑checks on
host(and the existing checks forport/uriin the SDS variant) combined with the expandederror:blocks—now freeing protocol, host, port, and uri (viaflb_freeorflb_sds_destroyas appropriate)—ensure that malformed URLs or allocation failures surface as-1without leaking memory.Behavior for well‑formed IPv4, domain, and bracketed IPv6 URLs is preserved while malformed or mismatched bracket cases are now rejected cleanly.
Also applies to: 1548-1560, 1580-1592, 1608-1638, 1669-1681, 1726-1740
This commit fixes IPv6 address handling in HTTP client Host headers by adding bracket notation when required and improving URL parsing validation. Changes: - Add automatic bracket wrapping for unbracketed IPv6 addresses in Host headers for both standard and non-standard ports - Add IPv6 bracketing for HTTPS default port (443) to ensure RFC compliance even when port is omitted (e.g., Host: [::1]) - Fix off-by-one error in IPv6 bracket stripping (was removing one extra character) - Fix incorrect length calculation in flb_utils_copy_host_sds for bracketed IPv6 extraction (changed from absolute position to relative length to properly account for pos_init offset) - Constrain IPv6 bracket validation to host portion only, preventing false negatives when brackets appear in URL paths or query strings - Use memchr with length limit for consistent and safe bracket detection in both IPv6 and non-IPv6 cases - Improve error handling in URL parsing with proper cleanup on failure - Update TLS flag checking to use flb_stream_get_flag_status() for more reliable detection Tests: - Add test for IPv6 with HTTPS on default port 443 - Add test cases for brackets in URL paths and query strings - Add test cases for malformed bracket scenarios Signed-off-by: Shelby Hagman <shelbyzh@amazon.com>
ShelbyZ
force-pushed
the
ipv6-changes
branch
from
b0a448a to
03ce384
Compare
1 participant
This commit fixes IPv6 address handling in HTTP client Host headers
by adding bracket notation when required and improving URL parsing
validation.
Changes:
Host headers for both standard and non-standard ports
compliance even when port is omitted (e.g., Host: [::1])
extra character)
bracketed IPv6 extraction (changed from absolute position to
relative length to properly account for pos_init offset)
false negatives when brackets appear in URL paths or query strings
detection in both IPv6 and non-IPv6 cases
more reliable detection
Tests:
Addresses #10699
ECR images with changes:
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
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.