oauth2: generic client and server side auth implementation (WIP) #11286
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds OAuth2 client and OAuth2‑JWT validation (JWKS fetch/cache, JWT parsing, RSA verification), integrates OAuth2 into HTTP client with retry-on-401, extends crypto API with RSA-from-components verification, wires config-map and properties for input/output plugins, and adds tests and build entries. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as External Client
participant InHTTP as in_http Plugin
participant JWTctx as flb_oauth2_jwt_ctx
participant JWKS as JWKS Server
participant Crypto as flb_crypto
participant OutHTTP as flb_http_client
participant OAuth2 as OAuth2 Token Server
Note over Client,InHTTP: Incoming request validation
Client->>InHTTP: HTTP request (Authorization: Bearer ...)
InHTTP->>JWTctx: flb_oauth2_jwt_validate(Authorization)
JWTctx->>JWTctx: parse token, extract kid/alg/claims
JWTctx->>JWTctx: check JWKS cache for kid
alt JWKS miss/stale
JWTctx->>JWKS: GET JWKS URL
JWKS-->>JWTctx: JWKS JSON
JWTctx->>JWTctx: parse & cache keys
end
JWTctx->>Crypto: build EVP_PKEY from modulus/exponent
Crypto-->>JWTctx: verification result
JWTctx-->>InHTTP: VALID / INVALID
alt VALID
InHTTP->>Client: 201 / process request
else INVALID
InHTTP->>Client: 401 Unauthorized
end
Note over OutHTTP,OAuth2: Outbound OAuth2 flow for plugin requests
OutHTTP->>OAuth2: request token (if needed)
OAuth2-->>OutHTTP: access_token + expires_in
OutHTTP->>OutHTTP: cache token (expires_at)
OutHTTP->>OutHTTP: send request with Authorization: Bearer
alt Upstream 401
OutHTTP->>OAuth2: invalidate/refresh token
OAuth2-->>OutHTTP: new token
OutHTTP->>OutHTTP: retry request with refreshed token
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
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 |
…ation This commit introduces a new OAuth2 JWT validation interface that allows Fluent Bit to validate incoming JWT tokens on the server side. The interface supports: - JWT parsing and validation - JWKS (JSON Web Key Set) fetching and caching - RSA signature verification using flb_crypto abstraction - Configurable validation rules (issuer, audience, client ID) Signed-off-by: Eduardo Silva <[email protected]>
This commit extends the flb_crypto abstraction layer to support RSA signature verification operations. The new functions handle OpenSSL 1.1.1 and 3.x compatibility internally, providing a unified API for cryptographic operations. New functions: - flb_crypto_build_rsa_public_key_from_components() - flb_crypto_init_from_rsa_components() - flb_crypto_verify() - flb_crypto_verify_simple() The implementation uses a hybrid approach for OpenSSL 3.x compatibility while maintaining functionality with OpenSSL 1.1.1. Signed-off-by: Eduardo Silva <[email protected]>
This commit adds OAuth2 JWT validation to the HTTP input plugin, allowing Fluent Bit to validate incoming bearer tokens. The implementation uses an independent config map pattern similar to net.* properties. Configuration properties: - oauth2.validate: enable/disable validation - oauth2.issuer: expected issuer claim - oauth2.jwks_url: JWKS endpoint URL - oauth2.allowed_audience: required audience claim - oauth2.allowed_clients: list of authorized client IDs - oauth2.jwks_refresh_interval: JWKS cache refresh interval The validation is performed lazily (JWKS is fetched on first use) to avoid blocking plugin initialization. Signed-off-by: Eduardo Silva <[email protected]>
This commit adds OAuth2 client credentials grant flow to the HTTP output plugin, allowing Fluent Bit to obtain and use OAuth2 access tokens for outgoing requests. The implementation uses an independent config map pattern similar to net.* properties. Configuration properties: - oauth2.enable: enable/disable OAuth2 - oauth2.token_url: OAuth2 token endpoint - oauth2.client_id: OAuth2 client ID - oauth2.client_secret: OAuth2 client secret - oauth2.scope: optional OAuth2 scope - oauth2.auth_method: authentication method (basic or post) - oauth2.refresh_skew_seconds: token refresh skew The implementation includes automatic token refresh on 401 responses and proper connection handling for retries. Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/flb_oauth2_jwt.c (1)
1363-1397: Add mutex protection for JWKS cache concurrent access.The
oauth2_ctxis created once per HTTP plugin instance and shared across all concurrent connections. Multiple request handlers can simultaneously access and modifyctx->jwks_cache(check/updatelast_refresh, fetch keys, lookup entries) without synchronization, creating race conditions during:
- Cache refresh check and JWKS fetch (lines 1363-1370)
- Hash table lookups and insertions (lines 1374-1376)
- Cache clearing operations (line 1077)
Add a
pthread_mutex_ttostruct flb_oauth2_jwt_ctxand protect all jwks_cache access with lock/unlock calls.
🧹 Nitpick comments (3)
src/flb_oauth2.c (2)
317-411: Consider handling insufficient token buffer in JSON parsing.The JSON parser uses a fixed token buffer size of 32 (line 324). If the OAuth2 response contains more than 32 JSON tokens (e.g., additional fields, nested objects),
jsmn_parsewill fail. While OAuth2 responses are typically simple, some providers may include extra metadata or error details.Consider checking if
jsmn_parsereturnsJSMN_ERROR_NOMEMand reallocating the token buffer, or at minimum, document this limitation and log a more specific error message when the buffer is insufficient.🔎 Example approach to handle token buffer overflow
- jsmn_init(&parser); - tokens = flb_calloc(1, sizeof(jsmntok_t) * tokens_size); - if (!tokens) { - flb_errno(); - return -1; - } - - ret = jsmn_parse(&parser, json_data, json_size, tokens, tokens_size); - if (ret <= 0) { - flb_error("[oauth2] cannot parse payload"); - flb_free(tokens); - return -1; - } + jsmn_init(&parser); + tokens = flb_calloc(1, sizeof(jsmntok_t) * tokens_size); + if (!tokens) { + flb_errno(); + return -1; + } + + ret = jsmn_parse(&parser, json_data, json_size, tokens, tokens_size); + if (ret == JSMN_ERROR_NOMEM) { + flb_error("[oauth2] JSON response too complex (exceeds %d tokens)", tokens_size); + flb_free(tokens); + return -1; + } + else if (ret <= 0) { + flb_error("[oauth2] cannot parse payload"); + flb_free(tokens); + return -1; + }
742-794: Payload append function doesn't URL-encode values.The
flb_oauth2_payload_appendfunction appends raw key-value pairs to the payload without URL-encoding, unlikeoauth2_append_kvwhich encodes values usingflb_uri_encode. This may be intentional if callers are expected to provide pre-encoded values, but it could lead to malformed request bodies if special characters (e.g.,&,=, spaces) are present in keys or values.Consider whether URL encoding should be applied here, or document that callers must provide URL-safe values.
src/flb_oauth2_jwt.c (1)
996-1001: Consider more robust port parsing.The current code uses
atoiwhich has limited error detection. While the<= 0check catches most invalid cases,strtolwith proper error checking would be more robust for detecting malformed port strings.🔎 Optional improvement using strtol
if (port_str && port_str[0] != '\0') { - port = atoi(port_str); - if (port <= 0) { + char *endptr; + long port_long = strtol(port_str, &endptr, 10); + if (*endptr != '\0' || port_long <= 0 || port_long > 65535) { flb_error("[oauth2_jwt] invalid port in JWKS URL"); goto cleanup; } + port = (int)port_long; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/flb_oauth2.c(5 hunks)src/flb_oauth2_jwt.c(1 hunks)tests/internal/input_chunk_routes.c(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T18:45:02.283Z
Learnt from: edsiper
Repo: fluent/fluent-bit PR: 11286
File: src/flb_http_client.c:1657-1715
Timestamp: 2025-12-17T18:45:02.283Z
Learning: In the Fluent Bit OAuth2 implementation (src/flb_oauth2.c), the function flb_oauth2_get_access_token returns a pointer to the cached token stored in the OAuth2 context (ctx->access_token), not a new allocation. The OAuth2 context manages the token lifecycle, so callers must not free the returned token pointer.
Applied to files:
src/flb_oauth2_jwt.csrc/flb_oauth2.c
🧬 Code graph analysis (2)
tests/internal/input_chunk_routes.c (1)
src/flb_kv.c (1)
flb_kv_release(123-133)
src/flb_oauth2.c (1)
src/flb_config_map.c (1)
flb_config_map_create(252-361)
⏰ 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). (31)
- 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=On, 3.31.6, clang, clang++)
- 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, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 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_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 (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=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 (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=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=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: PR - fuzzing test
- 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-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (15)
tests/internal/input_chunk_routes.c (1)
118-118: LGTM! Previous cleanup concern resolved.The initialization of
oauth2_jwt_propertiesat line 118 is now properly paired with cleanup at line 244. This follows the same pattern aspropertiesandnet_properties, ensuring no resource leaks.Also applies to: 244-244
src/flb_oauth2.c (12)
35-84: Config map is well-structured.The OAuth2 configuration map properly defines all necessary fields with appropriate types, defaults, and documentation. The use of
offsetoffor direct struct mapping is correct.
100-138: Helper functions look correct.The helper functions
key_cmp,oauth2_reset_state, andoauth2_apply_defaultsare properly implemented. The default forenablednow correctly matches the config map default ofFLB_FALSE.
140-223: Config cloning and cleanup is properly implemented.The
oauth2_clone_configfunction now correctly handles allocation failures by callingflb_oauth2_config_destroyon all error paths, preventing the memory leak identified in previous reviews. The cleanup function properly destroys all SDS strings.
225-315: Upstream setup handles both HTTP and HTTPS correctly.The
oauth2_setup_upstreamfunction properly parses the authentication URL, validates the protocol, creates the appropriate upstream connection (TCP or TLS), and applies the connect timeout from configuration. Error handling correctly cleans up temporary buffers.
505-579: HTTP request handling is correct.The
oauth2_http_requestfunction properly manages the upstream connection, sets appropriate timeouts from configuration, handles both Basic and POST authentication methods, and ensures the connection is released on all code paths (success and error).
581-621: Token refresh logic is well-implemented.The token refresh logic correctly determines when a token needs refreshing (considering the configured skew), builds the request body, makes the HTTP request, and properly cleans up the body buffer after use.
623-640: Creation function properly delegates to config-based constructor.The
flb_oauth2_createfunction correctly creates a temporary config and delegates toflb_oauth2_create_from_config, then cleans up the temporary config. Theexpire_secparameter is marked as unused (line 629), which suggests it may be retained for API compatibility or future use.
642-700: Config-based creation has comprehensive error handling.The
flb_oauth2_create_from_configfunction properly validates inputs (especiallytoken_url), allocates all necessary resources, and ensures all error paths callflb_oauth2_destroyfor consistent cleanup. Lock initialization and upstream setup are correctly validated.
702-728: Destroy function comprehensively cleans up all resources.The
flb_oauth2_destroyfunction properly cleans up all allocated resources including state, SDS strings, TLS context, upstream connection, config, and lock. The NULL check at the beginning prevents issues with double-free or invalid pointers.
796-899: Token access functions correctly implement locking and lifecycle management.The token access functions properly use the context lock to ensure thread-safe access to the cached token. The
flb_oauth2_get_access_tokenfunction correctly implements the retry-on-refresh pattern, andflb_oauth2_invalidate_tokensafely marks tokens as expired. Based on learnings, callers receive a pointer to the cached token and must not free it.
901-912: Config map accessor now properly handles errors.The
flb_oauth2_get_config_mapfunction now checks the return value offlb_config_map_createand logs an error if NULL is returned, addressing the concern from previous reviews.
413-452: Memory leak:tmpis not freed on error paths.The function
oauth2_append_kvallocates an encoded stringtmpat line 422 but only destroys it on the success path (line 449). If any of theflb_sds_catcalls at lines 429, 436, or 442 fail, the function returns NULL without freeingtmp, causing a memory leak.🔎 Proposed fix to free tmp on all error paths
static flb_sds_t oauth2_append_kv(flb_sds_t buffer, const char *key, const char *value) { flb_sds_t tmp; if (!value) { return buffer; } tmp = flb_uri_encode(value, strlen(value)); if (!tmp) { flb_errno(); return NULL; } if (flb_sds_len(buffer) > 0) { buffer = flb_sds_cat(buffer, "&", 1); if (!buffer) { flb_sds_destroy(tmp); return NULL; } } buffer = flb_sds_cat(buffer, key, strlen(key)); if (!buffer) { flb_sds_destroy(tmp); return NULL; } buffer = flb_sds_cat(buffer, "=", 1); if (!buffer) { flb_sds_destroy(tmp); return NULL; } buffer = flb_sds_cat(buffer, tmp, flb_sds_len(tmp)); flb_sds_destroy(tmp); return buffer; }Likely an incorrect or invalid review comment.
src/flb_oauth2_jwt.c (2)
1433-1462: LGTM: Allowed clients validation is well-structured.The nested iteration through
allowed_clientscorrectly handles the multi-level list structure (flb_config_map_val → flb_slist_entry). Thegototo break out of nested loops is appropriate, and the logic properly rejects tokens without a matchingclient_id.
1-1516: Strong OAuth2 JWT implementation with thorough validation.This is a comprehensive and well-structured implementation that addresses all previously identified issues:
- ✅ JWT parsing with proper base64url decoding and validation
- ✅ JWKS fetching with TLS support and system certificate verification
- ✅ RSA signature verification using crypto abstraction
- ✅ Complete policy enforcement (expiration, issuer, audience, client_id)
- ✅ Security hardening (time-based refresh prevents DoS from malicious
kidvalues)- ✅ Proper resource cleanup in all error paths
- ✅ Array audience handling per JWT spec (RFC 7519)
The code quality is high, with clear structure, helpful comments, and defensive checks throughout.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/flb_oauth2.c (1)
413-452: Memory leak: original buffer lost whenflb_sds_catfails.When
flb_sds_catreturns NULL (due to allocation failure during resize), the original buffer is not freed byflb_sds_catitself - it remains allocated. However,oauth2_append_kvassigns NULL tobufferand returns NULL, causing the caller (oauth2_build_body) to lose the reference to the original buffer.For example at line 429: if
flb_sds_cat(buffer, "&", 1)fails,bufferbecomes NULL but the original SDS memory is leaked.🔎 Proposed fix to free buffer on error
static flb_sds_t oauth2_append_kv(flb_sds_t buffer, const char *key, const char *value) { flb_sds_t tmp; + flb_sds_t new_buf; if (!value) { return buffer; } tmp = flb_uri_encode(value, strlen(value)); if (!tmp) { flb_errno(); + flb_sds_destroy(buffer); return NULL; } if (flb_sds_len(buffer) > 0) { - buffer = flb_sds_cat(buffer, "&", 1); - if (!buffer) { + new_buf = flb_sds_cat(buffer, "&", 1); + if (!new_buf) { + flb_sds_destroy(buffer); flb_sds_destroy(tmp); return NULL; } + buffer = new_buf; } - buffer = flb_sds_cat(buffer, key, strlen(key)); - if (!buffer) { + new_buf = flb_sds_cat(buffer, key, strlen(key)); + if (!new_buf) { + flb_sds_destroy(buffer); flb_sds_destroy(tmp); return NULL; } + buffer = new_buf; - buffer = flb_sds_cat(buffer, "=", 1); - if (!buffer) { + new_buf = flb_sds_cat(buffer, "=", 1); + if (!new_buf) { + flb_sds_destroy(buffer); flb_sds_destroy(tmp); return NULL; } + buffer = new_buf; - buffer = flb_sds_cat(buffer, tmp, flb_sds_len(tmp)); + new_buf = flb_sds_cat(buffer, tmp, flb_sds_len(tmp)); flb_sds_destroy(tmp); + if (!new_buf) { + flb_sds_destroy(buffer); + return NULL; + } - return buffer; + return new_buf; }This issue was previously flagged in past reviews on lines 454-503 but the fix was not fully applied to
oauth2_append_kvitself.src/flb_oauth2_jwt.c (1)
59-63: Add mutex protection to JWKS cache for concurrent access safety.The context structure lacks a mutex for the
jwks_cache, exposing race conditions during concurrent JWT validation calls. Multiple threads may simultaneously check cache expiration, fetch and parse JWKS keys, and perform lookups without synchronization. Add a mutex field toflb_oauth2_jwks_cache, initialize it inoauth2_jwks_cache_init(), and protect all cache access inflb_oauth2_jwt_validate()and JWKS fetch/parse operations.
🧹 Nitpick comments (1)
src/flb_oauth2.c (1)
885-899: Consider adding lock protection forflb_oauth2_token_expired.This function reads
ctx->expires_atwithout acquiring the lock, whileflb_oauth2_invalidate_tokenand the refresh logic modify it under lock. On most platforms reading atime_tis atomic, but for strict thread-safety consistency with the rest of the API, consider protecting this read.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
include/fluent-bit/flb_compat.h(3 hunks)include/fluent-bit/flb_oauth2_jwt.h(1 hunks)src/flb_oauth2.c(5 hunks)src/flb_oauth2_jwt.c(1 hunks)tests/internal/input_chunk_routes.c(2 hunks)tests/internal/oauth2.c(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/internal/input_chunk_routes.c
- include/fluent-bit/flb_compat.h
- tests/internal/oauth2.c
- include/fluent-bit/flb_oauth2_jwt.h
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T18:45:02.283Z
Learnt from: edsiper
Repo: fluent/fluent-bit PR: 11286
File: src/flb_http_client.c:1657-1715
Timestamp: 2025-12-17T18:45:02.283Z
Learning: In the Fluent Bit OAuth2 implementation (src/flb_oauth2.c), the function flb_oauth2_get_access_token returns a pointer to the cached token stored in the OAuth2 context (ctx->access_token), not a new allocation. The OAuth2 context manages the token lifecycle, so callers must not free the returned token pointer.
Applied to files:
src/flb_oauth2_jwt.csrc/flb_oauth2.c
🧬 Code graph analysis (2)
src/flb_oauth2_jwt.c (1)
src/flb_utils.c (1)
flb_utils_url_split(1441-1534)
src/flb_oauth2.c (3)
src/flb_sds.c (5)
flb_sds_destroy(389-399)flb_sds_create(78-90)flb_sds_create_len(58-76)flb_sds_cat(120-141)flb_sds_increase(98-118)include/fluent-bit/flb_mem.h (2)
flb_calloc(84-96)flb_free(126-128)src/flb_config_map.c (1)
flb_config_map_create(252-361)
⏰ 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). (25)
- 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: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 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 (-DSANITIZE_UNDEFINED=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_COMPILER_STRICT_POINTER_TYPES=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_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, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 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_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: PR - fuzzing test
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=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_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
🔇 Additional comments (15)
src/flb_oauth2_jwt.c (9)
1-64: LGTM on file structure and type definitions.The file is well-organized with clear structure definitions for JWKS keys, cache, and JWT context. The includes cover all necessary dependencies for JWT parsing, HTTP client operations, and cryptographic verification.
126-165: LGTM on cache clear implementation.The function properly preserves the refresh interval, destroys all cached keys to prevent memory leaks, and recreates the hash table. Error propagation was correctly addressed with the return value.
265-374: LGTM on base64url decode implementation.The two-pass approach (sizing pass then decode pass) is correctly implemented. The function properly handles base64url to base64 conversion, padding, whitespace skipping, and character validation with appropriate cleanup on all error paths.
459-597: LGTM on JWT payload parsing.The implementation correctly handles all required claims (exp, iss, aud) and optional claims (azp, client_id). The
has_azpflag ensures proper precedence when bothazpandclient_idare present, and negative expiration values are properly rejected.
599-703: LGTM on JWT parse function.The signing input construction correctly uses
flb_sds_create_sizewith proper length calculation. All error paths properly cleanup viaflb_oauth2_jwt_destroy, and segment validation is thorough.
705-809: LGTM on JWKS key parsing.The function correctly validates RSA key type, extracts required components (kid, n, e), and properly cleans up partial allocations on error paths.
963-1125: LGTM on JWKS fetch implementation.The function properly handles default ports for HTTP/HTTPS, sets up TLS with hostname verification, clears the cache before refreshing (preventing stale keys), and uses a separate success flag for correct return value handling. The cleanup section properly releases all resources.
1127-1219: LGTM on audience check implementation.The function correctly handles both string and array
audclaim formats per JWT spec (RFC 7519), returning true if any element matches the allowed audience.
1274-1470: LGTM on JWT validation function.The validation flow correctly implements:
- Time-based only JWKS refresh (preventing DoS via random kid values)
- Proper signature verification using JWKS
- Expiration, issuer, and audience validation
- Nested config map iteration for allowed clients
The function properly cleans up the JWT structure on all exit paths.
src/flb_oauth2.c (6)
34-84: LGTM on OAuth2 config map definition.The config map properly defines all OAuth2 configuration options with appropriate types, defaults, and descriptions. The structure aligns well with the
flb_oauth2_configstruct.
140-205: LGTM on config clone implementation.Each string allocation failure properly calls
flb_oauth2_config_destroy(dst)before returning, preventing the partial allocation leak that was flagged in past reviews.
505-579: LGTM on HTTP request implementation.The function properly handles upstream connection with IPv6 fallback, applies configured timeouts, sets up basic authentication when required, and correctly parses the token response. All error paths release resources appropriately.
642-700: LGTM on context creation from config.The function properly validates required configuration (token_url), initializes the lock for thread safety, and cleans up on all error paths via
flb_oauth2_destroy.
814-838: LGTM on get access token implementation.The function correctly uses locking for thread safety. Per the retrieved learnings,
token_outreceives a pointer to the cached token stored in the OAuth2 context (ctx->access_token), not a new allocation - callers must not free the returned pointer. This is consistent with the implementation at line 809 inoauth2_get_token_locked.
901-912: LGTM on config map accessor.The function now properly checks for NULL return from
flb_config_map_createand logs an error before returning, addressing the previous review concern.
Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/flb_oauth2.c (1)
909-932: Consider adding lock protection for token_len and token_expired.These query functions read shared state (
access_token,expires_at) without lock protection, which could lead to race conditions during concurrent token refresh operations. On 32-bit systems, readingexpires_at(likely 64-bittime_t) may tear. While the impact is minor (stale reads), adding lock protection would ensure consistency.🔎 Proposed fix to add lock protection
int flb_oauth2_token_len(struct flb_oauth2 *ctx) { + int ret; + int len; + + ret = flb_lock_acquire(&ctx->lock, + FLB_LOCK_DEFAULT_RETRY_LIMIT, + FLB_LOCK_DEFAULT_RETRY_DELAY); + if (ret != 0) { + return -1; + } + if (!ctx->access_token) { + flb_lock_release(&ctx->lock, + FLB_LOCK_DEFAULT_RETRY_LIMIT, + FLB_LOCK_DEFAULT_RETRY_DELAY); return -1; } - return flb_sds_len(ctx->access_token); + len = flb_sds_len(ctx->access_token); + + flb_lock_release(&ctx->lock, + FLB_LOCK_DEFAULT_RETRY_LIMIT, + FLB_LOCK_DEFAULT_RETRY_DELAY); + + return len; } int flb_oauth2_token_expired(struct flb_oauth2 *ctx) { + int ret; + int expired; time_t now; + ret = flb_lock_acquire(&ctx->lock, + FLB_LOCK_DEFAULT_RETRY_LIMIT, + FLB_LOCK_DEFAULT_RETRY_DELAY); + if (ret != 0) { + return FLB_TRUE; + } + if (!ctx->access_token) { + flb_lock_release(&ctx->lock, + FLB_LOCK_DEFAULT_RETRY_LIMIT, + FLB_LOCK_DEFAULT_RETRY_DELAY); return FLB_TRUE; } now = time(NULL); - if (ctx->expires_at <= now) { - return FLB_TRUE; + expired = (ctx->expires_at <= now) ? FLB_TRUE : FLB_FALSE; + + flb_lock_release(&ctx->lock, + FLB_LOCK_DEFAULT_RETRY_LIMIT, + FLB_LOCK_DEFAULT_RETRY_DELAY); + + return expired; - } - - return FLB_FALSE; }
🧹 Nitpick comments (2)
src/flb_crypto.c (1)
156-264: Clarify the comment about encoding.The comment at line 156 says "base64url encoded", but the function signature accepts raw bytes (
unsigned char *modulus_bytes,unsigned char *exponent_bytes). The function expects decoded bytes, not base64url strings.Update the comment to clarify that these are raw bytes (e.g., "Build RSA public key from modulus and exponent (raw bytes, typically decoded from base64url JWK)").
🔎 Proposed comment fix
-/* Build RSA public key from modulus and exponent (base64url encoded) */ +/* Build RSA public key from modulus and exponent (raw bytes) */ static int flb_crypto_build_rsa_public_key_from_components(unsigned char *modulus_bytes,
Memory management is correct.
The ownership transfers are properly handled in both OpenSSL 1.x and 3.x paths, and the fix for the OpenSSL 3.x memory leak (storing duplicates in temporaries and freeing on RSA_set0_key failure) is confirmed as correct.
src/flb_oauth2.c (1)
225-315: Consider deferring TLS context creation to HTTPS-only paths.The function creates a TLS context at line 275 regardless of protocol, but only uses it for HTTPS at line 290. For HTTP connections (line 293), the TLS context remains unused yet allocated. This is a minor resource inefficiency.
🔎 Proposed refactor to conditionally create TLS
- ctx->tls = flb_tls_create(FLB_TLS_CLIENT_MODE, - FLB_TRUE, - -1, - NULL, - NULL, - NULL, - NULL, - NULL, - NULL); - if (!ctx->tls) { - flb_error("[oauth2] error initializing TLS context"); - goto error; - } - if (strcmp(prot, "https") == 0) { + ctx->tls = flb_tls_create(FLB_TLS_CLIENT_MODE, + FLB_TRUE, + -1, + NULL, + NULL, + NULL, + NULL, + NULL, + NULL); + if (!ctx->tls) { + flb_error("[oauth2] error initializing TLS context"); + goto error; + } ctx->u = flb_upstream_create_url(config, auth_url, FLB_IO_TLS, ctx->tls); } else { ctx->u = flb_upstream_create_url(config, auth_url, FLB_IO_TCP, NULL); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/flb_crypto.c(3 hunks)src/flb_oauth2.c(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T18:45:02.283Z
Learnt from: edsiper
Repo: fluent/fluent-bit PR: 11286
File: src/flb_http_client.c:1657-1715
Timestamp: 2025-12-17T18:45:02.283Z
Learning: In the Fluent Bit OAuth2 implementation (src/flb_oauth2.c), the function flb_oauth2_get_access_token returns a pointer to the cached token stored in the OAuth2 context (ctx->access_token), not a new allocation. The OAuth2 context manages the token lifecycle, so callers must not free the returned token pointer.
Applied to files:
src/flb_oauth2.c
🧬 Code graph analysis (1)
src/flb_oauth2.c (3)
src/flb_sds.c (6)
flb_sds_destroy(389-399)flb_sds_create(78-90)flb_sds_create_len(58-76)flb_sds_cat(120-141)flb_sds_increase(98-118)flb_sds_cat_safe(204-214)src/flb_lock.c (4)
flb_lock_init(6-17)flb_lock_destroy(19-30)flb_lock_acquire(32-65)flb_lock_release(67-100)src/flb_config_map.c (1)
flb_config_map_create(252-361)
⏰ 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). (31)
- 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 32bit, x86, x86-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_SIMD=Off, 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_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_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=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 (-DFLB_SIMD=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 (-DSANITIZE_ADDRESS=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_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 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 (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 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_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: PR - fuzzing test
- 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, clang, clang++, 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-without-cxx (3.31.6)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (26)
src/flb_crypto.c (5)
20-23: LGTM!The new OpenSSL headers are necessary for RSA key construction from modulus/exponent components and version detection.
26-62: LGTM!The OpenSSL 1.0.2 compatibility layer is well-implemented:
- EVP_MD_CTX macros correctly map to create/destroy equivalents
- RSA_set0_key shim properly handles ownership transfer semantics (frees old values, assigns new values, returns 1 on success)
- Direct RSA field access is valid for the opaque-structure-free OpenSSL 1.0.x
556-603: LGTM!The new
flb_crypto_init_from_rsa_componentsAPI is well-structured, mirrors the existingflb_crypto_initpattern, and properly handles all error paths with cleanup.
670-701: LGTM!The
flb_crypto_verify_simpleconvenience wrapper follows the established pattern of other_simplefunctions in this module and correctly handles initialization, verification, and cleanup.
643-656: Verify EVP_DigestVerify availability in OpenSSL 1.1.0.The version check at line 643 assumes
EVP_DigestVerifyis available in OpenSSL 1.1.0+ (>= 0x10100000L). Confirm whether this function was introduced in 1.1.0 or 1.1.1, and adjust the ifdef if needed (e.g.,>= 0x10101000Lfor 1.1.1).src/flb_oauth2.c (21)
34-84: LGTM! OAuth2 config map is well-structured.The configuration map properly defines all OAuth2 parameters with appropriate types, offsets, and descriptions. The timeout defaults correctly indicate fallback behavior.
100-107: LGTM! Clean key comparison helper.The inline function correctly validates length before comparing, preventing out-of-bounds access.
109-123: LGTM! State reset is properly implemented.The function safely destroys existing tokens and resets expiry fields, preventing use-after-free issues.
125-138: LGTM! Defaults are correctly initialized.The function properly initializes all fields, including explicit NULL initialization for pointers to prevent use of uninitialized memory.
140-205: LGTM! Config cloning with proper error cleanup.The function correctly handles allocation failures by calling
flb_oauth2_config_destroy(dst)on each error path, preventing the memory leak identified in previous reviews.
207-223: LGTM! Config cleanup is thorough.The function safely destroys all allocated strings and nullifies pointers, ensuring clean resource release.
413-474: LGTM! Key-value appending with robust error handling.The function correctly checks all
flb_sds_catreturn values and destroys the buffer on failure, addressing the memory leak concern from previous reviews. URL encoding and error paths are properly handled.
476-536: LGTM! Request body building with proper cleanup.The function correctly handles allocation failures by destroying the body buffer on each error path, resolving the memory leak identified in previous reviews. The manual payload fallback is also handled correctly.
538-612: LGTM! HTTP request handling is robust.The function correctly manages the upstream connection lifecycle, handles both TCP and TLS protocols, conditionally applies timeouts, and properly cleans up resources on all paths. Basic authentication is correctly applied only when configured.
614-629: LGTM! Token refresh wrapper is clean.The function correctly builds the request body, executes the HTTP request, and ensures cleanup regardless of outcome.
631-654: LGTM! Token refresh logic is correct.The function properly evaluates all conditions requiring token refresh, including forced refresh, missing token, zero expiry, and time-based expiry with skew.
656-673: LGTM! Convenience constructor is correctly implemented.The function properly creates a temporary config, delegates to the main constructor, and cleans up the temporary config. The unused
expire_secparameter is appropriately marked.
675-733: LGTM! Main constructor with comprehensive initialization.The function properly initializes the OAuth2 context, validates required configuration (token_url), initializes synchronization primitives, and uses consistent error handling by calling
flb_oauth2_destroyon all failure paths.
735-761: LGTM! Destructor thoroughly cleans up all resources.The function systematically destroys all allocated resources including tokens, strings, TLS context, upstream, config, and synchronization primitives.
763-773: LGTM! Payload clearing is correctly implemented.The function safely clears the payload buffer, sets the manual flag, and resets token state to ensure consistency.
775-827: LGTM! Payload appending uses safe string operations.The function correctly uses
flb_sds_cat_safefor all append operations, addressing the unchecked return value concern from previous reviews. Buffer growth and reallocation are properly handled.
829-845: LGTM! Token retrieval logic is sound.The function correctly evaluates refresh necessity, refreshes if needed, and returns the cached token pointer as documented in the learnings.
847-871: LGTM! Thread-safe token access is correctly implemented.The function properly guards token retrieval with mutex acquisition and release, ensuring thread-safe access to the shared token state.
873-889: LGTM! Token getter functions are straightforward.The functions provide a simple interface to retrieve the cached access token without forcing refresh, consistent with the documented token lifecycle management.
891-907: LGTM! Token invalidation with lock protection.The function safely invalidates the token by resetting its expiry timestamp under lock protection. The silent return on lock acquisition failure appears intentional for a best-effort invalidation operation.
934-945: LGTM! Config map creation with NULL check.The function correctly validates the return value from
flb_config_map_createand logs an appropriate error, addressing the concern from previous reviews.
| if (!token_type) { | ||
| token_type = flb_sds_create("Bearer"); | ||
| flb_debug("[oauth2] token_type missing; defaulting to Bearer"); | ||
| } |
There was a problem hiding this comment.
Check allocation failure when defaulting token_type to "Bearer".
If flb_sds_create("Bearer") fails at line 393, token_type remains NULL, but the code continues and sets ctx->token_type = NULL at line 406. While allocation failure for a small static string is rare, this could cause issues if downstream code assumes a valid token_type exists.
🔎 Proposed fix to handle allocation failure
if (!token_type) {
token_type = flb_sds_create("Bearer");
+ if (!token_type) {
+ flb_errno();
+ oauth2_reset_state(ctx);
+ return -1;
+ }
flb_debug("[oauth2] token_type missing; defaulting to Bearer");
}🤖 Prompt for AI Agents
In src/flb_oauth2.c around lines 392-395, the code defaults token_type to
flb_sds_create("Bearer") but does not check for allocation failure; if
flb_sds_create returns NULL the code later assigns a NULL token_type into ctx.
Fix by checking the return of flb_sds_create: if it returns NULL log an error
about allocation failure and handle it deterministically (either return an
error/cleanup from the current function or fall back to a safe non-allocating
option such as using a pointer to a static "Bearer" constant and a flag
indicating it is not an sds), ensuring downstream code never receives a NULL
token_type.
Signed-off-by: Eduardo Silva <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/internal/oauth2.c (2)
52-58: Consider documenting buffer size assumption.The
snprintfwill safely truncate if the formatted response exceedsMOCK_BODY_SIZE(1024 bytes). For test payloads, this is adequate, but consider adding a comment or assertion to make the assumption explicit.
367-367: Consider reducing test duration.The
sleep(2)makes this test take over 2 seconds to run. For faster test execution, consider using shorter expiry times (e.g.,expires_in=1, refresh_skew=0withsleep(1)) while still validating the refresh behavior.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/internal/oauth2.c(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T18:45:02.283Z
Learnt from: edsiper
Repo: fluent/fluent-bit PR: 11286
File: src/flb_http_client.c:1657-1715
Timestamp: 2025-12-17T18:45:02.283Z
Learning: In the Fluent Bit OAuth2 implementation (src/flb_oauth2.c), the function flb_oauth2_get_access_token returns a pointer to the cached token stored in the OAuth2 context (ctx->access_token), not a new allocation. The OAuth2 context manages the token lifecycle, so callers must not free the returned token pointer.
Applied to files:
tests/internal/oauth2.c
🧬 Code graph analysis (1)
tests/internal/oauth2.c (3)
src/flb_sds.c (2)
flb_sds_create_size(92-95)flb_sds_create(78-90)src/flb_oauth2.c (4)
flb_oauth2_create_from_config(675-733)flb_oauth2_parse_json_response(317-411)flb_oauth2_get_access_token(847-871)flb_oauth2_destroy(735-761)src/flb_config.c (1)
flb_config_init(233-486)
⏰ 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). (31)
- 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: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 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_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 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 (-DFLB_SIMD=Off, 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_MEMORY=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_SANITIZE_THREAD=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 (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=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_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: PR - fuzzing test
- 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)
- 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, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
- GitHub Check: pr-compile-without-cxx (3.31.6)
🔇 Additional comments (8)
tests/internal/oauth2.c (8)
101-104: Simple authorization check is adequate for test code.The
strstrapproach will matchlatest_tokenanywhere in the request after theAuthorization:header, which could theoretically produce false positives. For a mock test server, this is acceptable.
144-158: Blocking recv is acceptable for test server.The socket is set to blocking mode, and
recvis called in a bounded loop. If a client sends incomplete data,recvcould block, but the loop has proper bounds and break conditions. For a test server with controlled clients, this is acceptable.
173-232: Well-structured server startup with proper error handling.The server binds to loopback with dynamic port assignment, which is ideal for test isolation. Error paths properly clean up resources.
234-277: Excellent defensive programming for test reliability.The platform-specific readiness check prevents race conditions on macOS where the server thread may not have entered its
select()loop immediately afterpthread_createreturns. This is good practice for cross-platform test reliability.
279-287: Proper server shutdown sequence.The use of
shutdown()to wake blocking calls followed bypthread_joinensures clean thread termination. Since the server thread checks the stop flag every 200ms,pthread_joinshould complete quickly.
289-310: Helper function properly manages resources.The OAuth2 config is correctly initialized, used to create the context, and then destroyed. Since
flb_oauth2_create_from_configclones the config internally, this is safe.
312-329: Unit test correctly validates default value handling.The test verifies that
token_typedefaults to "Bearer" andexpires_indefaults to the constant when not present in the JSON response. Cleanup is properly handled.
331-377: Integration test thoroughly validates OAuth2 token lifecycle.The test correctly verifies:
- Initial token fetch from server
- Cached token reuse
- Token refresh after expiry
The cleanup sequence properly destroys resources in the correct order.
WIP --> This branch is under heavy development
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 / Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.