Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/db.c
Original file line number Diff line number Diff line change
Expand Up @@ -2888,6 +2888,10 @@ int sortGetKeys(struct serverCommand *cmd, robj **argv, int argc, getKeysResult
found_store = 1;
keys[num].pos = i + 1; /* <store-key> */
keys[num].flags = CMD_KEY_OW | CMD_KEY_UPDATE;
/* Skip the destination. It is a key name, so it must never be
* examined as an option: a key that spells one would hide the
* later STORE clause that SORT actually writes to. */
i++;
break;
}
}
Expand Down
31 changes: 31 additions & 0 deletions tests/unit/acl-v2.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,37 @@ start_server {tags {"acl external:skip"}} {
r del v1 mylist
}

test {Test SORT STORE destination that is named like an option} {
# Every decoy destination below is permitted, so the only reason to
# reject a command is the real destination the server writes to.
r ACL setuser test-sort-store on nopass ~allowed:* ~by ~get ~limit ~alpha +@all
r rpush allowed:src c b a

# A dedicated client, because deleting the user below kills it.
set r3 [valkey_client]
$r3 auth test-sort-store nopass

# A destination spelling an option that takes arguments hides the later
# STORE clause that SORT actually uses.
foreach keyword {by get limit} {
assert_equal "User test-sort-store has no permissions to access the 'forbidden:dst' key" \
[r ACL DRYRUN test-sort-store SORT allowed:src ALPHA STORE $keyword STORE forbidden:dst]
assert_error "*NOPERM*key*" {$r3 sort allowed:src ALPHA STORE $keyword STORE forbidden:dst}
assert_equal 0 [r exists forbidden:dst]
}

# A destination spelling STORE reports whatever follows it instead.
assert_equal "User test-sort-store has no permissions to access the 'store' key" \
[r ACL DRYRUN test-sort-store SORT allowed:src STORE store alpha]
assert_error "*NOPERM*key*" {$r3 sort allowed:src STORE store alpha}
assert_equal 0 [r exists store]

# cleanup
$r3 close
r ACL deluser test-sort-store
r del allowed:src
}

test {Test DRYRUN with wrong number of arguments} {
r ACL setuser test-dry-run +@all ~v*

Expand Down
16 changes: 16 additions & 0 deletions tests/unit/sort.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,22 @@ foreach command {SORT SORT_RO} {
r command getkeys sort abc store invalid store stillbad store def
} {abc def}

test "SORT extracts STORE correctly when the destination is named like an option" {
# A destination is a key name, never an option keyword, even when it
# spells one. Parsed as an option it would consume the arguments that
# follow it and hide the later STORE that SORT really writes to.
foreach keyword {by get limit} {
assert_equal {abc def} [r command getkeys sort abc store $keyword store def]
assert_equal [list abc $keyword] [r command getkeys sort abc store $keyword]
}

# A destination spelling STORE would instead be taken for another STORE
# clause, reporting whatever follows it.
assert_equal {abc store} [r command getkeys sort abc store store alpha]
assert_equal {abc STORE} [r command getkeys sort abc store STORE alpha]
assert_equal {abc store} [r command getkeys sort abc store store by w_*]
}

test "SORT DESC" {
assert_equal [lsort -decreasing -integer $result] [r sort tosort DESC]
}
Expand Down
Loading