Commit Graph

1161 Commits

Author SHA1 Message Date
Meir Shpilraien (Spielrein)
d2ad01ab3e
RedisModule_ResetDataset should not clear the functions. (#11268)
As mentioned on docs, `RM_ResetDataset` Performs similar operation to FLUSHALL.
As FLUSHALL do not clean the function, `RM_ResetDataset` should not clean the functions
as well.
2022-10-09 07:42:21 +03:00
aradz44
8e19415343
Added authentication failure and access denied metrics (#11288)
Added authentication failure and access denied metrics
2022-10-07 10:19:34 -07:00
Moti Cohen
210ad2e4db
Improve BLMPOP/BZMPOP/WAIT timeout overflow handling and error messages (#11338)
Refine getTimeoutFromObjectOrReply() out-of-range check.

Timeout is parsed (and verifies out of range) as double and
multiplied by 1000, added mstime() and stored in long-long
which might lead to out-of-range value of long-long.

Co-authored-by: moticless <moticless@github.com>
Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Ozan Tezcan <ozantezcan@gmail.com>
2022-10-06 12:12:05 +03:00
Madelyn Olson
663fbd3459
Stabilize cluster hostnames tests (#11307)
This PR introduces a couple of changes to improve cluster test stability:
1. Increase the cluster node timeout to 3 seconds, which is similar to the
   normal cluster tests, but introduce a new mechanism to increase the ping
   period so that the tests are still fast. This new config is a debug config.
2. Set `cluster-replica-no-failover yes` on a wider array of tests which are
   sensitive to failovers. This was occurring on the ARM CI.
2022-10-03 09:25:16 +03:00
Binbin
a549b78c48
Fix redis-cli cluster add-node race in cli.tcl (#11349)
There is a race condition in the test:
```
*** [err]: redis-cli --cluster add-node with cluster-port in tests/unit/cluster/cli.tcl
Expected '5' to be equal to '4' {assert_equal 5 [CI 0 cluster_known_nodes]} proc ::test)
```

When using cli to add node, there can potentially be a race condition
in which all nodes presenting cluster state o.k even though the added
node did not yet meet all cluster nodes.

This comment and the fix were taken from #11221. Also apply it in several
other similar places.
2022-10-03 09:21:41 +03:00
sundb
f106beebfa
Fix the missing server.dirty increment and redundant signalModifiedKey in serveClientBlockedOnList (#11326)
Mainly fix two minor bug
1. When handle BL*POP/BLMOVE commands with blocked clients, we should increment server.dirty.
2.  `listPopRangeAndReplyWithKey()` in `serveClientBlockedOnList()` should not repeat calling
   `signalModifiedKey()` which has been called when an element was pushed on the list.
   (was skipped in all bpop commands, other than blmpop) 

Other optimization
add `signal` param for `listElementsRemoved` to skip `signalModifiedKey()` to unify all pop operation.

Unifying all pop operations also prepares us for #11303, so that we can avoid having to deal with the
conversion from quicklist to listpack() in various places when the list shrinks.
2022-09-28 21:07:38 +03:00
guybe7
3330ea1864
RM_CreateCommand should not set CMD_KEY_VARIABLE_FLAGS automatically (#11320)
The original idea behind auto-setting the default (first,last,step) spec was to use
the most "open" flags when the user didn't provide any key-spec flags information.

While the above idea is a good approach, it really makes no sense to set
CMD_KEY_VARIABLE_FLAGS if the user didn't provide the getkeys-api flag:
in this case there's not way to retrieve these variable flags, so what's the point?

Internally in redis there was code to ignore this already, so this fix doesn't change
redis's behavior, it only affects the output of COMMAND command.
2022-09-28 14:15:07 +03:00
Ozan Tezcan
18920813a9
Ignore RM_Call deny-oom flag if maxmemory is zero (#11319)
If a command gets an OOM response and then if we set maxmemory to zero
to disable the limit, server.pre_command_oom_state never gets updated
and it stays true. As RM_Call() calls with "respect deny-oom" flag checks
server.pre_command_oom_state, all calls will fail with OOM.

Added server.maxmemory check in RM_Call() to process deny-oom flag
only if maxmemory is configured.
2022-09-26 10:03:45 +03:00
Shaya Potter
6e993a5dfa
Add RM_SetContextUser to support acl validation in RM_Call (and scripts) (#10966)
Adds a number of user management/ACL validaiton/command execution functions to improve a
Redis module's ability to enforce ACLs correctly and easily.

* RM_SetContextUser - sets a RedisModuleUser on the context, which RM_Call will use to both
  validate ACLs (if requested and set) as well as assign to the client so that scripts executed via
  RM_Call will have proper ACL validation.
* RM_SetModuleUserACLString - Enables one to pass an entire ACL string, not just a single OP
  and have it applied to the user
* RM_GetModuleUserACLString - returns a stringified version of the user's ACL (same format as dump
  and list).  Contains an optimization to cache the stringified version until the underlying ACL is modified.
* Slightly re-purpose the "C" flag to RM_Call from just being about ACL check before calling the
  command, to actually running the command with the right user, so that it also affects commands
  inside EVAL scripts. see #11231
2022-09-22 16:29:00 +03:00
Oran Agra
6d21560190
Fix heap overflow vulnerability in XAUTOCLAIM (CVE-2022-35951) (#11301)
Executing an XAUTOCLAIM command on a stream key in a specific state, with a
specially crafted COUNT argument may cause an integer overflow, a subsequent
heap overflow, and potentially lead to remote code execution.
The problem affects Redis versions 7.0.0 or newer.
2022-09-22 11:55:53 +03:00
Binbin
bb6513cbba
ACL default newly created user set USER_FLAG_SANITIZE_PAYLOAD flag (#11279)
Starting from 6.2, after ACL SETUSER user reset, the user
will carry the sanitize-payload flag. It was added in #7807,
and then ACL SETUSER reset is inconsistent with default
newly created user which missing sanitize-payload flag.

Same as `off` and `on` these two bits are mutually exclusive,
the default created user needs to have sanitize-payload flag.
Adds USER_FLAG_SANITIZE_PAYLOAD flag to ACLCreateUser.

Note that the bug don't have any real implications,
since the code in rdb.c (rdbLoadObject) checks for
`USER_FLAG_SANITIZE_PAYLOAD_SKIP`, so the fact that
`USER_FLAG_SANITIZE_PAYLOAD` is missing doesn't really matters.

Added tests to make sure it won't be broken in the future,
and updated the comment in ACLSetUser and redis.conf
2022-09-22 09:13:39 +03:00
Shay Fadida
eedb8b1724
Fix missing sections for INFO ALL with module (#11291)
When using `INFO ALL <section>`, when `section` is a specific module section. 
Redis will not print the additional section(s).

The fix in this case, will search the modules info sections if the user provided additional sections to `ALL`.

Co-authored-by: Oran Agra <oran@redislabs.com>
2022-09-21 08:10:03 +03:00
sundb
13d25dd95e
Fix crash due to delete entry from compress quicklistNode and wrongly split quicklistNode (#11242)
This PR mainly deals with 2 crashes introduced in #9357,
and fix the QUICKLIST-PACKED-THRESHOLD mess in external test mode.

1. Fix crash due to deleting an entry from a compress quicklistNode
   When inserting a large element, we need to create a new quicklistNode first,
   and then delete its previous element, if the node where the deleted element is
   located is compressed, it will cause a crash.
   Now add `dont_compress` to quicklistNode, if we want to use a quicklistNode
   after some operation, we can use this flag like following:

    ```c
    node->dont_compress = 1; /* Prevent to be compressed */
    some_operation(node); /* This operation might try to compress this node */
    some_other_operation(node); /* We can use this node without decompress it */
    node->dont_compress = 0; /* Re-able compression */
    quicklistCompressNode(node);
    ```

   Perhaps in the future, we could just disable the current entry from being
   compressed during the iterator loop, but that would require more work.

2. Fix crash due to wrongly split quicklist
   before #9357, the offset param of _quicklistSplitNode() will not negative.
   For now, when offset is negative, the split extent will be wrong.
   following example:
    ```c
    int orig_start = after ? offset + 1 : 0;
    int orig_extent = after ? -1 : offset;
    int new_start = after ? 0 : offset;
    int new_extent = after ? offset + 1 : -1;
    # offset: -2, after: 1, node->count: 2
    # current wrong range: [-1,-1] [0,-1]
    # correct range: [1,-1] [0, 1]
    ```

   Because only `_quicklistInsert()` splits the quicklistNode and only
   `quicklistInsertAfter()`, `quicklistInsertBefore()` call _quicklistInsert(), 
   so `quicklistReplaceEntry()` and `listTypeInsert()` might occur this crash.
   But the iterator of `listTypeInsert()` is alway from head to tail(iter->offset is
   always positive), so it is not affected.
   The final conclusion is this crash only occur when we insert a large element
   with negative index into a list, that affects `LSET` command and `RM_ListSet`
   module api.
     
3. In external test mode, we need to restore quicklist packed threshold after
   when the end of test.
4. Show `node->count` in quicklistRepr().
5. Add new tcl proc `config_get_set` to support restoring config in tests.
2022-09-19 09:47:52 +03:00
ranshid
c0ce97facc
fix test Migrate the last slot away from a node using redis-cli (#11221)
When using cli to add node, there can potentially be a race condition in
which all nodes presenting cluster state o.k even though the added node
did not yet meet all cluster nodes.
this adds another utility function to wait until all cluster nodes see the same cluster size
2022-09-06 16:54:24 -07:00
Shaya Potter
87e7973c7e
Add a dry run flag to RM_Call execution (#11158)
Add a new "D" flag to RM_Call which runs whatever verification the user requests,
but returns before the actual execution of the command.

It automatically enables returning error messages as CallReply objects to distinguish
success (NULL) from failure (CallReply returned).
2022-09-05 16:19:32 +03:00
Oran Agra
c3b7bde914
fix false valgrind error on new hash test (#11200)
New test fails on valgrind because strtold("+inf") with valgrind returns a non-inf result
same thing is done in incr.tcl.
2022-08-29 10:25:24 +03:00
Shaya Potter
bed6d759bc
Improve cmd_flags for script/functions in RM_Call (#11159)
When RM_Call was used with `M` (reject OOM), `W` (reject writes),
as well as `S` (rejecting stale or write commands in "Script mode"),
it would have only checked the command flags, but not the declared
script flag in case it's a command that runs a script.

Refactoring: extracts out similar code in server.c's processCommand
to be usable in RM_Call as well.
2022-08-28 13:10:10 +03:00
weimeng
8945067544
bugfix:del keys in slot replicate to replica, and trigger other invalidations (#11084)
Bugfix:
with the scenario if we force assigned a slot to other master,
old master will lose the slot ownership, then old master will
call the function delKeysInSlot() to delete all keys which in
the slot. These delete operations should replicate to replicas,
avoid the data divergence issue in master and replicas.

Additionally, in this case, we now call:
* signalModifiedKey (to invalidate WATCH)
* moduleNotifyKeyspaceEvent (key space notification for modules)
* dirty++ (to signal that the persistence file may be outdated)

Co-authored-by: weimeng <weimeng@didiglobal.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
2022-08-28 11:37:26 +03:00
chendianqiang
bc7fe41e58
fix hincrbyfloat not to create a key if the new value is invalid (#11149)
Check the validity of the value before performing the create operation,
prevents new data from being generated even if the request fails to execute.

Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: chendianqiang <chendianqiang@meituan.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
2022-08-28 11:33:41 +03:00
Oran Agra
c789fb0aa7
Fix assertion when a key is lazy expired during cluster key migration (#11176)
Redis 7.0 has #9890 which added an assertion when the propagation queue
was not flushed and we got to beforeSleep.
But it turns out that when processCommands calls getNodeByQuery and
decides to reject the command, it can lead to a key that was lazy
expired and is deleted without later flushing the propagation queue.

This change prevents lazy expiry from deleting the key at this stage
(not as part of a command being processed in `call`)
2022-08-24 19:39:15 +03:00
Binbin
78259826cd
Bump codespell from 2.1.0 to 2.2.1 in /.codespell (#11184)
add a few terms to the white list, and fix a few newly detected typos
2022-08-24 15:07:43 +03:00
Meir Shpilraien (Spielrein)
c1bd61a4a5
Reverts most of the changes of #10969 (#11178)
The PR reverts the changes made on #10969.
The reason for revert was trigger because of occasional test failure
that started after the PR was merged.

The issue is that if there is a lazy expire during the command invocation,
the `del` command is added to the replication stream after the command
placeholder. So the logical order on the primary is:

* Delete the key (lazy expiration)
* Command invocation

But the replication stream gets it the other way around:

* Command invocation (because the command is written into the placeholder)
* Delete the key (lazy expiration)

So if the command write to the key that was just lazy expired we will get
inconsistency between primary and replica.

One solution we considered is to add another lazy expire replication stream
and write all the lazy expire there. Then when replicating, we will replicate the
lazy expire replication stream first. This will solve this specific test failure but
we realize that the issues does not ends here and the more we dig the more
problems we find.One of the example we thought about (that can actually
crashes Redis) is as follow:

* User perform SINTERSTORE
* When Redis tries to fetch the second input key it triggers lazy expire
* The lazy expire trigger a module logic that deletes the first input key
* Now Redis hold the robj of the first input key that was actually freed

We believe we took the wrong approach and we will come up with another
PR that solve the problem differently, for now we revert the changes so we
will not have the tests failure.

Notice that not the entire code was revert, some parts of the PR are changes
that we would like to keep. The changes that **was** reverted are:

* Saving a placeholder for replication at the beginning of the command (`call` function)
* Order of the replication stream on active expire and eviction (we will decide how
  to handle it correctly on follow up PR)
* `Spop` changes are no longer needed (because we reverted the placeholder code)

Changes that **was not** reverted:

* On expire/eviction, wrap the `del` and the notification effect in a multi exec.
* `PropagateNow` function can still accept a special dbid, -1, indicating not to replicate select.
* Keep optimisation for reusing the `alsoPropagate` array instead of allocating it each time.

Tests:

* All tests was kept and only few tests was modify to work correctly with the changes
* Test was added to verify that the revert fixes the issues.
2022-08-24 12:51:36 +03:00
Oran Agra
4faddf18ca Build TLS as a loadable module
* Support BUILD_TLS=module to be loaded as a module via config file or
  command line. e.g. redis-server --loadmodule redis-tls.so
* Updates to redismodule.h to allow it to be used side by side with
  server.h by defining REDISMODULE_CORE_MODULE
* Changes to server.h, redismodule.h and module.c to avoid repeated
  type declarations (gcc 4.8 doesn't like these)
* Add a mechanism for non-ABI neutral modules (ones who include
  server.h) to refuse loading if they detect not being built together with
  redis (release.c)
* Fix wrong signature of RedisModuleDefragFunc, this could break
  compilation of a module, but not the ABI
* Move initialization of listeners in server.c to be after loading
  the modules
* Config TLS after initialization of listeners
* Init cluster after initialization of listeners
* Add TLS module to CI
* Fix a test suite race conditions:
  Now that the listeners are initialized later, it's not sufficient to
  wait for the PID message in the log, we need to wait for the "Server
  Initialized" message.
* Fix issues with moduleconfigs test as a result from start_server
  waiting for "Server Initialized"
* Fix issues with modules/infra test as a result of an additional module
  present

Notes about Sentinel:
Sentinel can't really rely on the tls module, since it uses hiredis to
initiate connections and depends on OpenSSL (won't be able to use any
other connection modules for that), so it was decided that when TLS is
built as a module, sentinel does not support TLS at all.
This means that it keeps using redis_tls_ctx and redis_tls_client_ctx directly.

Example code of config in redis-tls.so(may be use in the future):
RedisModuleString *tls_cfg = NULL;

void tlsInfo(RedisModuleInfoCtx *ctx, int for_crash_report) {
    UNUSED(for_crash_report);
    RedisModule_InfoAddSection(ctx, "");
    RedisModule_InfoAddFieldLongLong(ctx, "var", 42);
}

int tlsCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
{
    if (argc != 2) return RedisModule_WrongArity(ctx);
    return RedisModule_ReplyWithString(ctx, argv[1]);
}

RedisModuleString *getStringConfigCommand(const char *name, void *privdata) {
    REDISMODULE_NOT_USED(name);
    REDISMODULE_NOT_USED(privdata);
    return tls_cfg;
}

int setStringConfigCommand(const char *name, RedisModuleString *new, void *privdata, RedisModuleString **err) {
    REDISMODULE_NOT_USED(name);
    REDISMODULE_NOT_USED(err);
    REDISMODULE_NOT_USED(privdata);
    if (tls_cfg) RedisModule_FreeString(NULL, tls_cfg);
    RedisModule_RetainString(NULL, new);
    tls_cfg = new;
    return REDISMODULE_OK;
}

int RedisModule_OnLoad(void *ctx, RedisModuleString **argv, int argc)
{
    ....
    if (RedisModule_CreateCommand(ctx,"tls",tlsCommand,"",0,0,0) == REDISMODULE_ERR)
        return REDISMODULE_ERR;

    if (RedisModule_RegisterStringConfig(ctx, "cfg", "", REDISMODULE_CONFIG_DEFAULT, getStringConfigCommand, setStringConfigCommand, NULL, NULL) == REDISMODULE_ERR)
        return REDISMODULE_ERR;

    if (RedisModule_LoadConfigs(ctx) == REDISMODULE_ERR) {
        if (tls_cfg) {
            RedisModule_FreeString(ctx, tls_cfg);
            tls_cfg = NULL;
        }
        return REDISMODULE_ERR;
    }
    ...
}

Co-authored-by: zhenwei pi <pizhenwei@bytedance.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
2022-08-23 12:37:56 +03:00
Wen Hui
c3a0253bc8
Add 2 test cases for XDEL and XGROUP CREATE command (#11137)
This PR includes 2 missed test cases of XDEL and XGROUP CREATE command

1. one test case: XDEL delete multiply id once
2. 3 test cases:  XGROUP CREATE has ENTRIESREAD parameter,
   which equal 0 (special positive number), 3 and negative value.


Co-authored-by: Ubuntu <lucas.guang.yang1@huawei.com>
Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Binbin <binloveplay1314@qq.com>
2022-08-21 07:52:57 +03:00
Binbin
3a16ad30b7
Fix CLUSTERDOWN issue in cluster reshard unblock test (#11139)
change the cluster-node-timeout from 1 to 1000
2022-08-18 09:18:18 -07:00
Binbin
fc3956e8f4
Fix memory leak in moduleFreeCommand (#11147)
Currently, we call zfree(cmd->args), but the argument array
needs to be freed recursively (there might be sub-args).
Also fixed memory leaks on cmd->tips and cmd->history.

Fixes #11145
2022-08-18 12:36:01 +03:00
Meir Shpilraien (Spielrein)
508a138885
Fix replication inconsistency on modules that uses key space notifications (#10969)
Fix replication inconsistency on modules that uses key space notifications.

### The Problem

In general, key space notifications are invoked after the command logic was
executed (this is not always the case, we will discuss later about specific
command that do not follow this rules). For example, the `set x 1` will trigger
a `set` notification that will be invoked after the `set` logic was performed, so
if the notification logic will try to fetch `x`, it will see the new data that was written.
Consider the scenario on which the notification logic performs some write
commands. for example, the notification logic increase some counter,
`incr x{counter}`, indicating how many times `x` was changed.
The logical order by which the logic was executed is has follow:

```
set x 1
incr x{counter}
```

The issue is that the `set x 1` command is added to the replication buffer
at the end of the command invocation (specifically after the key space
notification logic was invoked and performed the `incr` command).
The replication/aof sees the commands in the wrong order:

```
incr x{counter}
set x 1
```

In this specific example the order is less important.
But if, for example, the notification would have deleted `x` then we would
end up with primary-replica inconsistency.

### The Solution

Put the command that cause the notification in its rightful place. In the
above example, the `set x 1` command logic was executed before the
notification logic, so it should be added to the replication buffer before
the commands that is invoked by the notification logic. To achieve this,
without a major code refactoring, we save a placeholder in the replication
buffer, when finishing invoking the command logic we check if the command
need to be replicated, and if it does, we use the placeholder to add it to the
replication buffer instead of appending it to the end.

To be efficient and not allocating memory on each command to save the
placeholder, the replication buffer array was modified to reuse memory
(instead of allocating it each time we want to replicate commands).
Also, to avoid saving a placeholder when not needed, we do it only for
WRITE or MAY_REPLICATE commands.

#### Additional Fixes

* Expire and Eviction notifications:
  * Expire/Eviction logical order was to first perform the Expire/Eviction
    and then the notification logic. The replication buffer got this in the
    other way around (first notification effect and then the `del` command).
    The PR fixes this issue.
  * The notification effect and the `del` command was not wrap with
    `multi-exec` (if needed). The PR also fix this issue.
* SPOP command:
  * On spop, the `spop` notification was fired before the command logic
    was executed. The change in this PR would have cause the replication
    order to be change (first `spop` command and then notification `logic`)
    although the logical order is first the notification logic and then the
    `spop` logic. The right fix would have been to move the notification to
    be fired after the command was executed (like all the other commands),
    but this can be considered a breaking change. To overcome this, the PR
    keeps the current behavior and changes the `spop` code to keep the right
    logical order when pushing commands to the replication buffer. Another PR
    will follow to fix the SPOP properly and match it to the other command (we
    split it to 2 separate PR's so it will be easy to cherry-pick this PR to 7.0 if
    we chose to).

#### Unhanded Known Limitations

* key miss event:
  * On key miss event, if a module performed some write command on the
    event (using `RM_Call`), the `dirty` counter would increase and the read
    command that cause the key miss event would be replicated to the replication
    and aof. This problem can also happened on a write command that open
    some keys but eventually decides not to perform any action. We decided
    not to handle this problem on this PR because the solution is complex
    and will cause additional risks in case we will want to cherry-pick this PR.
    We should decide if we want to handle it in future PR's. For now, modules
    writers is advice not to perform any write commands on key miss event.

#### Testing

* We already have tests to cover cases where a notification is invoking write
  commands that are also added to the replication buffer, the tests was modified
  to verify that the replica gets the command in the correct logical order.
* Test was added to verify that `spop` behavior was kept unchanged.
* Test was added to verify key miss event behave as expected.
* Test was added to verify the changes do not break lazy expiration.

#### Additional Changes

* `propagateNow` function can accept a special dbid, -1, indicating not
  to replicate `select`. We use this to replicate `multi/exec` on `propagatePendingCommands`
  function. The side effect of this change is that now the `select` command
  will appear inside the `multi/exec` block on the replication stream (instead of
  outside of the `multi/exec` block). Tests was modified to match this new behavior.
2022-08-18 10:16:32 +03:00
Valentino Geron
6a9cc20d94
Tests: Add missing key declaration in scripts (#11134)
Make sure the script calls in the tests declare the keys they intend to use.
Do that with minimal changes to existing lines (so many scripts still have a hard coded key names)

Co-authored-by: Valentino Geron <valentino@redis.com>
2022-08-16 22:04:22 +03:00
guybe7
1189680edd
Rename offset and xsetid tags (#11103)
There's really no point in having dedicated flags to test these features
(why shouldn't all commands/features get their own tag?)
2022-08-14 18:25:01 +03:00
sundb
8aad2ac352
Add missing lua_pop in luaGetFromRegistry (#11097)
This pr mainly has the following four changes:

1. Add missing lua_pop in `luaGetFromRegistry`.
    This bug affects `redis.register_function`, where `luaGetFromRegistry` in
    `luaRegisterFunction` will return null when we call `redis.register_function` nested.
    .e.g
    ```
    FUNCTION LOAD "#!lua name=mylib \n local lib=redis \n lib.register_function('f2', function(keys, args) lib.register_function('f1', function () end) end)"
    fcall f2 0
    ````
    But since we exit when luaGetFromRegistry returns null, it does not cause the stack to grow indefinitely.

3. When getting `REGISTRY_RUN_CTX_NAME` from the registry, use `serverAssert`
    instead of error return. Since none of these lua functions are registered at the time
    of function load, scriptRunCtx will never be NULL.
4. Add `serverAssert` for `luaLdbLineHook`, `luaEngineLoadHook`.
5. Remove `luaGetFromRegistry` from `redis_math_random` and
    `redis_math_randomseed`, it looks like they are redundant.
2022-08-14 11:50:18 +03:00
DarrenJiang13
44859a41ee
fix the client type in trackingInvalidateKey() (#11052)
Fix bug with scripts ignoring client tracking NOLOOP and
send an invalidation message anyway.
2022-08-10 11:58:54 +03:00
Valentino Geron
3270f2d54e
Tests: improve skip tags around resp3 (#11090)
some skip tags were missing on some tests
avoid using HELLO if denytags has resp3 (target server may not support it)

Co-authored-by: Valentino Geron <valentino@redis.com>
2022-08-07 16:32:31 +03:00
Huang Zhw
ec5034a2e3
acl: bitfield with get and set|incrby can be executed with readonly permission (#11086)
`bitfield` with `get` may not be readonly.

```
127.0.0.1:6384> acl setuser hello on nopass %R~* +@all
OK
127.0.0.1:6384> auth hello 1
OK
127.0.0.1:6384> bitfield hello set i8 0 1
(error) NOPERM this user has no permissions to access one of the keys used as arguments
127.0.0.1:6384> bitfield hello set i8 0 1 get i8 0
1) (integer) 0
2) (integer) 1
```

Co-authored-by: Oran Agra <oran@redislabs.com>
2022-08-07 09:21:19 +03:00
Valentino Geron
dcafee55a5
Fix acl tests to support --singledb flag (#11077)
* some of the tests don't clean the key the use
* marked tests with `{singledb:skip}` if they use SELECT

Co-authored-by: Valentino Geron <valentino@redis.com>
2022-08-03 12:11:32 +03:00
Wen Hui
beb9746a9f
Fix function load error message (#10964)
Update error messages for function load
2022-08-02 18:19:53 -07:00
Huang Zhw
61451b02cb
tracking pending invalidation message of flushdb sent by (#11068)
trackingHandlePendingKeyInvalidations should use proto.
2022-07-31 16:14:39 +03:00
Huang Zhw
6f0a27e38e
When client tracking is on, invalidation message of flushdb in a (#11038)
When FLUSHDB / FLUSHALL / SWAPDB is inside MULTI / EXEC, the
client side tracking invalidation message was interleaved with transaction response.
2022-07-26 13:28:37 +03:00
Meir Shpilraien (Spielrein)
020e046b42
Fix #11030, use lua_rawget to avoid triggering metatables and crash. (#11032)
Fix #11030, use lua_rawget to avoid triggering metatables.

#11030 shows how return `_G` from the Lua script (either function or eval), cause the
Lua interpreter to Panic and the Redis processes to exit with error code 1.
Though return `_G` only panic on Redis 7 and 6.2.7, the underline issue exists on older
versions as well (6.0 and 6.2). The underline issue is returning a table with a metatable
such that the metatable raises an error.

The following example demonstrate the issue:
```
127.0.0.1:6379> eval "local a = {}; setmetatable(a,{__index=function() foo() end}) return a" 0
Error: Server closed the connection
```
```
PANIC: unprotected error in call to Lua API (user_script:1: Script attempted to access nonexistent global variable 'foo')
```

The Lua panic happened because when returning the result to the client, Redis needs to
introspect the returning table and transform the table into a resp. In order to scan the table,
Redis uses `lua_gettable` api which might trigger the metatable (if exists) and might raise an error.
This code is not running inside `pcall` (Lua protected call), so raising an error causes the
Lua to panic and exit. Notice that this is not a crash, its a Lua panic that exit with error code 1.

Returning `_G` panics on Redis 7 and 6.2.7 because on those versions `_G` has a metatable
that raises error when trying to fetch a none existing key.

### Solution

Instead of using `lua_gettable` that might raise error and cause the issue, use `lua_rawget`
that simply return the value from the table without triggering any metatable logic.
This is promised not to raise and error.

The downside of this solution is that it might be considered as breaking change, if someone
rely on metatable in the returned value. An alternative solution is to wrap this entire logic
with `pcall` (Lua protected call), this alternative require a much bigger refactoring.

### Back Porting

The same fix will work on older versions as well (6.2, 6.0). Notice that on those version,
the issue can cause Redis to crash if inside the metatable logic there is an attempt to accesses
Redis (`redis.call`). On 7.0, there is not crash and the `redis.call` is executed as if it was done
from inside the script itself.

### Tests

Tests was added the verify the fix
2022-07-26 10:33:50 +03:00
Viktor Söderqvist
5032de50f2
Gossip forgotten nodes on CLUSTER FORGET (#10869)
Gossip the cluster node blacklist in ping and pong messages.
This means that CLUSTER FORGET doesn't need to be sent to all nodes in a cluster.
It can be sent to one or more nodes and then be propagated to the rest of them.

For each blacklisted node, its node id and its remaining blacklist TTL is gossiped in a
cluster bus ping extension (introduced in #9530).
2022-07-26 10:28:13 +03:00
Binbin
5ce64ab010
Fix timing issue in cluster test (#11008)
A timing issue like this was reported in freebsd daily CI:
```
*** [err]: Sanity test push cmd after resharding in tests/unit/cluster/cli.tcl
Expected 'CLUSTERDOWN The cluster is down' to match '*MOVED*'
```

We additionally wait for each node to reach a consensus on the cluster
state in wait_for_condition to avoid the cluster down error.

The fix just like #10495, quoting madolson's comment:
Cluster check just verifies the the config state is self-consistent,
waiting for cluster_state to be okay is an independent check that all
the nodes actually believe each other are healthy.

At the same time i noticed that unit/moduleapi/cluster.tcl has an exact
same test, may have the same problem, also modified it.
2022-07-18 20:35:13 -07:00
Oran Agra
2825b6057b
Fix heap overflow corruption in XAUTOCLAIM (CVE-2022-31144) (#11002)
The temporary array for deleted entries reply of XAUTOCLAIM was
insufficient, but also in fact the COUNT argument should be used to
control the size of the reply, so instead of terminating the loop by
only counting the claimed entries, we'll count deleted entries as well.

Fix #10968
Addresses CVE-2022-31144
2022-07-18 11:36:19 +03:00
Madelyn Olson
3abdec9969
Fix cluster hostnames test causing failover while running valgrind (#10991)
In the newly added cluster hostnames test, the primary is failing over during the reboot
for valgrind so we are validating the wrong node. This change just sets the replica to
prevent taking over, which seems to fix the test.

We could have also set the timeout higher, but it slows down the test.
2022-07-17 09:57:34 +03:00
Madelyn Olson
8a4e3bcd8d
Cluster test improvements (#10920)
* Restructured testing to allow running cluster tests easily as part of the normal testing
2022-07-12 10:41:29 -07:00
Binbin
35e8ae3eb5
Add cluster-port support to redis-cli --cluster (#10344)
In #9389, we add a new `cluster-port` config and make cluster bus port configurable,
and currently redis-cli --cluster create/add-node doesn't support with a configurable `cluster-port` instance.
Because redis-cli uses the old way (port + 10000) to send the `CLUSTER MEET` command.

Now we add this support on redis-cli `--cluster`, note we don't need to explicitly pass in the
`cluster-port` parameter, we can get the real `cluster-port` of the node in `clusterManagerNodeLoadInfo`,
so the `--cluster create` and `--cluster add-node` interfaces have not changed.

We will use the `cluster-port` when we are doing `CLUSTER MEET`, also note that `CLUSTER MEET` bus-port
parameter was added in 4.0, so if the bus_port (the one in redis-cli) is 0, or equal (port + 10000),
we just call `CLUSTER MEET` with 2 arguments, using the old form.

Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com>
2022-07-11 11:23:31 +03:00
Harkrishn Patro
0ab885a685
Account sharded pubsub channels memory consumption (#10925)
Account sharded pubsub channels memory consumption in client memory usage
computation to accurately evict client based on the set threshold for `maxmemory-clients`.
2022-07-04 09:18:57 +03:00
Yossi Gottlieb
69d5576832
Fix TLS tests on newer tcl-tls/OpenSSL. (#10910)
Before this commit, TLS tests on Ubuntu 22.04 would fail as dropped
connections result with an ECONNABORTED error thrown instead of an empty
read.
2022-07-03 13:34:14 +03:00
jonnyomerredis
35c2ee8716
Add sharded pubsub keychannel count to client info (#10895)
When calling CLIENT INFO/LIST, and in various debug prints, Redis is printing
the number of pubsub channels / patterns the client is subscribed to.
With the addition of sharded pubsub, it would be useful to print the number of
keychannels the client is subscribed to as well.
2022-06-28 10:11:17 +03:00
RinChanNOW!
2854637385
Support conversion between RedisModuleString and unsigned long long (#10889)
Since the ranges of `unsigned long long` and `long long` are different, we cannot read an
`unsigned long long` integer from a `RedisModuleString` by `RedisModule_StringToLongLong` . 

So I added two new Redis Module APIs to support the conversion between these two types:
* `RedisModule_StringToULongLong`
* `RedisModule_CreateStringFromULongLong`

Signed-off-by: RinChanNOWWW <hzy427@gmail.com>
2022-06-26 15:02:52 +03:00
Binbin
d443e312ad
redis-server command line arguments allow passing config name and value in the same arg (#10866)
This commit has two topics.

## Passing config name and value in the same arg
In #10660 (Redis 7.0.1), when we supported the config values that can start with `--` prefix (one of the two topics of that PR),
we broke another pattern: `redis-server redis.config "name value"`, passing both config name
and it's value in the same arg, see #10865

This wasn't a intended change (i.e we didn't realize this pattern used to work).
Although this is a wrong usage, we still like to fix it.

Now we support something like:
```
src/redis-server redis.conf "--maxmemory '700mb'" "--maxmemory-policy volatile-lru" --proc-title-template --my--title--template --loglevel verbose
```

## Changes around --save
Also in this PR, we undo the breaking change we made in #10660 on purpose.
1. `redis-server redis.conf --save --loglevel verbose` (missing `save` argument before anotehr argument).
    In 7.0.1, it was throwing an wrong arg error.
    Now it will work and reset the save, similar to how it used to be in 7.0.0 and 6.2.x.
3. `redis-server redis.conf --loglevel verbose --save` (missing `save` argument as last argument).
    In 6.2, it did not reset the save, which was a bug (inconsistent with the previous bullet).
    Now we will make it work and reset the save as well (a bug fix).
2022-06-26 14:36:39 +03:00
Viktor Söderqvist
6272ca609e
Add RM_SetClientNameById and RM_GetClientNameById (#10839)
Adding Module APIs to let the module read and set the client name of an arbitrary connection.
2022-06-26 14:34:59 +03:00