…or hincrby commands (#10479)

Fixed a bug that used the `hincrbyfloat` or `hincrby` commands to make the field or value exceed the
`hash_max_listpack_value` but did not change the object encoding of the hash structure.

Add a length check for field and value, check the length of value first, if the length of value does not
exceed `hash_max_listpack_value` then check the length of field.

If the length of field or value is too long, it will reduce the efficiency of listpack, and the object encoding
will become hashtable after AOF restart, so this is also to keep the same before and after AOF restart.

Fixes in command argument in json files
* Fixes BITFIELD's syntax ("sub-commands" can be repeated, and OVERFLOW is only valid for SET and INCR)
* Improves readability of SET (reordered)
* Fixes GEOSEARCH and GEOSEARCH_RO syntices (use `oneof` for mutually exclusive group instead of `optional`)
* Fixes MIGRATE syntax (use `oneof` for mutually exclusive group instead of `optional`)
* Fixes MODULE LOADEX syntax (the `CONFIG` token should be repeated too when using multiple configs)

other:
* make generate-command-help.rb accept a path to commands.json, or read it from stdin (e.g. `generate-commands-json.py | generate-command-help.rb -`)

The bug was when using REDISMODULE_YIELD_FLAG_CLIENTS.
in that case we would have only set the CLIENTS type flag in
server.busy_module_yield_flags and then clear that flag when exiting
RM_Yield, so we would never call unblockPostponedClients when the
context is destroyed.

This didn't really have any actual implication, which is why the tests
couldn't (and still can't) find that since the bug only happens when
using CLIENT, but in this case we won't have any clients to un-postpone
i.e. clients will get rejected with BUSY error, rather than being
postponed.

Unrelated:
* Adding tests for nested contexts, just in case.
* Avoid nested RM_Yield calls

…0550)

The `auto-aof-rewrite-percentage` config defines at what growth percentage
an automatic AOF rewrite is triggered.
This normally works OK since the size of the AOF file at the end of a rewrite
is stored in `server.aof_rewrite_base_size`.
However, on startup, redis used to store the entire size of the AOF file into that
variable, resulting in a wrong automatic AOF rewrite trigger (could have been
triggered much later than desired).
This issue would only affect the first AOFRW after startup, after that future AOFRW
would have been triggered correctly.
This bug existed in all previous versions of Redis.

This PR unifies the meaning of `server.aof_rewrite_base_size`, which only represents
the size of BASE AOF.
Note that after an AOFRW this size includes the size of the incremental file (all the
commands that executed during rewrite), so that auto-aof-rewrite-percentage is the
ratio from the size of the AOF after rewrite.
However, on startup, it is complicated to know that size, and we compromised on
taking just the size of the base file, this means that the first rewrite after startup can
happen a little bit too soon.

Co-authored-by: Oran Agra <[email protected]>
Co-authored-by: yoav-steinberg <[email protected]>

* improve malloc efficiency: reduce call times of zrealloc

Co-authored-by: Madelyn Olson <[email protected]>

copy paste error..

Co-authored-by: Oran Agra <[email protected]>

Add field to COMMAND DOCS response to denote the name of the module
that added that command.
COMMAND LIST can filter by module, but if you get the full commands list,
you may still wanna know which command belongs to which module.
The alternative would be to do MODULE LIST, and then multiple calls to COMMAND LIST

Similarly to LCS, some modules would want to try to allocate memory, and
fail gracefully if the allocation fails

Change `RM_OpenKey` to return `RedisModuleKey*` instead of `void*`.
Which is the input type of other APIs that take the value from RM_OpenKey.

Durability of database is a big and old topic, in this regard Redis use AOF to
support it, and `appendfsync=alwasys` policy is the most strict level, guarantee
all data is both written and synced on disk before reply success to client.

But there are some cases have been overlooked, and could lead to durability broken.

1. The most clear one is about threaded-io mode
   we should also set client's write handler with `ae_barrier` in
   `handleClientsWithPendingWritesUsingThreads`, or the write handler would be
   called after read handler in the next event loop, it means the write command result
   could be replied to client before flush to AOF.
2. About blocked client (mostly by module)
   in `beforeSleep()`, `handleClientsBlockedOnKeys()` should be called before
   `flushAppendOnlyFile()`, in case the unblocked clients modify data without persistence
   but send reply.
3. When handling `ProcessingEventsWhileBlocked`
   normally it takes place when lua/function/module timeout, and we give a chance to users
   to kill the slow operation, but we should call `flushAppendOnlyFile()` before
   `handleClientsWithPendingWrites()`, in case the other clients in the last event loop get
   acknowledge before data persistence.
   for a instance:
   ```
   in the same event loop
   client A executes set foo bar
   client B executes eval "for var=1,10000000,1 do end" 0
   ```
   after the script timeout, client A will get `OK` but lose data after restart (kill redis when
   timeout) if we don't flush the write command to AOF.
4. A more complex case about `ProcessingEventsWhileBlocked`
   it is lua timeout in transaction, for example
   `MULTI; set foo bar; eval "for var=1,10000000,1 do end" 0; EXEC`, then client will get set
   command's result before the whole transaction done, that breaks atomicity too.
   fortunately, it's already fixed by #5428 (although it's not the original purpose just a side
   effect : )), but module timeout should be fixed too.

case 1, 2, 3 are fixed in this commit, the module issue in case 4 needs a followup PR.

* Extending the use of hashTypeGetValue.

Functions hashTypeExists, hashTypeGetValueLength and addHashFieldToReply
have a similar pattern on calling hashTypeGetFromHashTable or
hashTypeGetFromZipList depending on the underlying data structure. What
does functions are duing is exactly what hashTypeGetValue does. Those
functions were changed to use existing function hashTypeGetValue making
the code more consistent.

Co-authored-by: Madelyn Olson <[email protected]>

Allow specifying an ACL log reason, which is shown in the log. Right now it always shows "unknown", which is a little bit cryptic. This is a breaking change, but this API was added as part of 7 so it seems ok to stabilize it still.

Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 2 to 3.
- [Release notes](https://github.com/actions/upload-artifact/releases)
- [Commits](actions/upload-artifact@v2...v3)

---
updated-dependencies:
- dependency-name: actions/upload-artifact
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Apparently, some modules can afford deprecating command arguments
(something that was never done in Redis, AFAIK), so in order to represent
this piece of information, we added the `deprecated_since` field to redisCommandArg
(in symmetry to the already existing `since` field).

This commit adds `const char *deprecated_since` to `RedisModuleCommandArg`,
which is technically a breaking change, but since 7.0 was not released yet, we decided to let it slide

Add an optional keyspace event when new keys are added to the db.

This is useful for applications where clients need to be aware of the redis keyspace.
Such an application can SCAN once at startup and then listen for "new" events (plus
others associated with DEL, RENAME, etc).

…#10574)

we can observe that when adding to a stream without ID there is a duplicate work
on sds creation/freeing/sdslen that costs ~11% of the CPU cycles. 

This PR avoids it by not freeing the sds after the first reply. 
The expected reduction in CPU cycles is around 9-10% 

Additionally, we now pre-allocate the sds to the right size, to avoid realloc.
this brought another ~10% improvement

Co-authored-by: Oran Agra <[email protected]>

This PR fix the following minor errors before Redis 7 release:

ZRANGEBYLEX command in deprecated in 6.2.0, and could be replaced by ZRANGE with the
BYLEX argument, but in the document, the words is written incorrect in " by ZRANGE with the BYSCORE argument"

Fix function zpopmaxCommand incorrect comment

The comments of function zmpopCommand and bzmpopCommand are not consistent with document description, fix them

Co-authored-by: Ubuntu <[email protected]>

we had a panic in streamLastValidID when the stream metadata
said it's not empty, but the rax is empty.

…#10575)

By the convention of errors, there is supposed to be a space between the code and the name.
While looking at some lua stuff I noticed that interpreter errors were not adding the space,
so some clients will try to map the detailed error message into the error.

We have tests that hit this condition, but they were just checking that the string "starts" with ERR.
I updated some other tests with similar incorrect string checking. This isn't complete though, as
there are other ways we check for ERR I didn't fix.

Produces some fun output like:
```
# Errorstats
errorstat_ERR:count=1
errorstat_ERRuser_script_1_:count=1
```

Add APIs to allow modules to compute the memory consumption of opaque objects owned by redis.
Without these, the mem_usage callbacks of module data types are useless in many cases.

Other changes:
Fix streamRadixTreeMemoryUsage to include the size of the rax structure itself

From #9166, we call several times of prepareReplicasToWrite when propagating
one write command to replication stream (once per argument, same as we do for
normal clients), that is not necessary. Now we only call it one time per command
at the begin of feeding replication stream.

This results in reducing CPU consumption and slightly better performance,
specifically when there are many replicas.

The tests verify that loading a binary payload to the Lua interpreter raises an error.
The Lua code modification was done here: fdf9d45
which force the Lau interpreter to always use the text parser.

since PUBLISH and SPUBLISH use different dictionaries for channels and clients,
and we already have an API for PUBLISH, it only makes sense to have one for SPUBLISH

Add test coverage and unifying some test infrastructure.

…ck) (#10486)

When the score doesn't have fractional part, and can be stored as an integer,
we use the integer capabilities of listpack to store it, rather than convert it to string.
This already existed before this PR (lpInsert dose that conversion implicitly).

But to do that, we would have first converted the score from double to string (calling `d2string`),
then pass the string to `lpAppend` which identified it as being an integer and convert it back to an int.
Now, instead of converting it to a string, we store it using lpAppendInteger`.

Unrelated:
---
* Fix the double2ll range check (negative and positive ranges, and also the comparison operands
  were slightly off. but also, the range could be made much larger, see comment).
* Unify the double to string conversion code in rdb.c with the one in util.c
* Small optimization in lpStringToInt64, don't attempt to convert strings that are obviously too long.

Benchmark;
---
Up to 20% improvement in certain tight loops doing zzlInsert with large integers.
(if listpack is pre-allocated to avoid realloc, and insertion is sorted from largest to smaller)

There is a implicit conversion warning in clang:
```
util.c:574:23: error: implicit conversion from 'long long' to 'double'
changes value from -4611686018427387903 to -4611686018427387904
[-Werror,-Wimplicit-const-int-float-conversion]
    if (d < -LLONG_MAX/2 || d > LLONG_MAX/2)
```

introduced in #10486

Co-authored-by: sundb <[email protected]>

…10573)

RM_Yield was missing a call to protectClient to prevent redis from
processing future commands of the yielding client.

Adding tests that fail without this fix.

This would be complicated to solve since nested calls to RM_Call used to
replace the current_client variable with the module temp client.

It looks like it's no longer necessary to do that, since it was added
back in #9890 to solve two issues, both already gone:
1. call to CONFIG SET maxmemory could trigger a module hook calling
   RM_Call. although this specific issue is gone, arguably other hooks
   like keyspace notification, can do the same.
2. an assertion in lookupKey that checks the current command of the
   current client, introduced in #9572 and removed in #10248

fix typo. `LCS[j+(blen+1)*j]` -> `LCS[j+(blen+1)*i]`

…dReplyWithListpack" (#10623)

* Fix timing issue in slowlog redact test

This test failed once in my daily CI (test-sanitizer-address (clang))
```
*** [err]: SLOWLOG - Some commands can redact sensitive fields in tests/unit/slowlog.tcl
Expected 'migrate 127.0.0.1 25649 key 9 5000 AUTH2 (redacted) (redacted)' to match '* key 9 5000 AUTH (redacted)' (context: type eval line 12 cmd {assert_match {* key 9 5000 AUTH (redacted)} [lindex [lindex [r slowlog get] 1] 3]} proc ::test)
```

The reason is that with slowlog-log-slower-than 10000,
slowlog get will have a chance to exceed 10ms.

Change slowlog-log-slower-than from 10000 to -1, disable it.
Also handles a same potentially problematic test above.
This is actually the same timing issue as #10432.

But also avoid repeated calls to `SLOWLOG GET`

If was first added in #9890 to solve the problem of
CONFIG SET maxmemory causing eviction inside MULTI/EXEC,
but that problem is already fixed (CONFIG SET doesn't evict
directly, it just schedules a later eviction)

Keep that condition may hide bugs (i.e. performEvictions
should always expect to have an empty server.also_propagate)

This avoids random memory spikes and enables --large-memory tests to run
on moderately sized systems.

This case is interesting because it originates from cron,
rather than from another command.

The idea came from looking at #9890 and #10573, and I was wondering if RM_Call
would work properly when `server.current_client == NULL`

…ror responses (#10612)

1. Disk error and slave count checks didn't flag the transactions or counted correctly in command stats (regression from #10372  , 7.0 RC3)
2. RM_Call will reply the same way Redis does, in case of non-exisitng command or arity error
3. RM_WrongArtiy will consider the full command name
4. Use lowercase 'u' in "unknonw subcommand" (to align with "unknown command")

Followup work of #10127

…0633)

fix an unclear comment quicklist container formats to quicklist node container formats
Add a comment to 'zi' in quicklistIter (Where it first appeared)
Why do I add a comment to zi:
Because it is not a variable name with a clear meaning, and its name seems to be from the deprecated ziplist.

minor cleanup for recent changes.

A change in #10612 introduced a regression.
when replying with garbage bytes to the caller, we must make sure it
doesn't include any newlines.

in the past it called rejectCommandFormat which did that trick.
but now it calls rejectCommandSds, which doesn't, so we need to make sure
to sanitize the sds.

…h # (#10615)

Adds the `allow-cross-slot-keys` flag to Eval scripts and Functions to allow
scripts to access keys from multiple slots.
The default behavior is now that they are not allowed to do that (unlike before).
This is a breaking change for 7.0 release candidates (to be part of 7.0.0), but
not for previous redis releases since EVAL without shebang isn't doing this check.

Note that the check is done on both the keys declared by the EVAL / FCALL command
arguments, and also the ones used by the script when making a `redis.call`.

A note about the implementation, there seems to have been some confusion
about allowing access to non local keys. I thought I missed something in our
wider conversation, but Redis scripts do block access to non-local keys.
So the issue was just about cross slots being accessed.

…tion errors (#10504)

* Till now, replicas that were unable to persist, would still execute the commands
  they got from the master, now they'll panic by default, and we add a new
  `replica-ignore-disk-errors` config to change that.
* Till now, when a command failed on a replica or AOF-loading, it only logged a
  warning and a stat, we add a new `propagation-error-behavior` config to allow
  panicking in that state (may become the default one day)

Note that commands that fail on the replica can either indicate a bug that could
cause data inconsistency between the replica and the master, or they could be
in some cases (specifically in previous versions), a result of a command (e.g. EVAL)
that failed on the master, but still had to be propagated to fail on the replica as well.

Missing a typeof, we will get errors like this:
- multiple definition of `replicationErrorBehavior'
- ld: error: duplicate symbol: replicationErrorBehavior

Introduced in #10504

The SHUTDOWN command has various flags to change it's default behavior,
but in some cases establishing a connection to redis is complicated and it's easier
for the management software to use signals. however, so far the signals could only
trigger the default shutdown behavior.
Here we introduce the option to control shutdown arguments for SIGTERM and SIGINT.

New config options:
`shutdown-on-sigint [nosave | save] [now] [force]` 
`shutdown-on-sigterm [nosave | save] [now] [force]`

Implementation:
Support MULTI_ARG_CONFIG on createEnumConfig to support multiple enums to be applied as bit flags.

Co-authored-by: Oran Agra <[email protected]>

…manifest only when AOFRW is done. (#10616)

Changes:

- When AOF is enabled **after** startup, the data accumulated during `AOF_WAIT_REWRITE`
  will only be stored in a temp INCR AOF file. Only after the first AOFRW is successful, we will
  add it to manifest file.
  Before this fix, the manifest referred to the temp file which could cause a restart during that
  time to load it without it's base.
- Add `aof_rewrites_consecutive_failures` info field for  aofrw limiting implementation.

Now we can guarantee that these behaviors of MP-AOF are the same as before (past redis releases):
- When AOF is enabled after startup, the data accumulated during `AOF_WAIT_REWRITE` will only
  be stored in a visible place. Only after the first AOFRW is successful, we will add it to manifest file.
- When disable AOF, we did not delete the AOF file in the past so there's no need to change that
  behavior now (yet).
- When toggling AOF off and then on (could be as part of a full-sync), a crash or restart before the
  first rewrite is completed, would result with the previous version being loaded (might not be right thing,
  but that's what we always had).

Enables registration of an enum config that'll let the user pass multiple keywords that
will be combined with `|` as flags into the integer config value.

```
    const char *enum_vals[] = {"none", "one", "two", "three"};
    const int int_vals[] = {0, 1, 2, 4};

    if (RedisModule_RegisterEnumConfig(ctx, "flags", 3, REDISMODULE_CONFIG_DEFAULT | REDISMODULE_CONFIG_BITFLAGS, enum_vals, int_vals, 4, getFlagsConfigCommand, setFlagsConfigCommand, NULL, NULL) == REDISMODULE_ERR) {
        return REDISMODULE_ERR;
    }
```
doing:
`config set moduleconfigs.flags "two three"` will result in 6 being passed to`setFlagsConfigCommand`.

The new feature can be turned off and on using the new `lua_enablereadonlytable` Lua API.

Today, Redis wrap the user Lua code with a Lua function.
For example, assuming the user code is:

```
return redis.call('ping')
```

The actual code that would have sent to the Lua interpreter was:

```
f_b3a02c833904802db9c34a3cf1292eee3246df3c() return redis.call('ping') end
```

The wraped code would have been saved on the global dictionary with the
following name: `f_<script sha>` (in our example `f_b3a02c833904802db9c34a3cf1292eee3246df3c`).

This approach allows one user to easily override the implementation a another user code, example:

```
f_b3a02c833904802db9c34a3cf1292eee3246df3c = function() return 'hacked' end
```

Running the above code will cause `evalsha b3a02c833904802db9c34a3cf1292eee3246df3c 0` to return
hacked although it should have returned `pong`.

Another disadventage is that Redis basically runs code on the loading (compiling) phase without been
aware of it. User can do code injection like this:

```
return 1 end <run code on compling phase> function() return 1
```

The wraped code will look like this and the entire `<run code on compling phase>` block will run outside
of eval or evalsha context:

```
f_<sha>() return 1 end <run code on compling phase> function() return 1 end
```

Use the new `lua_enablereadonlytable` Lua API to protect the global tables of
both evals scripts and functions. For eval scripts, the implemetation is easy,
We simply call `lua_enablereadonlytable` on the global table to turn it into
a readonly table.

On functions its more complecated, we want to be able to switch globals between
load run and function run. To achieve this, we create a new empty table that
acts as the globals table for function, we control the actual globals using metatable
manipulation. Notice that even if the user gets a pointer to the original tables, all
the tables are set to be readonly (using `lua_enablereadonlytable` Lua API) so he can
not change them. The following inlustration better explain the solution:

```
Global table {} <- global table metatable {.__index = __real_globals__}
```

The `__real_globals__` is set depends on the run context (function load or function call).

Why this solution is needed and its not enough to simply switch globals?
When we run in the context of function load and create our functions, our function gets
the current globals that was set when they were created. Replacing the globals after
the creation will not effect them. This is why this trick it mandatory.

…hite list.

The white list is done by setting a metatable on the global table before initializing
any library. The metatable set the `__newindex` field to a function that check
the white list before adding the field to the table. Fields which is not on the
white list are simply ignored.

After initialization phase is done we protect the global table and each table
that might be reachable from the global table. For each table we also protect
the table metatable if exists.

Followup fix for #10616

# Lua readonly tables
The PR adds support for readonly tables on Lua to prevent security vulnerabilities:
* (CVE-2022-24736) An attacker attempting to load a specially crafted Lua script
  can cause NULL pointer dereference which will result with a crash of the
  redis-server process. This issue affects all versions of Redis.
* (CVE-2022-24735) By exploiting weaknesses in the Lua script execution
  environment, an attacker with access to Redis can inject Lua code that will
  execute with the (potentially higher) privileges of another Redis user.

The PR is spitted into 4 commits.

### Change Lua to support readonly tables

This PR modifies the Lua interpreter code to support a new flag on tables. The new flag indicating that the table is readonly and any attempt to perform any writes on such a table will result in an error. The new feature can be turned off and on using the new `lua_enablereadonlytable` Lua API. The new API can be used **only** from C code. Changes to support this feature was taken from https://luau-lang.org/

### Change eval script to set user code on Lua registry

Today, Redis wrap the user Lua code with a Lua function. For example, assuming the user code is:

```
return redis.call('ping')
```

The actual code that would have sent to the Lua interpreter was:

```
f_b3a02c833904802db9c34a3cf1292eee3246df3c() return redis.call('ping') end
```

The warped code would have been saved on the global dictionary with the following name: `f_<script sha>` (in our example `f_b3a02c833904802db9c34a3cf1292eee3246df3c`). This approach allows one user to easily override the implementation of another user code, example:

```
f_b3a02c833904802db9c34a3cf1292eee3246df3c = function() return 'hacked' end
```

Running the above code will cause `evalsha b3a02c833904802db9c34a3cf1292eee3246df3c 0` to return `hacked` although it should have returned `pong`. Another disadvantage is that Redis basically runs code on the loading (compiling) phase without been aware of it. User can do code injection like this:

```
return 1 end <run code on compling phase> function() return 1
```

The warped code will look like this and the entire `<run code on compiling phase>` block will run outside of eval or evalsha context:

```
f_<sha>() return 1 end <run code on compling phase> function() return 1 end
```

The commits puts the user code on a special Lua table called the registry. This table is not accessible to the user so it can not be manipulated by him. Also there is no longer a need to warp the user code so there is no risk in code injection which will cause running code in the wrong context.

### Use `lua_enablereadonlytable` to protect global tables on eval and function

The commit uses the new `lua_enablereadonlytable` Lua API to protect the global tables of both evals scripts and functions. For eval scripts, the implementation is easy, We simply call `lua_enablereadonlytable` on the global table to turn it into a readonly table.

On functions its more complected, we want to be able to switch globals between load run and function run. To achieve this, we create a new empty table that acts as the globals table for function, we control the actual globals using metatable manipulations. Notice that even if the user gets a pointer to the original tables, all the tables are set to be readonly (using `lua_enablereadonlytable` Lua API) so he can not change them. The following better explains the solution:

```
Global table {} <- global table metatable {.__index = __real_globals__}
```

The `__real_globals__` is depends on the run context (function load or function call).

Why is this solution needed and its not enough to simply switch globals? When we run in the context of function load and create our functions, our function gets the current globals that was set when they were created. Replacing the globals after the creation will not effect them. This is why this trick it mandatory.

### Protect the rest of the global API and add an allowed list to the provided API

The allowed list is done by setting a metatable on the global table before initialising any library. The metatable set the `__newindex` field to a function that check the allowed list before adding the field to the table. Fields which is not on the
allowed list are simply ignored.

After initialisation phase is done we protect the global table and each table that might be reachable from the global table. For each table we also protect the table metatable if exists.

### Performance

Performance tests was done on a private computer and its only purpose is to show that this fix is not causing any performance regression.

case 1: `return redis.call('ping')`
case 2: `for i=1,10000000 do redis.call('ping') end`

|                             | Unstable eval | Unstable function | lua_readonly_tables eval | lua_readonly_tables function |
|-----------------------------|---------------|-------------------|--------------------------|------------------------------|
| case1 ops/sec               | 235904.70     | 236406.62         | 232180.16               | 230574.14                   |
| case1 avg latency ms        | 0.175         | 0.164             | 0.178                    | 0.149                        |
| case2 total time in seconds | 3.373         | 3.444s            | 3.268                   | 3.278                        |

### Breaking changes

* `print` function was removed from Lua because it can potentially cause the Redis processes to get stuck (if no one reads from stdout). Users should use redis.log. An alternative is to override the `print` implementation and print the message to the log file.

All the work by @MeirShpilraien, i'm just publishing it.