From 7e3cd1a17a9542dd8feeec67a6f3238a2137ec85 Mon Sep 17 00:00:00 2001 From: Hristo Staykov Date: Fri, 22 May 2026 15:38:32 +0300 Subject: [PATCH] Add comprehensive tests for pointer-key pubsub_subscriptions refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cover the remaining test plan gaps for the user-pointer keyed pubsub_subscriptions dict, exercising provenance tracking, lifecycle cleanup, infrastructure integration, and counter correctness. tests/unit/acl.tcl (11 tests): - Core vulnerability regression for channel, pattern, and shard subscriptions (subscribe as user A, re-auth as B, revoke A → killed) - ACL DELUSER on a provenance user kills the subscribed client - Duplicate subscribe after re-auth honours first-user-wins semantics - Many user switches on one connection; revoking any provenance user kills the client - RESET clears all per-user subscription state - UNSUBSCRIBE/PUNSUBSCRIBE with no args clears entries across all provenance users - PUBSUB NUMSUB and NUMPAT stay accurate through subscribe, re-auth, and revocation tests/unit/memefficiency.tcl (1 test): - Active defrag with multi-user subscriptions: two users subscribe to 25k channels each, memory is fragmented and defragged, then every channel is verified to still deliver messages — exercises the defrag path that no longer defragments pointer keys in the outer dict tests/cluster/tests/25-pubsubshard-slot-migration.tcl (1 test): - Shard slot migration with a multi-user subscription: subscribe as a custom ACL user, re-auth as default, migrate the slot, and verify sunsubscribe is delivered correctly --- .../tests/25-pubsubshard-slot-migration.tcl | 38 +++ tests/unit/acl.tcl | 262 ++++++++++++++++++ tests/unit/memefficiency.tcl | 92 ++++++ 3 files changed, 392 insertions(+) diff --git a/tests/cluster/tests/25-pubsubshard-slot-migration.tcl b/tests/cluster/tests/25-pubsubshard-slot-migration.tcl index fd774a8d7..c7d196a36 100644 --- a/tests/cluster/tests/25-pubsubshard-slot-migration.tcl +++ b/tests/cluster/tests/25-pubsubshard-slot-migration.tcl @@ -189,6 +189,44 @@ test "Delete a slot, verify sunsubscribe message" { $subscribeclient close } +test "Migrate a slot with multi-user shard subscriptions, verify sunsubscribe is delivered correctly" { + set channelname ch5 + set slot [$cluster cluster keyslot $channelname] + array set nodefrom [$cluster masternode_for_slot $slot] + array set nodeto [$cluster masternode_notfor_slot $slot] + + $nodefrom(link) ACL SETUSER slotuser on nopass ~* &* +@all + + set subscribeclient [redis_deferring_client_by_addr $nodefrom(host) $nodefrom(port)] + $subscribeclient deferred 1 + + $subscribeclient hello 3 AUTH slotuser slotuser + $subscribeclient read + + $subscribeclient ssubscribe $channelname + $subscribeclient read + + $subscribeclient auth default "" + $subscribeclient read + + $nodefrom(link) spublish $channelname pre-migrate + assert_equal "smessage $channelname pre-migrate" [$subscribeclient read] + + assert_equal {OK} [$nodefrom(link) cluster setslot $slot migrating $nodeto(id)] + assert_equal {OK} [$nodeto(link) cluster setslot $slot importing $nodefrom(id)] + assert_equal {OK} [$nodefrom(link) cluster setslot $slot node $nodeto(id)] + + set msg [$subscribeclient read] + assert {"sunsubscribe" eq [lindex $msg 0]} + assert {$channelname eq [lindex $msg 1]} + assert {"0" eq [lindex $msg 2]} + + assert_equal {OK} [$nodeto(link) cluster setslot $slot node $nodeto(id)] + + $subscribeclient close + $nodefrom(link) ACL DELUSER slotuser +} + test "Reset cluster, verify sunsubscribe message" { set channelname ch4 set slot [$cluster cluster keyslot $channelname] diff --git a/tests/unit/acl.tcl b/tests/unit/acl.tcl index 52a1198aa..123d59653 100644 --- a/tests/unit/acl.tcl +++ b/tests/unit/acl.tcl @@ -343,6 +343,268 @@ start_server {tags {"acl external:skip"}} { $rd close } {0} + # ─── Provenance: subscription revocation across re-auth ─── + + test {Provenance: channel subscription is killed when originating user's permissions are revoked} { + r ACL SETUSER provuser on nopass ~* &* +@all + set rd [redis_deferring_client] + $rd HELLO 3 AUTH provuser provuser + $rd read + $rd SUBSCRIBE secret + assert_match {subscribe secret 1} [$rd read] + # Re-auth as default — subscription stays under provuser + $rd AUTH default "" + $rd read + $rd CLIENT SETNAME prov-channel + $rd read + # Revoke provuser's channel access + r ACL SETUSER provuser resetchannels + # Client must be killed — provenance entry is under provuser + catch {$rd read} e + assert_match {*I/O error*} $e + assert_no_match {*prov-channel*} [r CLIENT LIST] + $rd close + r ACL DELUSER provuser + } + + test {Provenance: pattern subscription is killed when originating user's permissions are revoked} { + r ACL SETUSER provuser on nopass ~* &* +@all + set rd [redis_deferring_client] + $rd HELLO 3 AUTH provuser provuser + $rd read + $rd PSUBSCRIBE secret:* + assert_match {psubscribe secret:* 1} [$rd read] + $rd AUTH default "" + $rd read + $rd CLIENT SETNAME prov-pattern + $rd read + r ACL SETUSER provuser resetchannels + catch {$rd read} e + assert_match {*I/O error*} $e + assert_no_match {*prov-pattern*} [r CLIENT LIST] + $rd close + r ACL DELUSER provuser + } + + test {Provenance: shard channel subscription is killed when originating user's permissions are revoked} { + r ACL SETUSER provuser on nopass ~* &* +@all + set rd [redis_deferring_client] + $rd HELLO 3 AUTH provuser provuser + $rd read + $rd SSUBSCRIBE secret + assert_match {ssubscribe secret 1} [$rd read] + $rd AUTH default "" + $rd read + $rd CLIENT SETNAME prov-shard + $rd read + r ACL SETUSER provuser resetchannels + catch {$rd read} e + assert_match {*I/O error*} $e + assert_no_match {*prov-shard*} [r CLIENT LIST] + $rd close + r ACL DELUSER provuser + } + + # ─── Provenance: ACL DELUSER on originating user ─── + + test {Provenance: ACL DELUSER kills client that holds subscriptions from deleted user} { + r ACL SETUSER provuser on nopass ~* &* +@all + set rd [redis_deferring_client] + $rd HELLO 3 AUTH provuser provuser + $rd read + $rd SUBSCRIBE chan1 + $rd read + # Re-auth as default, subscription remains under provuser + $rd AUTH default "" + $rd read + $rd CLIENT SETNAME prov-deluser + $rd read + r ACL DELUSER provuser + catch {$rd read} e + assert_match {*I/O error*} $e + assert_no_match {*prov-deluser*} [r CLIENT LIST] + $rd close + } {0} + + # ─── Provenance: duplicate subscribe after re-auth (first user wins) ─── + + test {Provenance: duplicate subscribe after re-auth attributes to first user} { + r ACL SETUSER provuser on nopass ~* &* +@all + set rd [redis_deferring_client] + $rd HELLO 3 AUTH provuser provuser + $rd read + $rd SUBSCRIBE chan1 + assert_match {subscribe chan1 1} [$rd read] + # Re-auth and subscribe to the same channel — should be a no-op + $rd AUTH default "" + $rd read + $rd SUBSCRIBE chan1 + assert_match {subscribe chan1 1} [$rd read] + $rd CLIENT SETNAME prov-dup + $rd read + # Revoke provuser (the originating user) — must kill + r ACL SETUSER provuser resetchannels + catch {$rd read} e + assert_match {*I/O error*} $e + assert_no_match {*prov-dup*} [r CLIENT LIST] + $rd close + r ACL DELUSER provuser + } + + # ─── Provenance: many user switches on one connection ─── + + test {Provenance: many user switches with subscriptions, revoking one kills client} { + r ACL SETUSER user1 on nopass ~* &* +@all + r ACL SETUSER user2 on nopass ~* &* +@all + r ACL SETUSER user3 on nopass ~* &* +@all + set rd [redis_deferring_client] + $rd HELLO 3 AUTH user1 user1 + $rd read + $rd SUBSCRIBE ch1 + $rd read + $rd AUTH user2 user2 + $rd read + $rd SUBSCRIBE ch2 + $rd read + $rd AUTH user3 user3 + $rd read + $rd SUBSCRIBE ch3 + $rd read + $rd CLIENT SETNAME multi-user + $rd read + # Verify all subscriptions deliver + r PUBLISH ch1 msg1 + assert_match {*msg1*} [$rd read] + r PUBLISH ch2 msg2 + assert_match {*msg2*} [$rd read] + r PUBLISH ch3 msg3 + assert_match {*msg3*} [$rd read] + # Revoke user2 — client must be killed + r ACL SETUSER user2 resetchannels + catch {$rd read} e + assert_match {*I/O error*} $e + assert_no_match {*multi-user*} [r CLIENT LIST] + $rd close + r ACL DELUSER user1 user2 user3 + } + + # ─── Lifecycle: RESET after multi-user subscribe ─── + + test {Provenance: RESET clears all per-user subscription state} { + r ACL SETUSER provuser on nopass ~* &* +@all + set rd [redis_deferring_client] + $rd HELLO 3 AUTH provuser provuser + $rd read + $rd SUBSCRIBE ch1 + $rd read + $rd AUTH default "" + $rd read + $rd SUBSCRIBE ch2 + $rd read + # RESET should clear everything + $rd RESET + $rd read + # Client should be out of pubsub mode — normal commands should work + $rd SET testkey testval + assert_match {OK} [$rd read] + $rd DEL testkey + $rd read + # PUBSUB NUMSUB should show zero for both channels + assert_equal {ch1 0 ch2 0} [r PUBSUB NUMSUB ch1 ch2] + $rd close + r ACL DELUSER provuser + } + + # ─── Lifecycle: unsubscribe-all after multi-user subscribe ─── + + test {Provenance: UNSUBSCRIBE with no args clears all per-user channel entries} { + r ACL SETUSER provuser on nopass ~* &* +@all + set rd [redis_deferring_client] + $rd HELLO 3 AUTH provuser provuser + $rd read + $rd SUBSCRIBE ch1 ch2 + $rd read ; # subscribe ch1 + $rd read ; # subscribe ch2 + $rd AUTH default "" + $rd read + $rd SUBSCRIBE ch3 + $rd read + # Unsubscribe all channels (no args) + $rd UNSUBSCRIBE + $rd read ; # unsubscribe ch1 + $rd read ; # unsubscribe ch2 + $rd read ; # unsubscribe ch3 + assert_equal {ch1 0 ch2 0 ch3 0} [r PUBSUB NUMSUB ch1 ch2 ch3] + $rd close + r ACL DELUSER provuser + } + + test {Provenance: PUNSUBSCRIBE with no args clears all per-user pattern entries} { + r ACL SETUSER provuser on nopass ~* &* +@all + set rd [redis_deferring_client] + $rd HELLO 3 AUTH provuser provuser + $rd read + $rd PSUBSCRIBE foo:* + $rd read + $rd AUTH default "" + $rd read + $rd PSUBSCRIBE bar:* + $rd read + $rd PUNSUBSCRIBE + $rd read ; # punsubscribe foo:* + $rd read ; # punsubscribe bar:* + assert_equal {0} [r PUBSUB NUMPAT] + $rd close + r ACL DELUSER provuser + } + + # ─── PUBSUB NUMSUB/NUMPAT correctness after provenance operations ─── + + test {Provenance: PUBSUB NUMSUB stays correct through subscribe, re-auth, and revocation} { + r ACL SETUSER provuser on nopass ~* &* +@all + set rd [redis_deferring_client] + $rd HELLO 3 AUTH provuser provuser + $rd read + $rd SUBSCRIBE ch1 + $rd read + assert_equal {ch1 1} [r PUBSUB NUMSUB ch1] + $rd AUTH default "" + $rd read + $rd SUBSCRIBE ch2 + $rd read + assert_equal {ch1 1 ch2 1} [r PUBSUB NUMSUB ch1 ch2] + # Revoke provuser — client killed, all subscriptions gone + r ACL SETUSER provuser resetchannels + catch {$rd read} e + assert_match {*I/O error*} $e + assert_equal {ch1 0 ch2 0} [r PUBSUB NUMSUB ch1 ch2] + $rd close + r ACL DELUSER provuser + } + + test {Provenance: PUBSUB NUMPAT stays correct through subscribe, re-auth, and revocation} { + r ACL SETUSER provuser on nopass ~* &* +@all + set rd [redis_deferring_client] + $rd HELLO 3 AUTH provuser provuser + $rd read + $rd PSUBSCRIBE foo:* + $rd read + assert_equal {1} [r PUBSUB NUMPAT] + $rd AUTH default "" + $rd read + $rd PSUBSCRIBE bar:* + $rd read + assert_equal {2} [r PUBSUB NUMPAT] + r ACL SETUSER provuser resetchannels + catch {$rd read} e + assert_match {*I/O error*} $e + assert_equal {0} [r PUBSUB NUMPAT] + $rd close + r ACL DELUSER provuser + } + + # ─── End of provenance tests ─── + test {blocked command gets rejected when reprocessed after permission change} { r auth default "" r config resetstat diff --git a/tests/unit/memefficiency.tcl b/tests/unit/memefficiency.tcl index f488ca85f..cb5e8162c 100644 --- a/tests/unit/memefficiency.tcl +++ b/tests/unit/memefficiency.tcl @@ -582,6 +582,98 @@ run_solo {defrag} { $rd_pubsub close } + test "Active defrag pubsub multi-user subscriptions: $type" { + r flushdb + r config set hz 100 + r config set activedefrag no + wait_for_defrag_stop 500 100 + r config resetstat + r config set active-defrag-threshold-lower 5 + r config set active-defrag-cycle-min 65 + r config set active-defrag-cycle-max 75 + r config set active-defrag-ignore-bytes 1500kb + r config set maxmemory 0 + + r ACL SETUSER defraguser on nopass ~* &* +@all + + set n 25000 + set dummy_channel "[string repeat x 400]" + set rd_default [redis_deferring_client] + set rd_extra [redis_deferring_client] + $rd_extra AUTH defraguser defraguser + $rd_extra read + + set rd_filler [redis_deferring_client] + for {set j 0} {$j < $n} {incr j} { + set channel_name "$dummy_channel[format "%06d" $j]" + if {$j % 2 == 0} { + $rd_default subscribe $channel_name + $rd_default read + } else { + $rd_extra subscribe $channel_name + $rd_extra read + } + $rd_filler setbit k$j [expr {[string length $channel_name] * 8}] 1 + $rd_filler read + } + + after 120 + assert_lessthan [s allocator_frag_ratio] 1.05 + + set batch_size 1000 + for {set j 0} {$j < $n} {incr j} { + $rd_filler del k$j + if {($j + 1) % $batch_size == 0} { + for {set i 0} {$i < $batch_size} {incr i} { + $rd_filler read + } + } + } + set remaining [expr {$n % $batch_size}] + for {set j 0} {$j < $remaining} {incr j} { $rd_filler read } + if {$type eq "cluster"} { + $rd_filler config resetstat + $rd_filler read + } + $rd_filler close + after 120 + assert_morethan [s allocator_frag_ratio] 1.35 + + catch {r config set activedefrag yes} e + if {[r config get activedefrag] eq "activedefrag yes"} { + wait_for_condition 50 100 { + [s total_active_defrag_time] ne 0 + } else { + after 120 + puts [r info memory] + puts [r info stats] + puts [r memory malloc-stats] + fail "defrag not started." + } + + wait_for_defrag_stop 500 100 1.05 + + after 120 + } + + for {set j 0} {$j < $n} {incr j} { + set channel "$dummy_channel[format "%06d" $j]" + r publish $channel "hello" + if {$j % 2 == 0} { + assert_equal "message $channel hello" [$rd_default read] + $rd_default unsubscribe $channel + $rd_default read + } else { + assert_equal "message $channel hello" [$rd_extra read] + $rd_extra unsubscribe $channel + $rd_extra read + } + } + $rd_default close + $rd_extra close + r ACL DELUSER defraguser + } + test "Active defrag IDMP streams: $type" { r flushdb r config set hz 100