On Thu, Nov 11, 2021 at 05:05:06AM -0500, Jeff King wrote: > > Since SHA1 is essentially random, there's a 1:256 chance of that > > happening, assuming that the file's content is random. > > The most exact fix here would be to read the final byte, increment it > mod-256, and write it back, which would ensure it was always changed. > But that's a bit annoying to do in shell. Perhaps just corrupting more > bytes is a good solution? The patch below should reduce your changes to > 1 in 2^80. > > diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh > index 3f69e43178..a612e44547 100755 > --- a/t/t5319-multi-pack-index.sh > +++ b/t/t5319-multi-pack-index.sh > @@ -482,8 +482,10 @@ test_expect_success 'corrupt MIDX is not reused' ' > ' > > test_expect_success 'verify incorrect checksum' ' > - pos=$(($(wc -c <$objdir/pack/multi-pack-index) - 1)) && > - corrupt_midx_and_verify $pos "\377" $objdir "incorrect checksum" > + pos=$(($(wc -c <$objdir/pack/multi-pack-index) - 10)) && > + corrupt_midx_and_verify $pos \ > + "\377\377\377\377\377\377\377\377\377\377" \ > + $objdir "incorrect checksum" > ' > > test_expect_success 'repack progress off for redirected stderr' ' > > There are other variants, of course. Just appending a single byte to the > file is enough to give you a high probability of failing the checksum > (since it shifts it all by one byte, making it essentially random), but > the corrupt_midx() helper doesn't support that. Thanks SZEDER for the report, and thanks Peff for the fix :). I agree that it's annoyingly cumbersome to write back the last byte incremented mod-256. So I'm content to just make it astronomically unlikely to run into a collision in practice. (As a matter of fact, I'm surprised that the current implementation hasn't produced failures for us more often). Peff: do you want me to turn this into a patch or were you planning on it? Thanks, Taylor
