zip_reader: clean up after hash setup failures on entry open - #1068
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. WalkthroughThe reader now validates hash algorithm and digest-size metadata before creating a hash context. Hash setup failures use shared cleanup. Reader tests cover oversized, unsupported, and mismatched digest metadata, including repeated entry-open attempts. ChangesHash validation
Suggested reviewers: Priority: ⬇️ Low Change: Bug fix Merge Risk: ⚪ Minimal · up to The hash metadata validation and retry cleanup changes are ready to merge based on the available evidence. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
When the hash object cannot be allocated or the entry names an algorithm the backend does not support, the entry was left open and a partially initialized hash object was kept. A retry of the open then returned success and read the entry without verification. Route every failure after the low-level open through one cleanup that drops the hash object and closes the entry. Assisted-By: Claude <noreply@anthropic.com>
A hash extra field could declare a supported algorithm with any digest length up to the buffer size. A zero length record made the comparison on close trivially succeed, and a longer one compared uninitialized bytes. Reject any length other than the algorithm's digest size before the hash object is created. Assisted-By: Claude <noreply@anthropic.com>
66ae211 to
67169fc
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Please upload reports for the commit 67169fc to get more accurate results. Additional details and impacted files@@ Coverage Diff @@
## develop #1068 +/- ##
===============================
===============================
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Follow-up to #1047 for the remaining review threads on
mz_zip_reader_entry_open.When the hash object could not be allocated or the entry named an unsupported algorithm, the entry was left open and a half initialized hash object was kept. A retry of the open then returned
MZ_OKand read the entry without verification. Every failure after the low-level open now drops the hash object and closes the entry.A hash record could also declare a supported algorithm with any digest length up to
MZ_HASH_MAX_SIZE. A zero length record made the compare on close trivially succeed. The length must now equal the algorithm's digest size, otherwise the open fails withMZ_FORMAT_ERROR.Sources are the review threads on #1047, #1047 (comment) and #1047 (comment).