Skip to content

[BUG] DIGEST/IFDEQ/IFDNE are unusually opinionated about leading zeros and case #14496

@mgravell

Description

@mgravell

(for reader context; the digest here is a 64-bit hash of the associated value, via xxhash3, expressed as hex - so typically 16 hex characters)

Summary

  1. the server does not ever return leading zeros in a hash from DIGEST
  • this means returned hashes can appear shorter than may be anticipated, and may cause client bugs if clients assume length 16
  1. the server does not expect or allow leading zeros in a hash in IFDEQ / IFDNE
  • if a client sends leading zeros (presumably from a locally computed hash, rather than via DIGEST), it does not get considered as a match; correctness failure
  1. the server demands case-sensitivity on the hex in IFDEQ / IFDNE
  • if a client sends upper-case hex (the same value, fundamentally), it does not get considered as a match; correctness failure

Repro

Example 1 (leading zeros, with an example that hashes to 0x00006c38adf31777):

> set foo v8lf0c11xh8ymlqztfd3eeq16kfn4sspw7fqmnuuq3k3t75em5wdizgcdw7uc26nnf961u2jkfzkjytls2kwlj7626sd
+ OK
> digest foo
$ 6c38adf31777
> delex foo ifdeq 00006c38adf31777
: 0
> delex foo ifdeq 0006c38adf31777
: 0
> delex foo ifdeq 006c38adf31777
: 0
> delex foo ifdeq 06c38adf31777
: 0
> delex foo ifdeq 6c38adf31777
: 1

Example 2 (case sensitivity):

> set foo v8lf0c11xh8ymlqztfd3eeq16kfn4sspw7fqmnuuq3k3t75em5wdizgcdw7uc26nnf961u2jkfzkjytls2kwlj7626sd
+ OK
> digest foo
$ 6c38adf31777
> delex foo ifdeq 6C38ADF31777
: 0
> delex foo ifdeq 6c38adf31777
: 1

Suggestions

(both under the category of "least surprises"):

  1. hashes should always be exactly 16 hex chars, in both directions
  • this should be enforced, i.e. sending a different number of characters in IFDEQ / IFDNE etc should simply be rejected by the server
  1. if you disagree on "1", then leading zeros in hashes should be considered as valid and equivalent in IFDEQ / IFDNE, i.e. 6c38adf31777 and 00006c38adf31777 should be semantically identical
  2. hashes should be compared in a case insensitive way in IDFEQ / IFDNE, i.e. 00006c38adf31777 and 00006C38ADF31777 should be semantically identical
  3. there should be no change to IFEQ / IFNE - leading zeros and case should be significant

(everything DELEX should also apply identically to SET)

As a side note: I'm super intrigued as to what the correct DIGEST is for something that hashes to all zeros; is it $1\r\n0\r\n ? or is it $0\r\n\r\n ? I lack the computational power to find an example, though. If the logic remains "omit leading zeros", the behaviour of all-zero should be documented however - along with instructions to now buy a lottery ticket.


Justification (prior/consistency) for case-insensitivity:

> script load 'return 42'
$ 1fa00e76656cc152ad327c13fe365858fd7be306
> evalsha 1fa00e76656cc152ad327c13fe365858fd7be306 0
: 42
> evalsha 1FA00E76656CC152AD327C13FE365858FD7BE306 0
: 42

Justification (prior/consistency) for retaining and enforcing leading zeros:

> script load 'return 3'
$ 09d3822de862f46d784e6a36848b4f0736dda47a
> evalsha 09d3822de862f46d784e6a36848b4f0736dda47a 0
: 3
> evalsha 9d3822de862f46d784e6a36848b4f0736dda47a 0
- "NOSCRIPT No matching script. Please use EVAL."

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions