fix(plugin-redis): stop truncating values and keep binary values byte exact - #2008
Merged
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Redis string values were cut at 1,000 characters with
...appended, and that cut value was the only copy the app held.Root cause
truncatePreviewinRedisPluginDriver+ResultBuilding.swiftcapped every Value cell at 1,000 UTF-16 units. For a STRING key the Value column is not a preview, it is the value, and there is no lazy cell fetch inPluginDatabaseDriver, so the truncated string reached the grid, the JSON tab, Copy JSON, the row inspector and exports. Editing such a cell and saving issuedSET key <cut value + "...">and destroyed the rest in Redis.All three browse entry points (
KEYBROWSE,SCAN,KEYS) fed the same builder, and the export stream had a second copy of the same pipeline.Values
Strings now return complete.
GETis O(1), so there is no reason to bound it, and this matches every other driver in the app, which returns a whole TEXT column and lets the grid truncate at display time. With no incomplete value in the row, the write-back data loss goes away by construction rather than by a guard.Collections stay bounded because unbounded
HGETALLandLRANGE 0 -1block Redis's single thread. The byte cut is gone: previews are built withJSONSerializationinstead of hand-rolled escaping and string concatenation, so a cell always holds valid JSON. Slashes are left unescaped so URLs stay readable.A new Length column carries what Redis itself reports (
STRLEN,LLEN,HLEN,SCARD,ZCARD,XLEN), so you can see how much a collection preview leaves out. It costs one extra pipelined O(1) command in a pipeline that was already being issued, and it is read-only through the existingimmutableColumnsmechanism that MongoDB and Etcd already use.The export stream and the paged browse now share one row builder, which is why this deletes more than it adds in that file.
Sidebar
The key tree ran
KEYS *through the browse builder, so with full values it would have retained the value of every key just to draw a tree. It now uses aKEYTREEcommand returning Key and Type only, and scans withSCANrather thanKEYS, which blocks the server for its whole run.Binary
A value that is not valid UTF-8 was shown as base64 with no indication, and saving wrote the base64 back.
parseReplyalready preserved exact bytes; onlyredisReplyToStringlost them.The Redis command path is now byte exact end to end:
RedisArgumentCodecimplements Redis's own argument grammar (sdssplitargs, whatredis-cliuses).splitdecodes\xHH,\n,\r,\t,\b,\aand both quote styles, and rejects unbalanced quotes instead of guessing.quoteemits the same grammar back. This replaces the hand-rolledescapeArgumentand the oldCharacter-based tokenizer, which could not represent arbitrary bytes at all.[Data]. hiredis was always length aware (redisCommandArgvwithargvlen); only the Swift side narrowed it. Text overloads sit on the byte primitive so the 59 ASCII call sites are unchanged.SET,HSET,LPUSH,RPUSH,SADD,SREM,ZADDandZREMcarryData. Keys, patterns, flags and numbers stayString..bytes, not only from the column type, so a binary value in Redis's mixed-type Value column opens the hex editor instead of the inline text editor.Behaviour change
A command with unbalanced quotes now errors instead of being silently mangled, matching
redis-cli.\xHHin the query editor writes a raw byte, so commands paste both ways.Tests
New:
RedisArgumentCodecTests,RedisBinaryValueTests,RedisKeySummaryTests,RedisKeyTreeCommandTests, plus binary cases inCellInteractionResolverTestsand browse-column cases inRedisStatementGeneratorTests.RedisCommandParseris now symlinked intoPluginTestSources, so the tests exercise the real parser rather than a copy.Coverage includes all 256 byte values through quote then split, 200 random blobs, gzip and MessagePack headers, the full redis-cli grammar, generator output re-parsed back to the original bytes, and a hostile value that tries to append
DEL victimto the command.No PluginKit ABI change, and Redis is a bundled plugin, so no registry release is needed.
Not covered
Keys stay UTF-8 text. The sidebar tree, namespace grouping, filters and layout persistence all treat keys as strings, so binary keys are a separate change of similar size. It is now documented as a limitation rather than a silent drop.
Inside a collection preview a binary element still renders as base64, since JSON cannot carry raw bytes. Those cells are previews and are not writable.
No UI automation was added. The grid change is a resolver decision covered by unit tests; driving a binary Redis value through a UI test would need a fixture server.