From edc1669cedffd5b1eeb5944ae601432bf21b266d Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Wed, 19 Jul 2023 13:44:40 +0530 Subject: [PATCH] Ported tests all pass --- kittens/transfer/algorithm.c | 17 ++++++++++++----- kitty_tests/file_transmission.py | 23 +++++++++++++++-------- tools/rsync/algorithm.go | 7 ++++--- tools/rsync/api_test.go | 2 +- 4 files changed, 32 insertions(+), 17 deletions(-) diff --git a/kittens/transfer/algorithm.c b/kittens/transfer/algorithm.c index b584c1071..f46e40a17 100644 --- a/kittens/transfer/algorithm.c +++ b/kittens/transfer/algorithm.c @@ -339,10 +339,16 @@ apply_op(Patcher *self, Operation op, PyObject *read, PyObject *write) { if (!wret) return false; } return true; case OpHash: { - uint8_t expected[64]; + uint8_t actual[64]; if (op.data.len != self->rsync.checksummer.hash_size) { PyErr_SetString(RsyncError, "checksum digest not the correct size"); return false; } - self->rsync.checksummer.digest(self->rsync.checksummer.state, expected); - if (memcmp(expected, op.data.buf, op.data.len) != 0) { PyErr_SetString(RsyncError, "checksum does not match, this usually happens because one of the involved files was altered while the operation was in progress"); return false; } + self->rsync.checksummer.digest(self->rsync.checksummer.state, actual); + if (memcmp(actual, op.data.buf, self->rsync.checksummer.hash_size) != 0) { + DECREF_AFTER_FUNCTION PyObject *b1 = PyBytes_FromStringAndSize((char*)actual, self->rsync.checksummer.hash_size); + DECREF_AFTER_FUNCTION PyObject *h1 = PyObject_CallMethod(b1, "hex", NULL); + DECREF_AFTER_FUNCTION PyObject *b2 = PyBytes_FromStringAndSize((char*)op.data.buf, self->rsync.checksummer.hash_size); + DECREF_AFTER_FUNCTION PyObject *h2 = PyObject_CallMethod(b2, "hex", NULL); + PyErr_Format(RsyncError, "Failed to verify overall file checksum actual: %S != expected: %S, this usually happens because one of the involved files was altered while the operation was in progress.", h1, h2); + return false; } } return true; } PyErr_SetString(RsyncError, "Unknown operation type"); @@ -504,7 +510,7 @@ static size_t parse_signature_block(Differ *self, uint8_t *data, size_t len) { if (len < 20) return 0; int weak_hash = le32dec(data + 8); - SignatureMap *sm; + SignatureMap *sm = NULL; HASH_FIND_INT(self->signature_map, &weak_hash, sm); if (sm == NULL) { sm = calloc(1, sizeof(SignatureMap)); @@ -713,11 +719,12 @@ read_next(Differ *self) { self->window.sz = self->rsync.block_size; rolling_checksum_full(&self->rc, self->buf.data + self->window.pos, self->window.sz); } - SignatureMap *sm; + SignatureMap *sm = NULL; int weak_hash = self->rc.val; uint64_t block_index = 0; HASH_FIND_INT(self->signature_map, &weak_hash, sm); if (sm != NULL && find_strong_hash(sm, self->rsync.hasher.oneshot64(self->buf.data + self->window.pos, self->window.sz), &block_index)) { + if (!send_data(self)) return false; if (!enqueue(self, (Operation){.type=OpBlock, .block_index=block_index})) return false; self->window.pos += self->window.sz; self->data.pos = self->window.pos; diff --git a/kitty_tests/file_transmission.py b/kitty_tests/file_transmission.py index 47ce9b888..4898798c8 100644 --- a/kitty_tests/file_transmission.py +++ b/kitty_tests/file_transmission.py @@ -78,10 +78,10 @@ def patch_data(data, *patches): def run_roundtrip_test(self: 'TestFileTransmission', src_data, changed, num_of_patches, total_patch_size): buf = memoryview(bytearray(30)) signature = bytearray(0) - p = Patcher(len(src_data)) + p = Patcher(len(changed)) n = p.signature_header(buf) signature.extend(buf[:n]) - src = memoryview(src_data) + src = memoryview(changed) bs = p.block_size while src: n = p.sign_block(src[:bs], buf) @@ -93,6 +93,7 @@ def run_roundtrip_test(self: 'TestFileTransmission', src_data, changed, num_of_p d.add_signature_data(src[:13]) src = src[13:] d.finish_signature_data() + del src, signature src = memoryview(src_data) delta = bytearray(0) def read_into(b): @@ -107,6 +108,7 @@ def run_roundtrip_test(self: 'TestFileTransmission', src_data, changed, num_of_p while d.next_op(read_into, write_delta): pass delta = memoryview(delta) + del src def read_at(pos, output) -> int: b = changed[pos:] @@ -119,14 +121,19 @@ def run_roundtrip_test(self: 'TestFileTransmission', src_data, changed, num_of_p def write_changes(b): output.extend(b) - while delta: - p.apply_delta_data(delta[:11], read_at, write_changes) - delta = delta[11:] - p.finish_delta_data() - self.assertEqual(src_data, bytes(output), f'\n\nsrc:\n{src_data.decode()}\nchanged:\n{changed.decode()}\noutput:\n{output.decode()}') + def debug_msg(): + return f'\n\nsrc:\n{src_data.decode()}\nchanged:\n{changed.decode()}\noutput:\n{output.decode()}' + try: + while delta: + p.apply_delta_data(delta[:11], read_at, write_changes) + delta = delta[11:] + p.finish_delta_data() + except Exception as err: + self.fail(f'{err}\n{debug_msg()}') + self.assertEqual(src_data, bytes(output), debug_msg()) limit = 2 * (p.block_size * num_of_patches) if limit > -1: - self.assertLess( + self.assertLessEqual( p.total_data_in_delta, limit, f'Unexpectedly poor delta performance: {total_patch_size=} {p.total_data_in_delta=} {limit=}') diff --git a/tools/rsync/algorithm.go b/tools/rsync/algorithm.go index 4d309d50e..2d4b46e4e 100644 --- a/tools/rsync/algorithm.go +++ b/tools/rsync/algorithm.go @@ -11,6 +11,7 @@ package rsync import ( "bytes" "encoding/binary" + "encoding/hex" "fmt" "hash" "io" @@ -314,9 +315,9 @@ func (r *rsync) ApplyDelta(alignedTarget io.Writer, target io.ReadSeeker, op Ope return err } case OpHash: - expected := r.checksummer.Sum(nil) - if !bytes.Equal(expected, op.Data) { - return fmt.Errorf("Failed to verify overall file checksum. This usually happens if some data was corrupted in transit or one of the involved files was altered while the transfer was in progress.") + actual := r.checksummer.Sum(nil) + if !bytes.Equal(actual, op.Data) { + return fmt.Errorf("Failed to verify overall file checksum actual: %s != expected: %s. This usually happens if some data was corrupted in transit or one of the involved files was altered while the transfer was in progress.", hex.EncodeToString(actual), hex.EncodeToString(op.Data)) } } return nil diff --git a/tools/rsync/api_test.go b/tools/rsync/api_test.go index 2dd8c715d..302405f21 100644 --- a/tools/rsync/api_test.go +++ b/tools/rsync/api_test.go @@ -85,7 +85,7 @@ func run_roundtrip_test(t *testing.T, src_data, changed []byte, num_of_patches, // Now try with serialization using_serialization = true - p = NewPatcher(int64(len(src_data))) + p = NewPatcher(int64(len(changed))) signature_of_changed := bytes.Buffer{} ss_it := p.CreateSignatureIterator(bytes.NewReader(changed), &signature_of_changed) var err error