Skip to content

NPE Crash with Invalid Credentials in ClientPool::GuardedClient::TestConnection #29

@yuzifeng1984

Description

@yuzifeng1984

Problem

When connecting with empty or invalid credentials, the client can crash with a NullPointerException.

Issue 1: NPE in TestConnection() with nullptr client

File: timeplus/timeplus.cpp lines 74, 78, 96

Root Cause:

  1. GuardedClient default constructs with client = nullptr
  2. If GetGuardedClient() throws (e.g., TimeoutError when pool is exhausted), client remains nullptr
  3. The catch blocks call guarded_client.TestConnection() with client == nullptr
  4. TestConnection() calls client->Ping() causing NPE
void ExecuteWithRetries(std::function<void(Client&)> func, BaseResult& result) {
    ClientPool::GuardedClient guarded_client;  // client = nullptr
    for (int retries = config_.max_retries; retries >= 0; --retries) {
        try {
            if (guarded_client.client == nullptr) {
                guarded_client = client_pool_.GetGuardedClient(...);  // May throw TimeoutError
            }
            ...
        } catch (const ProtocolError& ex) {
            guarded_client.TestConnection();  // NPE if client is nullptr!
        } catch (const ServerError& ex) {
            guarded_client.TestConnection();  // NPE if client is nullptr!
        } catch (const std::exception& ex) {
            guarded_client.TestConnection();  // NPE if client is nullptr!
        }
    }
}

Fix for Issue 1:

Add null check in TestConnection() (timeplus/client_pool.cpp):

void ClientPool::GuardedClient::TestConnection() noexcept {
    if (!client) {
        valid = false;
        return;
    }
    try {
        client->Ping();
        valid = true;
    } catch (...) {
        valid = false;
    }
    ...
}

Issue 2: NPE in TRACE macro (debug builds only)

File: timeplus/client_pool.cpp line 40

GetCurrentEndpoint() returns std::optional<Endpoint>, but the TRACE macro dereferenced it without checking. This only affects debug builds with TRACE_TIMEPLUS_CPP defined.

Note: This was already fixed by adding a null check before the TRACE statement.

Files to Modify

  • timeplus/client_pool.cpp - Add null check for client in TestConnection()

Verification

  1. Build the project
  2. Test with invalid credentials or exhausted connection pool to verify no crash occurs

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions