Re: svn commit: r1826720 - /subversion/trunk/subversion/libsvn_fs_fs/cached_data.c

Philip Martin <philip_at_codematters.co.uk> writes:

> It seems wrong for our checksum code to allow the wrong length, but
> preventing dump when the data could otherwise be extracted is also
> wrong. Perhaps we should simply allow the checksum code to read any
> length when rb->len is zero? That doesn't seem right. Perhaps the
> error should just be a warning as below?

I've decided the error should be an FS warning.

Issue 4554 only occurs when the representation is PLAIN rather than
DELTA and the rest of the code now recognises that and fixes up the zero
length to have the correct value. So issue 4554 will not trigger this
new error, that's good.

The new error will trigger if the length is zero for a DELTA
representation, as the code cannot fix that length, and also for a
non-zero but incorrect length. Since this is repository corruption the
ideal repsonse would be to indicate the problem but to allow the data to
be retreived if possible.

Consider

 svnadmin create repo --compatible-version 1.8
 svnmucc -mm -U file://`pwd`/repo put ../src/INSTALL f
 chmod +w repo/db/revs/0/1

Now edit the r1 file and change the expanded length:

text: 1 0 17791 55700 df5d0e251e4b811f0ed63694e7a2cd00 fb07bf4d765ccf4e18afe74d55

being carefull to keep the file the same length, e.g. change 55700 to
45700. Using 1.10

  svnadmin verify repo

will not report an error, but

  svnadmin dump repo | svnadmin load repo2

will fail because the dump file contains the wrong length. The dump
file does contain the full text, so by editing the dump file the data
could be retreived. With the new warning on trunk I get:

$ subversion/svnadmin/svnadmin verify repo
* Verifying repository metadata ...
* Verified revision 0.
svnadmin: warning: apr_err=SVN_ERR_FS_CORRUPT
svnadmin: warning: W160004: Length mismatch while reading representation expected 45700 got 55700
* Verified revision 1.

which also gives me the information I need to correct the dump file and
also allows the dump to run.

-- 
Philip