On Wed, Mar 7, 2018 at 10:37 PM, Philip Martin <philip_at_codematters.co.uk>
wrote:
>
> The cause of the bug, as identified by Melissa, is that the wrong
> expanded size if computed -- it gets set to the size of the delta in the
> protorev file. The svn_stream_t that is producing the fulltext works in
> 16K chunks, so the total amout of fulltext goes up in steps: 16K, 32K,
> 48K, etc. If one of those steps happens to match the erroneous expanded
> size the stream treats that as the end of the fulltext. The stream then
> finalizes the MD5 checksum for the partial fulltext and the MD5 checksum
> failure occurs.
>
> For most commits the protorev delta is not a multiple of 16K and so the
> erroneous expanded size does not match any of the fulltext steps.
> Eventually the stream reaches EOF and gets a short read less than 16K.
> The function svn_stream_contents_same2 recognises the short read as EOF
> and ends the comparison between the two streams, however the stream
> itself has not finalized the checksum because of the length mismatch.
> This means we did not verify the MD5 checksum for the protorev fulltext
> but we did verify that the protorev fulltext matches the fulltext in the
> repository.
>
Is it possible and does it make sense to always continue reading until
EOF, when the read is either 0 or < 16k? In other words to eliminate
the comparison against the expanded size?
Alternatively, if reading up to the expanded size and then stopping,
does it make sense to attempt another read, which must come back as 0,
to verify that the expanded size and actual size are equal? If not
equal, an error message on this issue would be far more helpful than
the one currently output.
On Thu, Mar 8, 2018 at 6:41 AM, Philip Martin <philip_at_codematters.co.uk>
wrote:
>
> https://issues.apache.org/jira/browse/SVN-4722
>
> My reproduction doesn't trigger the bug in 1.8 but that seems to be
> because 1.8 has some other problem and the rep-cache deduplication
> doesn't trigger. The third commit restoring the content in the first
> commit simply stores a new delta.
>
Have you had a chance to test against the 1.10 rc?
Received on 2018-03-08 15:25:54 CET