Skip to content

bug(redis): list item edit/delete by index is racy and mishandles binary values #1009

Description

@ZhuchkaTriplesix

Summary

RedisKeyEditor._listSet and _listRemove (lib/features/redis/redis_key_editor.dart:480 and :492) edit/delete a Redis list element by numeric index, using two separate round trips:

Future<void> _listSet(int index, String newValue) async {
  ...
  await widget.connection.lset(_cmdKey, index, newValue);
  await _load();
}

Future<void> _listRemove(int index, RedisBulkValue item) async {
  ...
  final sentinel = '__QUERYA_DEL_${DateTime.now().microsecondsSinceEpoch}__';
  await widget.connection.lset(_cmdKey, index, sentinel);
  await widget.connection.lrem(_cmdKey, 1, sentinel);
  await _load();
}

Problems

  1. Race with concurrent writers. index is taken from the last _load()'s snapshot of the list. If another client pushes/pops/reorders the list between that load and the LSET/LREM call, the index no longer refers to the element the user saw and clicked on — the wrong element gets overwritten or deleted. There's no WATCH/MULTI or optimistic value check against what was displayed.

  2. Delete can leave a sentinel behind. _listRemove does LSET index sentinel followed by LREM 1 sentinel as two independent commands. If the connection drops, times out, or the app crashes between them, the list is left with a stray __QUERYA_DEL_<timestamp>__ string element instead of either the original value or a clean removal.

  3. Binary (non-UTF-8) list items are edited via their display label. The edit dialog is seeded from _listValue[i].label:

    final edited = await showAppDialog<String>(
      context: context,
      builder: (ctx) => _RedisEditListDialogContent(
        index: i,
        initialValue: _listValue[i].label,
      ),
    );

    For a binary value, .label is a lossy display representation (not necessarily round-trippable), so committing an "edit" the user didn't intend to make (e.g. just opening and closing, or making an unrelated single-char change) can silently corrupt binary list data.

Suggested fix

  • For edit: read the current value at index immediately before LSET and compare against what was displayed; abort with an error if it changed (optimistic concurrency), or use WATCH/MULTI/EXEC if the RESP client supports it.
  • For delete: prefer a single atomic operation if the driver/protocol allows it, or at minimum detect and clean up an orphaned sentinel on next load.
  • Disable (or clearly warn on) editing for binary/non-UTF-8 list items rather than routing them through the lossy .label text field.

Found during a code-quality review of the dev branch's Redis list-editing changes (redisRename/redisLrem additions), not from a reported user incident.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingconnectionsDatabase connections, URI parsing, poolsdata-gridInteractive data grid, cell editor, filtering, groupings

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions