Skip to content

Conversation

@bryancall
Copy link
Contributor

@bryancall bryancall commented Jan 29, 2026

Summary

Fix four Coverity UNINIT (uninitialized variable) issues:

  • CID 1497238: Value-initialize DiagsConfigState in reconfigure_diags()
  • CID 1644237: Value-initialize ParsedValue in ParsedConfigCache::parse()
  • CID 1644228: Value-initialize TLSClientHelloSummary in test_ja4.cc
  • CID 1533658: Fix inverted condition for Client-IP parsing in background_fetch and cache_fill plugins

The last fix (CID 1533658) is an actual logic bug introduced in #10980 - the condition cfg_value.front() == '*' should be cfg_value.front() != '*' to correctly skip loading when the value is exactly "*" (which signals "match any address" via an empty IPRange).

Value-initialize DiagsConfigState to ensure the outputs array
members are initialized to false before use. This fixes Coverity
CID 1497238 (UNINIT).
Explicitly value-initialize ParsedValue to ensure the variant member
is properly initialized. This fixes Coverity CID 1644237 (UNINIT).
Value-initialize TLSClientHelloSummary to ensure all members are
properly initialized before use. This fixes Coverity CID 1644228 (UNINIT).
The condition for parsing Client-IP was inverted - it should load the
IP range when the value is NOT a single '*' character. With the old
logic, single-character non-'*' values would skip loading, leaving
the IPRange uninitialized.

This fixes Coverity CID 1533658 (UNINIT).
@bryancall bryancall requested a review from ezelkow1 January 29, 2026 23:09
@bryancall bryancall self-assigned this Jan 29, 2026
@bryancall bryancall added this to the 10.2.0 milestone Jan 29, 2026
@bryancall bryancall added the Bug label Jan 29, 2026
@juice928
Copy link

👋 Hi, I'm an automated AI code review bot. I ran some checks on this PR and found 3 points that might be worth attention (could be false positives, please use your judgment):

  1. The logic for Client-IP wildcard handling could be refined to ensure correct parsing.

    • plugins/background_fetch/configs.cc:L139: plugins/background_fetch/configs.cc:L139
    • Impact: Current logic may incorrectly trigger parsing errors for '*' while skipping other valid single-character values.
    • Suggestion: Updating the condition to size != 1 || front != '*' should help correctly skip the wildcard and parse other values.
  2. Initializing the ParsedValue object in HttpConfig would improve memory safety.

    • src/proxy/http/HttpConfig.cc:L748: src/proxy/http/HttpConfig.cc:L748
    • Impact: Leaving members uninitialized might lead to unpredictable behavior or stability concerns during execution.
    • Suggestion: Consider using ParsedValue result{}; to ensure the structure is safely zero-initialized.
  3. It would be safer to initialize DiagsConfigState to avoid using uninitialized stack memory.

    • src/proxy/shared/DiagsConfig.cc:L45: src/proxy/shared/DiagsConfig.cc:L45
    • Impact: Ensuring this state is initialized helps maintain system stability and prevents risks from leftover data on the stack.
    • Suggestion: Consider changing the declaration to DiagsConfigState c{}; for consistent and safe initialization.

If you find these suggestions disruptive, you can reply "stop" , and I'll automatically skip this repository in the future.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses four Coverity UNINIT (uninitialized variable) warnings and fixes an actual logic bug introduced in #10980. The changes include value-initializing three struct/class instances to ensure all members are properly initialized, and correcting an inverted condition in Client-IP parsing logic that was preventing the wildcard "*" value from being handled correctly.

Changes:

  • Added value-initialization ({}) to fix three uninitialized variable warnings in DiagsConfig, HttpConfig, and ja4_fingerprint test code
  • Fixed inverted condition logic bug in background_fetch and cache_fill plugins to correctly handle the "*" wildcard for Client-IP matching

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/proxy/shared/DiagsConfig.cc Value-initialize DiagsConfigState to fix CID 1497238
src/proxy/http/HttpConfig.cc Value-initialize ParsedValue to fix CID 1644237
plugins/experimental/ja4_fingerprint/test_ja4.cc Value-initialize TLSClientHelloSummary to fix CID 1644228
plugins/experimental/cache_fill/configs.cc Fix inverted condition (== to !=) for Client-IP wildcard handling (CID 1533658)
plugins/background_fetch/configs.cc Fix inverted condition (== to !=) for Client-IP wildcard handling (CID 1533658)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants