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

Fields

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