Skip to content

[BUG] CLIENT TRACKING ON BCAST enables tracking despite detecting duplicate prefix collision when overlap is not at index 0 #3582

Description

@beebs-systap

Describe the bug

When CLIENT TRACKING ON BCAST PREFIX xxx PREFIX yyy PREFIX yyy is issued and the overlapping prefix pair is not at index 0 in the argument list, the server correctly detects the collision and sends an error reply — but then proceeds to enable tracking anyway and sends a second +OK reply. This results in a double reply (protocol violation) and overlapping prefixes being registered, which can cause duplicate invalidation messages.

In src/tracking.c, function checkPrefixCollisionsOrReply() documents a boolean contract: return 1 for no collision, return 0 for collision. The existing-prefix overlap check correctly does return 0; on collision. However, the self-overlap check does return i; — the outer loop variable.

When i == 0, return 0 is accidentally correct. When i > 0, the function returns a non-zero (truthy) value. The caller in src/networking.c checks:

if (!checkPrefixCollisionsOrReply(c, prefix, numprefix)) {
    zfree(prefix);
    return;
}

A truthy return causes the caller to fall through to enableTracking() and addReply(c, shared.ok). Since addReplyErrorFormat was already called inside checkPrefixCollisionsOrReply, the client receives two replies for one command — a protocol violation — and tracking is enabled with overlapping prefixes.

To reproduce

> CLIENT TRACKING ON BCAST PREFIX xxx PREFIX yyy PREFIX yyy
-ERR Prefix 'yyy' overlaps with another provided prefix 'yyy'. Prefixes for a single client must not overlap.
+OK

The duplicate prefix yyy is at index 1. The self-overlap is detected at i=1, j=2, so return i returns 1 (truthy). The caller does not abort, and both the error and +OK are sent.

Note: If the duplicate is the first prefix (PREFIX yyy PREFIX yyy PREFIX xxx), i=0 and return 0 accidentally works correctly. The bug only manifests when the colliding prefix pair is not at index 0.

Expected behavior

> CLIENT TRACKING ON BCAST PREFIX xxx PREFIX yyy PREFIX yyy
-ERR Prefix 'yyy' overlaps with another provided prefix 'yyy'. Prefixes for a single client must not overlap.

A single error reply, no +OK, and tracking remains disabled.

Additional information

  • Valkey version: unstable (HEAD)
  • Origin: Bug was introduced in redis/redis#8176 (Jan 2021) and inherited by Valkey at fork time.
  • The bug is a double-reply protocol violation, which can corrupt pipelined responses and confuse client libraries.
  • The bug only triggers when the colliding prefix pair is not at index 0 in the argument list (i.e., there must be at least one non-colliding prefix before the duplicate pair).
  • Suggested fix: Change return i; to return 0; in the self-overlap branch of checkPrefixCollisionsOrReply() at src/tracking.c:146, matching the function's documented contract and the pattern used by the existing-prefix collision check above it.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions