1. 16 Jan, 2023 1 commit
  2. 27 Apr, 2022 3 commits
    • filipe oliveira's avatar
      Optimize deferred replies to use shared objects instead of sprintf (#10334) · 35e697de
      filipe oliveira authored
      
      
      Avoid sprintf/ll2string on setDeferredAggregateLen()/addReplyLongLongWithPrefix() when we can used shared objects.
      In some pipelined workloads this achieves about 10% improvement.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit b857928b)
      35e697de
    • Oran Agra's avatar
      Fix and improve module error reply statistics (#10278) · d6a8e64e
      Oran Agra authored
      This PR handles several aspects
      1. Calls to RM_ReplyWithError from thread safe contexts don't violate thread safety.
      2. Errors returning from RM_Call to the module aren't counted in the statistics (they
        might be handled silently by the module)
      3. When a module propagates a reply it got from RM_Call to it's client, then the error
        statistics are counted.
      
      This is done by:
      1. When appending an error reply to the output buffer, we avoid updating the global
        error statistics, instead we cache that error in a deferred list in the client struct.
      2. When creating a RedisModuleCallReply object, the deferred error list is moved from
        the client into that object.
      3. when a module calls RM_ReplyWithCallReply we copy the deferred replies to the dest
        client (if that's a real client, then that's when the error statistics are updated to the server)
      
      Note about RM_ReplyWithCallReply: if the original reply had an array with errors, and the module
      replied with just a portion of the original reply, and not the entire reply, the errors are currently not
      propagated and the errors stats will not get propagated.
      
      Fix #10180
      
      (cherry picked from commit b099889a)
      d6a8e64e
    • Huang Zhw's avatar
      Make tracking invalidation messages always after command's reply (#9422) · 0fb96d55
      Huang Zhw authored
      Tracking invalidation messages were sometimes sent in inconsistent order,
      before the command's reply rather than after.
      In addition to that, they were sometimes embedded inside other commands
      responses, like MULTI-EXEC and MGET.
      
      (cherry picked from commit fd135f3e)
      0fb96d55
  3. 04 Oct, 2021 5 commits
    • DarrenJiang13's avatar
      [BUGFIX] Add some missed error statistics (#9328) · 1ed0f049
      DarrenJiang13 authored
      add error counting for some missed behaviors.
      
      (cherry picked from commit 43eb0ce3)
      1ed0f049
    • Oran Agra's avatar
      Improvements to corrupt payload sanitization (#9321) · 4b04ca0b
      Oran Agra authored
      
      
      Recently we found two issues in the fuzzer tester: #9302 #9285
      After fixing them, more problems surfaced and this PR (as well as #9297) aims to fix them.
      
      Here's a list of the fixes
      - Prevent an overflow when allocating a dict hashtable
      - Prevent OOM when attempting to allocate a huge string
      - Prevent a few invalid accesses in listpack
      - Improve sanitization of listpack first entry
      - Validate integrity of stream consumer groups PEL
      - Validate integrity of stream listpack entry IDs
      - Validate ziplist tail followed by extra data which start with 0xff
      Co-authored-by: default avatarsundb <sundbcn@gmail.com>
      (cherry picked from commit 0c90370e)
      4b04ca0b
    • menwen's avatar
      Add latency monitor sample when key is deleted via lazy expire (#9317) · e34f06ae
      menwen authored
      Fix that there is no sample latency after the key expires via expireIfNeeded().
      Some refactoring for shared code.
      
      (cherry picked from commit ca559819)
      e34f06ae
    • Oran Agra's avatar
      Fix ziplist and listpack overflows and truncations (CVE-2021-32627, CVE-2021-32628) · 2775a352
      Oran Agra authored
      - fix possible heap corruption in ziplist and listpack resulting by trying to
        allocate more than the maximum size of 4GB.
      - prevent ziplist (hash and zset) from reaching size of above 1GB, will be
        converted to HT encoding, that's not a useful size.
      - prevent listpack (stream) from reaching size of above 1GB.
      - XADD will start a new listpack if the new record may cause the previous
        listpack to grow over 1GB.
      - XADD will respond with an error if a single stream record is over 1GB
      - List type (ziplist in quicklist) was truncating strings that were over 4GB,
        now it'll respond with an error.
      2775a352
    • Oran Agra's avatar
      Prevent unauthenticated client from easily consuming lots of memory (CVE-2021-32675) · 757f8f77
      Oran Agra authored
      This change sets a low limit for multibulk and bulk length in the
      protocol for unauthenticated connections, so that they can't easily
      cause redis to allocate massive amounts of memory by sending just a few
      characters on the network.
      The new limits are 10 arguments of 16kb each (instead of 1m of 512mb)
      757f8f77
  4. 21 Jul, 2021 4 commits
    • Huang Zhw's avatar
      On 32 bit platform, the bit position of GETBIT/SETBIT/BITFIELD/BITCOUNT,BITPOS... · 835d15b5
      Huang Zhw authored
      On 32 bit platform, the bit position of GETBIT/SETBIT/BITFIELD/BITCOUNT,BITPOS may overflow (see CVE-2021-32761) (#9191)
      
      GETBIT, SETBIT may access wrong address because of wrap.
      BITCOUNT and BITPOS may return wrapped results.
      BITFIELD may access the wrong address but also allocate insufficient memory and segfault (see CVE-2021-32761).
      
      This commit uses `uint64_t` or `long long` instead of `size_t`.
      related https://github.com/redis/redis/pull/8096
      
      At 32bit platform:
      > setbit bit 4294967295 1
      (integer) 0
      > config set proto-max-bulk-len 536870913
      OK
      > append bit "\xFF"
      (integer) 536870913
      > getbit bit 4294967296
      (integer) 0
      
      When the bit index is larger than 4294967295, size_t can't hold bit index. In the past,  `proto-max-bulk-len` is limit to 536870912, so there is no problem.
      
      After this commit, bit position is stored in `uint64_t` or `long long`. So when `proto-max-bulk-len > 536870912`, 32bit platforms can still be correct.
      
      For 64bit platform, this problem still exists. The major reason is bit pos 8 t...
      835d15b5
    • Yossi Gottlieb's avatar
      Fix CLIENT UNBLOCK crashing modules. (#9167) · 79fa5618
      Yossi Gottlieb authored
      Modules that use background threads with thread safe contexts are likely
      to use RM_BlockClient() without a timeout function, because they do not
      set up a timeout.
      
      Before this commit, `CLIENT UNBLOCK` would result with a crash as the
      `NULL` timeout callback is called. Beyond just crashing, this is also
      logically wrong as it may throw the module into an unexpected client
      state.
      
      This commits makes `CLIENT UNBLOCK` on such clients behave the same as
      any other client that is not in a blocked state and therefore cannot be
      unblocked.
      
      (cherry picked from commit aa139e2f)
      79fa5618
    • Oran Agra's avatar
      Test infra, handle RESP3 attributes and big-numbers and bools (#9235) · 6cd84b64
      Oran Agra authored
      - promote the code in DEBUG PROTOCOL to addReplyBigNum
      - DEBUG PROTOCOL ATTRIB skips the attribute when client is RESP2
      - networking.c addReply for push and attributes generate assertion when
        called on a RESP2 client, anything else would produce a broken
        protocol that clients can't handle.
      
      (cherry picked from commit 6a5bac30)
      6cd84b64
    • perryitay's avatar
      Fail EXEC command in case a watched key is expired (#9194) · 3f4f9b63
      perryitay authored
      
      
      There are two issues fixed in this commit: 
      1. we want to fail the EXEC command in case there is a watched key that's logically
         expired but not yet deleted by active expire or lazy expire.
      2. we saw that currently cache time is update in every `call()` (including nested calls),
         this time is being also being use for the isKeyExpired comparison, we want to update
         the cache time only in the first call (execCommand)
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      (cherry picked from commit ac8b1df8)
      3f4f9b63
  5. 01 Jun, 2021 2 commits
  6. 03 May, 2021 2 commits
  7. 19 Apr, 2021 1 commit
    • Hanna Fadida's avatar
      Modules: adding a module type for key space notification (#8759) · 53a4d6c3
      Hanna Fadida authored
      Adding a new type mask ​for key space notification, REDISMODULE_NOTIFY_MODULE, to enable unique notifications from commands on REDISMODULE_KEYTYPE_MODULE type keys (which is currently unsupported).
      
      Modules can subscribe to a module key keyspace notification by RM_SubscribeToKeyspaceEvents,
      and clients by notify-keyspace-events of redis.conf or via the CONFIG SET, with the characters 'd' or 'A' 
      (REDISMODULE_NOTIFY_MODULE type mask is part of the '**A**ll' notation for key space notifications).
      
      Refactor: move some pubsub test infra from pubsub.tcl to util.tcl to be re-used by other tests.
      53a4d6c3
  8. 15 Apr, 2021 1 commit
    • guybe7's avatar
      Add a timeout mechanism for replicas stuck in fullsync (#8762) · d63d0260
      guybe7 authored
      Starting redis 6.0 (part of the TLS feature), diskless master uses pipe from the fork
      child so that the parent is the one sending data to the replicas.
      This mechanism has an issue in which a hung replica will cause the master to wait
      for it to read the data sent to it forever, thus preventing the fork child from terminating
      and preventing the creations of any other forks.
      
      This PR adds a timeout mechanism, much like the ACK-based timeout,
      we disconnect replicas that aren't reading the RDB file fast enough.
      d63d0260
  9. 11 Apr, 2021 1 commit
    • Wang Yuan's avatar
      Fix wrong check for aof fsync and handle aof fsync errno (#8751) · a0e19e3c
      Wang Yuan authored
      The bio aof fsync fd may be closed by main thread (AOFRW done handler)
      and even possibly reused for another socket, pipe, or file.
      This can can an EBADF or EINVAL fsync error, which will lead to -MISCONF errors failing all writes.
      We just ignore these errno because aof fsync did not really fail.
      
      We handle errno when fsyncing aof in bio, so we could know the real reason
      when users get -MISCONF Errors writing to the AOF file error
      
      Issue created with #8419
      a0e19e3c
  10. 05 Apr, 2021 1 commit
    • Huang Zhw's avatar
      Fix "default" and overwritten / reset users will not have pubsub channels... · 3b74b550
      Huang Zhw authored
      Fix "default" and overwritten / reset users will not have pubsub channels permissions by default. (#8723)
      
      Background:
      Redis 6.2 added ACL control for pubsub channels (#7993), which were supposed
      to be permissive by default to retain compatibility with redis 6.0 ACL. 
      But due to a bug, only newly created users got this `acl-pubsub-default` applied,
      while overwritten (updated) users got reset to `resetchannels` (denied).
      
      Since the "default" user exists before loading the config file,
      any ACL change to it, results in an update / overwrite.
      
      So when a "default" user is loaded from config file or include ACL
      file with no channels related rules, the user will not have any
      permissions to any channels. But other users will have default
      permissions to any channels.
      
      When upgraded from 6.0 with config rewrite, this will lead to
      "default" user channels permissions lost.
      When users are loaded from include file, then call "acl load", users
      will also lost channels permissions.
      
      Similarly, the `reset` ACL rule, would have reset the user to be denied
      access to any channels, ignoring `acl-pubsub-default` and breaking
      compatibility with redis 6.0.
      
      The implication of this fix is that it regains compatibility with redis 6.0,
      but breaks compatibility with redis 6.2.0 and 2.0.1. e.g. after the upgrade,
      the default user will regain access to pubsub channels.
      
      Other changes:
      Additionally this commit rename server.acl_pubusub_default to
      server.acl_pubsub_default and fix typo in acl tests.
      3b74b550
  11. 04 Apr, 2021 1 commit
    • Sokolov Yura's avatar
      Add cluster-allow-replica-migration option. (#5285) · 1cab9620
      Sokolov Yura authored
      
      
      Previously (and by default after commit) when master loose its last slot
      (due to migration, for example), its replicas will migrate to new last slot
      holder.
      
      There are cases where this is not desired:
      * Consolidation that results with removed nodes (including the replica, eventually).
      * Manually configured cluster topologies, which the admin wishes to preserve.
      
      Needlessly migrating a replica triggers a full synchronization and can have a negative impact, so
      we prefer to be able to avoid it where possible.
      
      This commit adds 'cluster-allow-replica-migration' configuration option that is
      enabled by default to preserve existed behavior. When disabled, replicas will
      not be auto-migrated.
      
      Fixes #4896
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      1cab9620
  12. 01 Apr, 2021 2 commits
    • Wang Yuan's avatar
      Handle remaining fsync errors (#8419) · 1eb85249
      Wang Yuan authored
      In `aof.c`, we call fsync when stop aof, and now print a log to let user know that if fail.
      In `cluster.c`, we now return error, the calling function already handles these write errors.
      In `redis-cli.c`, users hope to save rdb, we now print a message if fsync failed.
      In `rio.c`, we now treat fsync errors like we do for write errors. 
      In `server.c`, we try to fsync aof file when shutdown redis, we only can print one log if fail.
      In `bio.c`, if failing to fsync aof file, we will set `aof_bio_fsync_status` to error , and reject writing just like last writing aof error,  moreover also set INFO command field `aof_last_write_status` to error.
      1eb85249
    • Wen Hui's avatar
      generalize config file check for sentinel (#8730) · d5935bb0
      Wen Hui authored
      The implications of this change is just that in the past when a config file was missing,
      in some cases it was exiting before printing the sever startup prints and sometimes after,
      and now it'll always exit before printing them.
      d5935bb0
  13. 30 Mar, 2021 2 commits
    • Jérôme Loyet's avatar
      Add replica-announced config option (#8653) · 91f4f416
      Jérôme Loyet authored
      The 'sentinel replicas <master>' command will ignore replicas with
      `replica-announced` set to no.
      
      The goal of disabling the config setting replica-announced is to allow ghost
      replicas. The replica is in the cluster, synchronize with its master, can be
      promoted to master and is not exposed to sentinel clients. This way, it is
      acting as a live backup or living ghost.
      
      In addition, to prevent the replica to be promoted as master, set
      replica-priority to 0.
      91f4f416
    • Viktor Söderqvist's avatar
      Add support for plaintext clients in TLS cluster (#8587) · 5629dbe7
      Viktor Söderqvist authored
      The cluster bus is established over TLS or non-TLS depending on the configuration tls-cluster. The client ports distributed in the cluster and sent to clients are assumed to be TLS or non-TLS also depending on tls-cluster.
      
      The cluster bus is now extended to also contain the non-TLS port of clients in a TLS cluster, when available. The non-TLS port of a cluster node, when available, is sent to clients connected without TLS in responses to CLUSTER SLOTS, CLUSTER NODES, CLUSTER SLAVES and MOVED and ASK redirects, instead of the TLS port.
      
      The user was able to override the client port by defining cluster-announce-port. Now cluster-announce-tls-port is added, so the user can define an alternative announce port for both TLS and non-TLS clients.
      
      Fixes #8134
      5629dbe7
  14. 26 Mar, 2021 1 commit
    • Huang Zhw's avatar
      make processCommand check publish channel permissions. (#8534) · e138698e
      Huang Zhw authored
      Add publish channel permissions check in processCommand.
      
      processCommand didn't check publish channel permissions, so we can
      queue a publish command in a transaction. But when exec the transaction,
      it will fail with -NOPERM.
      
      We also union keys/commands/channels permissions check togegher in
      ACLCheckAllPerm. Remove pubsubCheckACLPermissionsOrReply in 
      publishCommand/subscribeCommand/psubscribeCommand. Always 
      check permissions in processCommand/execCommand/
      luaRedisGenericCommand.
      e138698e
  15. 25 Mar, 2021 1 commit
    • Oran Agra's avatar
      Fix SLOWLOG for blocked commands (#8632) · 497351ad
      Oran Agra authored
      * SLOWLOG didn't record anything for blocked commands because the client
        was reset and argv was already empty. there was a fix for this issue
        specifically for modules, now it works for all blocked clients.
      * The original command argv (before being re-written) was also reset
        before adding the slowlog on behalf of the blocked command.
      * Latency monitor is now updated regardless of the slowlog flags of the
        command or its execution (their purpose is to hide sensitive info from
        the slowlog, not hide the fact the latency happened).
      * Latency monitor now uses real_cmd rather than c->cmd (which may be
        different if the command got re-written, e.g. GEOADD)
      
      Changes:
      * Unify shared code between slowlog insertion in call() and
        updateStatsOnUnblock(), hopefully prevent future bugs from happening
        due to the later being overlooked.
      * Reset CLIENT_PREVENT_LOGGING in resetClient rather than after command
        processing.
      * Add a test for SLOWLOG and BLPOP
      
      Notes:
      - real_cmd == c->lastcmd, except inside MULTI and Lua.
      - blocked commands never happen in these cases (MULTI / Lua)
      - real_cmd == c->cmd, except for when the command is rewritten (e.g.
        GEOADD)
      - blocked commands (currently) are never rewritten
      - other than the command's CLIENT_PREVENT_LOGGING, and the
        execution flag CLIENT_PREVENT_LOGGING, other cases that we want to
        avoid slowlog are on AOF loading (specifically CMD_CALL_SLOWLOG will
        be off when executed from execCommand that runs from an AOF)
      497351ad
  16. 24 Mar, 2021 1 commit
  17. 22 Mar, 2021 2 commits
  18. 16 Mar, 2021 1 commit
  19. 14 Mar, 2021 1 commit
  20. 10 Mar, 2021 2 commits
    • guybe7's avatar
      Fix some issues with modules and MULTI/EXEC (#8617) · 3d0b427c
      guybe7 authored
      Bug 1:
      When a module ctx is freed moduleHandlePropagationAfterCommandCallback
      is called and handles propagation. We want to prevent it from propagating
      commands that were not replicated by the same context. Example:
      1. module1.foo does: RM_Replicate(cmd1); RM_Call(cmd2); RM_Replicate(cmd3)
      2. RM_Replicate(cmd1) propagates MULTI and adds cmd1 to also_propagagte
      3. RM_Call(cmd2) create a new ctx, calls call() and destroys the ctx.
      4. moduleHandlePropagationAfterCommandCallback is called, calling
         alsoPropagates EXEC (Note: EXEC is still not written to socket),
         setting server.in_trnsaction = 0
      5. RM_Replicate(cmd3) is called, propagagting yet another MULTI (now
         we have nested MULTI calls, which is no good) and then cmd3
      
      We must prevent RM_Call(cmd2) from resetting server.in_transaction.
      REDISMODULE_CTX_MULTI_EMITTED was revived for that purpose.
      
      Bug 2:
      Fix issues with nested RM_Call where some have '!' and some don't.
      Example:
      1. module1.foo does RM_Call of module2.bar without replication (i.e. no '!')
      2. module2.bar internally calls RM_Call of INCR with '!'
      3. at the end of module1.foo we call RM_ReplicateVerbatim
      
      We want the replica/AOF to see only module1.foo and not the INCR from module2.bar
      
      Introduced a global replication_allowed flag inside RM_Call to determine
      whether we need to replicate or not (even if '!' was specified)
      
      Other changes:
      Split beforePropagateMultiOrExec to beforePropagateMulti afterPropagateExec
      just for better readability
      3d0b427c
    • guybe7's avatar
      Cleanup ZADD_* flags (#8559) · 61a73de6
      guybe7 authored
      Have a clear separation between in and out flags
      
      Other changes:
      
      delete dead code in RM_ZsetIncrby: if zsetAdd returned error (happens only if
      the result of the operation is NAN or if score is NAN) we return immediately so
      there is no way that zsetAdd succeeded and returned NAN in the out-flags
      61a73de6
  21. 05 Mar, 2021 1 commit
  22. 03 Mar, 2021 1 commit
  23. 02 Mar, 2021 1 commit
  24. 01 Mar, 2021 1 commit
    • YaacovHazan's avatar
      Make port, tls-port and bind configurations modifiable (#8510) · a031d268
      YaacovHazan authored
      Add ability to modify port, tls-port and bind configurations by CONFIG SET command.
      
      To simplify the code and make it cleaner, a new structure
      added, socketFds, which contains the file descriptors array and its counter,
      and used for TCP, TLS and Cluster sockets file descriptors.
      a031d268
  25. 28 Feb, 2021 1 commit
    • Viktor Söderqvist's avatar
      Shared reusable client for RM_Call() (#8516) · 6122f1c4
      Viktor Söderqvist authored
      A single client pointer is added in the server struct. This is
      initialized by the first RM_Call() and reused for every subsequent
      RM_Call() except if it's already in use, which means that it's not
      used for (recursive) module calls to modules. For these, a new
      "fake" client is created each time.
      
      Other changes:
      * Avoid allocating a dict iterator in pubsubUnsubscribeAllChannels
        when not needed
      6122f1c4