diff --git a/src/networking.c b/src/networking.c index 9a86e6085..e624dd8f9 100644 --- a/src/networking.c +++ b/src/networking.c @@ -2800,6 +2800,13 @@ NULL return; } + if (options & CLIENT_TRACKING_BCAST) { + if (!checkPrefixCollisionsOrReply(c,prefix,numprefix)) { + zfree(prefix); + return; + } + } + enableTracking(c,redir,options,prefix,numprefix); } else if (!strcasecmp(c->argv[2]->ptr,"off")) { disableTracking(c); diff --git a/src/server.h b/src/server.h index 586e085e9..9fe15685e 100644 --- a/src/server.h +++ b/src/server.h @@ -1849,6 +1849,7 @@ uint64_t trackingGetTotalItems(void); uint64_t trackingGetTotalKeys(void); uint64_t trackingGetTotalPrefixes(void); void trackingBroadcastInvalidationMessages(void); +int checkPrefixCollisionsOrReply(client *c, robj **prefix, size_t numprefix); /* List data type */ void listTypeTryConversion(robj *subject, robj *value); diff --git a/src/tracking.c b/src/tracking.c index 852fa229b..1cf226e52 100644 --- a/src/tracking.c +++ b/src/tracking.c @@ -99,6 +99,57 @@ void disableTracking(client *c) { } } +static int stringCheckPrefix(unsigned char *s1, size_t s1_len, unsigned char *s2, size_t s2_len) { + size_t min_length = s1_len < s2_len ? s1_len : s2_len; + return memcmp(s1,s2,min_length) == 0; +} + +/* Check if any of the provided prefixes collide with one another or + * with an existing prefix for the client. A collision is defined as two + * prefixes that will emit an invalidation for the same key. If no prefix + * collision is found, 1 is return, otherwise 0 is returned and the client + * has an error emitted describing the error. */ +int checkPrefixCollisionsOrReply(client *c, robj **prefixes, size_t numprefix) { + for (size_t i = 0; i < numprefix; i++) { + /* Check input list has no overlap with existing prefixes. */ + if (c->client_tracking_prefixes) { + raxIterator ri; + raxStart(&ri,c->client_tracking_prefixes); + raxSeek(&ri,"^",NULL,0); + while(raxNext(&ri)) { + if (stringCheckPrefix(ri.key,ri.key_len, + prefixes[i]->ptr,sdslen(prefixes[i]->ptr))) + { + sds collision = sdsnewlen(ri.key,ri.key_len); + addReplyErrorFormat(c, + "Prefix '%s' overlaps with an existing prefix '%s'. " + "Prefixes for a single client must not overlap.", + (unsigned char *)prefixes[i]->ptr, + (unsigned char *)collision); + sdsfree(collision); + raxStop(&ri); + return 0; + } + } + raxStop(&ri); + } + /* Check input has no overlap with itself. */ + for (size_t j = i + 1; j < numprefix; j++) { + if (stringCheckPrefix(prefixes[i]->ptr,sdslen(prefixes[i]->ptr), + prefixes[j]->ptr,sdslen(prefixes[j]->ptr))) + { + addReplyErrorFormat(c, + "Prefix '%s' overlaps with another provided prefix '%s'. " + "Prefixes for a single client must not overlap.", + (unsigned char *)prefixes[i]->ptr, + (unsigned char *)prefixes[j]->ptr); + return i; + } + } + } + return -1; +} + /* Set the client 'c' to track the prefix 'prefix'. If the client 'c' is * already registered for the specified prefix, no operation is performed. */ void enableBcastTrackingForPrefix(client *c, char *prefix, size_t plen) { diff --git a/tests/unit/tracking.tcl b/tests/unit/tracking.tcl index 8ed2801e7..88cf9dc42 100644 --- a/tests/unit/tracking.tcl +++ b/tests/unit/tracking.tcl @@ -323,7 +323,25 @@ start_server {tags {"tracking"}} { set ping_reply [$rd read] assert {$inv_msg eq {invalidate key1}} assert {$ping_reply eq {PONG}} - } + } + + test {BCAST with prefix collisions throw errors} { + set r [redis_client] + catch {$r CLIENT TRACKING ON BCAST PREFIX FOOBAR PREFIX FOO} output + assert_match {ERR Prefix 'FOOBAR'*'FOO'*} $output + + catch {$r CLIENT TRACKING ON BCAST PREFIX FOO PREFIX FOOBAR} output + assert_match {ERR Prefix 'FOO'*'FOOBAR'*} $output + + $r CLIENT TRACKING ON BCAST PREFIX FOO PREFIX BAR + catch {$r CLIENT TRACKING ON BCAST PREFIX FO} output + assert_match {ERR Prefix 'FO'*'FOO'*} $output + + catch {$r CLIENT TRACKING ON BCAST PREFIX BARB} output + assert_match {ERR Prefix 'BARB'*'BAR'*} $output + + $r CLIENT TRACKING OFF + } test {Tracking gets notification on tracking table key eviction} { $rd_redirection HELLO 2