Skip to content

Sanitize dump payload: handle remaining empty key when RDB loading and restore command#9349

Merged
oranagra merged 3 commits into
redis:unstablefrom
sundb:fix-emptykey-with-santitize
Aug 9, 2021
Merged

Sanitize dump payload: handle remaining empty key when RDB loading and restore command#9349
oranagra merged 3 commits into
redis:unstablefrom
sundb:fix-emptykey-with-santitize

Conversation

@sundb

@sundb sundb commented Aug 9, 2021

Copy link
Copy Markdown
Collaborator

This pr mainly fixes empty keys due to RDB loading and restore command, which was omitted in #9297.

  1. When loading quicklsit, if all the ziplists in the quicklist are empty, NULL will be returned.
    If only some of the ziplists are empty, then we will skip the empty ziplists silently.
  2. When loading hash zipmap, if zipmap is empty, NULL will be returned.
  3. When loading hash ziplist, if ziplist is empty, NULL will be returned.
  4. Add RDB loading test with sanitize.

@sundb

sundb commented Aug 9, 2021

Copy link
Copy Markdown
Collaborator Author

@oranagra I'm not sure if intset needs to be handled in the same way, intsetValidateIntegrity already determines empty intset, should it be changed to the same as other data structures.
I feel it is needed.

@oranagra

oranagra commented Aug 9, 2021

Copy link
Copy Markdown
Member

@sundb i think we can leave intsets and even zipmaps out of this party (if they're empty we can fail)

@oranagra

oranagra commented Aug 9, 2021

Copy link
Copy Markdown
Member

or if you think it's nicer to handle an empty zipmap (just for code consistency), i'm fine with that too.

Comment thread tests/integration/corrupt-dump.tcl Outdated
Comment thread src/rdb.c Outdated
@sundb sundb force-pushed the fix-emptykey-with-santitize branch from abc0063 to bade4f3 Compare August 9, 2021 13:48
@sundb

sundb commented Aug 9, 2021

Copy link
Copy Markdown
Collaborator Author

@oranagra I think it is a better way to avoid adding code to rdb.c, it is huge enough.

@oranagra oranagra merged commit cbda492 into redis:unstable Aug 9, 2021
@sundb sundb deleted the fix-emptykey-with-santitize branch August 10, 2021 02:12
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Sep 8, 2021
…d restore command (redis#9349)

This commit mainly fixes empty keys due to RDB loading and restore command,
which was omitted in redis#9297.

1) When loading quicklsit, if all the ziplists in the quicklist are empty, NULL will be returned.
    If only some of the ziplists are empty, then we will skip the empty ziplists silently.
2) When loading hash zipmap, if zipmap is empty, sanitization check will fail.
3) When loading hash ziplist, if ziplist is empty, NULL will be returned.
4) Add RDB loading test with sanitize.
@oranagra oranagra mentioned this pull request Oct 4, 2021
oranagra pushed a commit that referenced this pull request Oct 4, 2021
…d restore command (#9349)

This commit mainly fixes empty keys due to RDB loading and restore command,
which was omitted in #9297.

1) When loading quicklsit, if all the ziplists in the quicklist are empty, NULL will be returned.
    If only some of the ziplists are empty, then we will skip the empty ziplists silently.
2) When loading hash zipmap, if zipmap is empty, sanitization check will fail.
3) When loading hash ziplist, if ziplist is empty, NULL will be returned.
4) Add RDB loading test with sanitize.

(cherry picked from commit cbda492)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants