gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions 8 Packages Projects Releases Wiki Activity

Decline unsubscribe related command in non-subscribed mode (#759)

Browse Source 
Now, when clients run the unsubscribe, sunsubscribe and punsubscribe
commands in the non-subscribed mode, it returns 0.
Indeed this is a bug, we should not allow client run these kind of
commands here.

Thus, this PR fixes this bug, but it is a break change for existing
clients

---------

Signed-off-by: hwware <wen.hui.ware@gmail.com>
This commit is contained in:
Wen Hui 2024-08-22 11:21:33 -04:00 committed by GitHub
parent 8d9b8c9d3d
commit 959dd3485b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 16 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/server.c
View File

@ -4037,6 +4037,12 @@ int processCommand(client *c) {
return C_OK; return C_OK;
} }
/* Not allow several UNSUBSCRIBE commands executed under non-pubsub mode */
if (!c->flag.pubsub && (c->cmd->proc == unsubscribeCommand || c->cmd->proc == sunsubscribeCommand ||
c->cmd->proc == punsubscribeCommand)) {
rejectCommandFormat(c, "-NOSUB '%s' command executed not in subscribed mode", c->cmd->fullname);
return C_OK;
}
/* Only allow commands with flag "t", such as INFO, REPLICAOF and so on, /* Only allow commands with flag "t", such as INFO, REPLICAOF and so on,
* when replica-serve-stale-data is no and we are a replica with a broken * when replica-serve-stale-data is no and we are a replica with a broken
* link with primary. */ * link with primary. */

3
tests/unit/info.tcl
View File

@ -424,7 +424,8 @@ start_server {tags {"info" "external:skip"}} {
set info [r info clients] set info [r info clients]
assert_equal [getInfoProperty $info pubsub_clients] {1} assert_equal [getInfoProperty $info pubsub_clients] {1}
# non-pubsub clients should not be involved # non-pubsub clients should not be involved
assert_equal {0} [unsubscribe $rd2 {non-exist-chan}] catch {unsubscribe $rd2 {non-exist-chan}} e
assert_match {*NOSUB*} $e
set info [r info clients] set info [r info clients]
assert_equal [getInfoProperty $info pubsub_clients] {1} assert_equal [getInfoProperty $info pubsub_clients] {1}
# close all clients # close all clients

26
tests/unit/pubsub.tcl
View File

@ -109,10 +109,12 @@ start_server {tags {"pubsub network"}} {
$rd1 close $rd1 close
} }
test "UNSUBSCRIBE from non-subscribed channels" { test "UNSUBSCRIBE and PUNSUBSCRIBE from non-subscribed channels" {
set rd1 [valkey_deferring_client] set rd1 [valkey_deferring_client]
assert_equal {0 0 0} [unsubscribe $rd1 {foo bar quux}] foreach command {unsubscribe punsubscribe} {
catch {$command $rd1 {foo bar quux}} e
assert_match {*NOSUB*} $e
}
# clean up clients # clean up clients
$rd1 close $rd1 close
} }
@ -202,14 +204,6 @@ start_server {tags {"pubsub network"}} {
$rd close $rd close
} {0} {resp3} } {0} {resp3}
test "PUNSUBSCRIBE from non-subscribed channels" {
set rd1 [valkey_deferring_client]
assert_equal {0 0 0} [punsubscribe $rd1 {foo.* bar.* quux.*}]
# clean up clients
$rd1 close
}
test "NUMSUB returns numbers, not strings (#1561)" { test "NUMSUB returns numbers, not strings (#1561)" {
r pubsub numsub abc def r pubsub numsub abc def
} {abc 0 def 0} } {abc 0 def 0}
@ -247,16 +241,6 @@ start_server {tags {"pubsub network"}} {
$rd1 close $rd1 close
} }
test "PUNSUBSCRIBE and UNSUBSCRIBE should always reply" {
# Make sure we are not subscribed to any channel at all.
r punsubscribe
r unsubscribe
# Now check if the commands still reply correctly.
set reply1 [r punsubscribe]
set reply2 [r unsubscribe]
concat $reply1 $reply2
} {punsubscribe {} 0 unsubscribe {} 0}
### Keyspace events notification tests ### Keyspace events notification tests
test "Keyspace notifications: we receive keyspace notifications" { test "Keyspace notifications: we receive keyspace notifications" {

5
tests/unit/pubsubshard.tcl
View File

@ -74,9 +74,8 @@ start_server {tags {"pubsubshard external:skip"}} {
test "SUNSUBSCRIBE from non-subscribed channels" { test "SUNSUBSCRIBE from non-subscribed channels" {
set rd1 [valkey_deferring_client] set rd1 [valkey_deferring_client]
assert_equal {0} [sunsubscribe $rd1 {foo}] catch {sunsubscribe $rd1 {foo}} e
assert_equal {0} [sunsubscribe $rd1 {bar}] assert_match {*NOSUB*} $e
assert_equal {0} [sunsubscribe $rd1 {quux}]
# clean up clients # clean up clients
$rd1 close $rd1 close