[Mon Apr 24 23:17:44 EST 2000]

Dear Tux, 

So, we now have an example of corruption detected by using file
checksums.  I'm awfully glad we put them back in!  So, now we just
have to work out what's going wrong!

I trapped into gdb, so we can poke around a little bit.  Also, we have
both the old and new files present, which is pretty cool.  The files
generated a match just at the end, which might be a little
suspicious.  We're using the version wired to 1024-byte blocks, and
it's walking through the rproxy cvsweb tree in sourceforge.  So
probably the header was less than 1024 bytes, but we fould a common
trailer. 

(hsync   ) Writing LITERAL(len=3733)
(hsync   ) Writing COPY(off=7168, len=214)
(hsync   ) flush 56 bytes of signature data
(hsync   ) Writing SIGNATURE(len=56)
(hsync   ) filesum is 1ce4f46035b8ad1921039e7ffbb31a77
(hsync   ) Writing CHECKSUM(len=16)
(hsync   ) Writing EOF

I'm finding this a bit strange because the original file has more than
214 bytes after position 7168.  In fact's it's 8406 bytes long.  Huh.

The 214 bytes of the old file starting at position 7168 are

[[[hange='document.diff_select.r2.selectedIndex=0'>
<BR>Type of Diff should be a&nbsp;<SELECT NAME="f">
<OPTION VALUE=h SELECTED>Colored Diff<OPTION VALUE=H>Long Colored Diff<OPTION VALUE=u>Unidiff<OPTION VALUE=c>Cont]]]

This chunk was supposed to be inserted at position 3733 (zero based).

I also retrieved the real file using curl(1).  Sure enough the
difference between them begins at byte 3734 (one-based).  So, it looks
like the literal data came through OK, but the short copy command was
wrong.

The real version shows this at that position:

[[[VALUE=rev>Revision</SELECT>
<INPUT TYPE=SUBMIT VALUE="  Sort  ">
</FORM>
</A><hr noshade>Created by a SourceForge version of <A href="http://stud.fh-heilbronn.de/~zeller/cgi/cvsweb.cgi/">CVSweb</A>.
</BODY></HTML>
]]]

and the reconstructed version finishes like this:

[[[ange='document.diff_select.r2.selectedIndex=0'>
<BR>Type of Diff should be a&nbsp;<SELECT NAME="f">
<OPTION VALUE=h SELECTED>Colored Diff<OPTION VALUE=H>Long Colored Diff<OPTION VALUE=u>Unidiff<OPTION VALUE=c>Conte]]]

I see that in the second case we did actually write out a 3948-byte
file, so the chunk written was in fact (- 3948 3734) 214 bytes.  So,
it seems we did execute the COPY command correctly, though we
generated it wrongly.  

The real file is also 3948 bytes.  There are more than 214 bytes at
the end of the file in common with the old version, so it was OK to
generate a COPY, but the instructions went wrong somehow.

There is a the old version for the string at the end of the correct
version, and it's at a position of 8192.  This is an extremely
sensible place for it to be, because it's on a block boundary.  Hmm!
7168 = 7 * 1024, so we sent the right instruction but were off by one
block length.

The encoding rproxy said 

(hsync   ) Read 8 sigs
(hsync   ) The block length is 1024
libhsync: _hs_must_read: unexpected eof
(hsync   ) found weak match for 0x141e5dc3 in token 8
(hsync   ) found token 8 in stream at abspos=3733     length=214   

(I think the `unexpected eof' comment is because it was trying to read
another signature integer and got an end-of-signature.  We can fix
that.)

Now, I *thought* the token numbers were one-based, so that would mean
this is wrong: a match at this position ought to be a match on token
nine. 

We shouldn't rule out the possibility that something else strange has
gone wrong, and we're using the wrong signatures for this file.
However, because this does line up exactly I don't think that' the
case.

If we have more than 8kb of data, how can it say that it only received
8 signatures?  There ought to be nine signatures to cover it.

Well, let's have a look through the signatures in memory.  Ah,
unfortunately it's too late; the encoder has exited.  Perhaps we can
set a breakpoint when emitting copy commands and have a look there.

OK, this seems to be fixed by cleaning up checksum.c.  What did I
change?  This will be r1.11 of checksum.c.  Not a lot.  In
_hs_make_sum_struct there was a variable `count' which was redundant,
so I merged it with the `index' variable.

BTW, we seem to be leaking. :-(  Time to run under ccmalloc again I think.
The simplest solution would be just to run in forking mode.


[Tue Apr 25 17:36:51 EST 2000]

So, we managed to find a version mismatch again, this time looking at
http://slashdot.org/comments.pl?sid=spsc.  This one's a little bit
harder because the page may not be exactly static so we can't find the
true version.

Still, I'm kind of relieved that the bug hasn't mysteriously gone
away.

In this case the received instructions were

(hsync   ) LITERAL(len=1001)
(hsync   ) COPY(offset=1024, len=2048)
(hsync   ) LITERAL(len=21791)
(hsync   ) COPY(offset=7168, len=256)
(hsync   ) SIGNATURE(len=284)
(hsync   ) CHECKSUM(len=16)

Again we're having trouble with a file which has a short match at the
end.  I'm happy to see that we are no longer leaking file
descriptors.  I wonder if this is also off by one?

The old file is 8448 = 8192 + 256 bytes long, so the short block
should be number *nine*.  What happened here?

Perhaps we emitted the same checksums for both blocks?  The old
signature file is 104 bytes, which is kind of strange.  It ought to
have a 4-byte magic, 4-byte block length, and then contain
fixed-length signature blocks, each of which will be 4 bytes for a
weak sum, plus 8 bytes for a strong sum for a total of 12.  So, we
have apparently only 8*12 = 96 bytes of signature data.  Did we
actually write the signature for the last block.  

At the moment the special case of writing out the block has on the
last block is commented out.  We need to rethink the integration of
checksumming and differencing in enc.c.

I think perhaps we should try constructing a file which has a matching
section only at the end.