Repository navigation
fix(drivers): handle null and trailing peer data when decoding - #197
Conversation
Elasticsearch search hits and mget docs can carry "_source": null.
The decoder left a nil map and then wrote the _id into it. That
write panicked and ended the process, including the iq mcp server.
A null _source now gives {"_id": id}, the same as no _source.
Couchbase decodeValue read only the first JSON value and ignored
the bytes after it. A body like {"a":1}garbage became {"a":1}, and
a copy lost the rest. Now any body with data after the first value
counts as not JSON and returns as the raw string. Trailing
whitespace is still allowed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Zsolt Gaspar <17578237+zsltg@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
| Layer / File(s) | Summary |
|---|---|
Reject trailing JSON data drivers/couchbase/normalize.go, drivers/couchbase/normalize_test.go |
decodeValue returns the full input as a string when data follows the first JSON value. Tests cover malformed and concatenated input, valid objects, and trailing whitespace. |
Elasticsearch null source handling
| Layer / File(s) | Summary |
|---|---|
Normalize null _source documents drivers/elasticsearch/normalize.go, drivers/elasticsearch/normalize_test.go, drivers/elasticsearch/scan_test.go |
decodeSource replaces a nil map with an empty map before normalization and _id injection. Tests cover direct decoding, scan results across pages, and mget results. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Suggested reviewers: codescene-access
Merge Risk: 
6b304
-
The decoding changes preserve valid data, avoid truncated Couchbase values, and safely normalize null Elasticsearch sources. No merge-blocking risk remains.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title is concise, specific, and accurately summarizes both decoder fixes: handling null values and trailing data. Description check The description explains the two defects, the intended behavior, scope decisions, and added regression tests. It also reports completed checks and identifies checks left to CI. The required sections a… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description explains the two defects, the intended behavior, scope decisions, and added regression tests. It also reports completed checks and identifies checks left to CI. The required sections are present and sufficiently complete.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 6 functions across 5 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
@coderabbitai review Generated by Claude Code |
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
✅ Action performed
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #197 +/- ##
=======================================
Coverage 96.43% 96.43%
=======================================
Files 145 145
Lines 14559 14563 +4
=======================================
+ Hits 14040 14044 +4
Misses 518 518
Partials 1 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
What and why
Two defects in how drivers decode documents that a database server returns. A security audit found both.
"_source": nullmadedecodeSourcewrite the_idinto a nil map. That panicked. The CLI has no recover, and underiq mcpthe whole server process ended. A null_sourcenow gives{"_id": id}, the same as an absent_source.decodeValueread the first JSON value and ignored the bytes after it. A body like{"a":1}garbagebecame{"a":1}, so a copy kept a prefix and lost the rest. A body with data after the first value now counts as not JSON and returns as the raw string, which is the documented behavior for a non-JSON body. Trailing whitespace is still allowed.I checked the sibling decoders:
decodeDoccan return a nil map for anulldocument, but no caller writes into it. Its input is one value that kivik already parsed, so it cannot carry trailing bytes.decodeJSONreads one value from a RedisJSON reply that the server produces. That shape is out of scope here.Checks
make checkmake cover: left to CImake security: left to CIscripts/mutation-gate.sh: left to CITestDecodeSource/null source still yields the id,TestScanSurvivesANullSource,TestGetSurvivesANullSource,TestDecodeValueTrailingData. With the fix reverted,TestDecodeSourcepanics andTestDecodeValueTrailingDatafails.scripts/capabilities.sh: not needed,go.modandgo.sumdid not changeGenerated by Claude Code
Summary by CodeRabbit
nullsource are now handled without errors and return a document containing its ID, including in scans and multi-get results.