Skip to content

Add BlobHeader integrity validation#265

Merged
koujl merged 1 commit intoeBay:mainfrom
koujl:blob_header
Mar 6, 2025
Merged

Add BlobHeader integrity validation#265
koujl merged 1 commit intoeBay:mainfrom
koujl:blob_header

Conversation

@koujl
Copy link
Copy Markdown
Contributor

@koujl koujl commented Mar 4, 2025

Implement CRC32 hash integrity check for BlobHeader to prevent crashes caused by faults in critical fields (size/algorithm...).

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 4, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 53.84615% with 12 lines in your changes missing coverage. Please review.

Project coverage is 62.24%. Comparing base (1746bcc) to head (942c9f4).
Report is 44 commits behind head on main.

Files with missing lines Patch % Lines
src/lib/homestore_backend/hs_homeobject.hpp 54.16% 9 Missing and 2 partials ⚠️
src/lib/homestore_backend/hs_blob_manager.cpp 50.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #265      +/-   ##
==========================================
- Coverage   63.15%   62.24%   -0.91%     
==========================================
  Files          32       33       +1     
  Lines        1900     2633     +733     
  Branches      204      315     +111     
==========================================
+ Hits         1200     1639     +439     
- Misses        600      828     +228     
- Partials      100      166      +66     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

switch (hash_algorithm) {
case HashAlgorithm::NONE:
return true;
case HashAlgorithm::CRC32: {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should use the same hash_algorithm between valid and seal , right?
seal is now always using crc32, so other hash_algorithm does not make sense to valid.

I mean can we add a member to indicate the hash_algorithm, and valid and seal will share it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, will fix in the next push.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yuwmao
yuwmao previously approved these changes Mar 5, 2025
Copy link
Copy Markdown
Contributor

@yuwmao yuwmao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Implement CRC32 hash integrity check for BlobHeader to prevent crashes
caused by faults in critical fields (size/algorithm...).
@koujl koujl merged commit 1f5683e into eBay:main Mar 6, 2025
@koujl koujl deleted the blob_header branch March 6, 2025 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants