mirage / irmin

Irmin is a distributed database that follows the same design principles as Git
https://irmin.org
ISC License
1.83k stars 154 forks source link

irmin-git: fix decoding contents in the middle of a buffer #2277

Closed metanivek closed 10 months ago

metanivek commented 10 months ago

This code previously assumed that it could read to the end of the buffer, but this is not always the case. This issue was discovered while testing the new batch API in irmin-client.

This PR updates irmin-git to use ocaml-git 3.14.0 or newer. This release of ocaml-git fixes the discovered bug and also exposes a function (length_with_header) that can be used to know the length of buffer read. I also updated the client/server example to use an in-memory git store, which can be used to manually verify this fix.

codecov-commenter commented 10 months ago

Codecov Report

Merging #2277 (08c4142) into main (2e7a6a2) will increase coverage by 0.01%. The diff coverage is 0.00%.

:exclamation: Current head 08c4142 differs from pull request most recent head 39d14fe. Consider uploading reports for the commit 39d14fe to get more accurate results

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

@@            Coverage Diff             @@
##             main    #2277      +/-   ##
==========================================
+ Coverage   68.12%   68.14%   +0.01%     
==========================================
  Files         133      133              
  Lines       15982    15983       +1     
==========================================
+ Hits        10888    10891       +3     
+ Misses       5094     5092       -2     
Files Coverage Δ
src/irmin-git/contents.ml 45.00% <0.00%> (-2.37%) :arrow_down:

... and 3 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

art-w commented 10 months ago

Thanks, it looks good! (~I'm guessing the CI doesn't yet see the new release of ocaml-git~ ... ~Hm but actually the commit used for the opam-repository is recent enough, so there might be a conflict in our dependencies~ Nevermind, the dependency analysis went through but now it seems stuck elsewhere)