diff --git a/src/constants.h b/src/constants.h index 0b1a0deb..36e6729c 100644 --- a/src/constants.h +++ b/src/constants.h @@ -27,4 +27,5 @@ // provide a way to control it (so it is at Xapian's default value of 64) #define MAX_INDEXABLE_TITLE_WORD_SIZE 240 +constexpr size_t CHECKSUM_BUFFER_SIZE = 64 * 1024; constexpr size_t TITLE_LISTING_BUFFER_SIZE = 64 * 1024; diff --git a/src/fileimpl.cpp b/src/fileimpl.cpp index 2060975b..3d1932c5 100644 --- a/src/fileimpl.cpp +++ b/src/fileimpl.cpp @@ -30,7 +30,7 @@ #include "zim_types.h" #include -#define CHUNK_SIZE 1024 +#include #include #include #include "_dirent.h" @@ -39,11 +39,11 @@ #include #include #include -#include #include #include "config.h" #include "log.h" #include "md5.h" +#include "constants.h" #include "tools.h" #include "fileheader.h" @@ -610,44 +610,26 @@ class Grouping struct zim_MD5_CTX md5ctx; zim_MD5Init(&md5ctx); - unsigned char ch[CHUNK_SIZE]; - offset_type checksumPos = header.getChecksumPos(); - offset_type toRead = checksumPos; - - for(auto part = zimFile->begin(); - part != zimFile->end(); - part++) { - std::ifstream stream(part->second->filename(), std::ios_base::in|std::ios_base::binary); - - while(toRead>=CHUNK_SIZE && stream.read(reinterpret_cast(ch),CHUNK_SIZE).good()) { - zim_MD5Update(&md5ctx, ch, CHUNK_SIZE); - toRead-=CHUNK_SIZE; - } - - // Previous read was good, so we have exited the previous `while` because - // `toRead(ch),toRead); - } - - // It updates the checksum with the remaining amount of data when we - // reach the end of the file or part - zim_MD5Update(&md5ctx, ch, stream.gcount()); - toRead-=stream.gcount(); - - if (stream.bad()) { - perror("error while reading file"); - return false; - } - if (!toRead) { - break; + std::vector buffer(CHECKSUM_BUFFER_SIZE); + const offset_type checksumPos = header.getChecksumPos(); + offset_type currentPos = 0; + try { + // Read through the logical archive so multipart boundaries and embedded + // file offsets are handled by the reader abstraction. + while (currentPos < checksumPos) { + const auto bytesToRead = static_cast( + std::min(buffer.size(), checksumPos - currentPos) + ); + zimReader->read( + reinterpret_cast(buffer.data()), + offset_t(currentPos), + zsize_t(bytesToRead) + ); + zim_MD5Update(&md5ctx, buffer.data(), bytesToRead); + currentPos += bytesToRead; } - } - - if (toRead) { + } catch (const std::exception& e) { + log_warn("error reading data for checksum: " << e.what()); return false; } diff --git a/src/writer/creator.cpp b/src/writer/creator.cpp index 4c79cdab..6bcf8bda 100644 --- a/src/writer/creator.cpp +++ b/src/writer/creator.cpp @@ -241,8 +241,6 @@ void writeDirentOffsets(BinaryFile& f, const DirentOffsets& direntOffsets) } } -constexpr size_t CHECKSUM_BUFFER_SIZE = 64 * 1024; - void writeChecksum(int fd) { struct zim_MD5_CTX md5ctx; diff --git a/test/archive.cpp b/test/archive.cpp index c2480dbe..0606fdfe 100644 --- a/test/archive.cpp +++ b/test/archive.cpp @@ -1020,6 +1020,27 @@ TEST_F(ZimArchive, openZIMFileMultiPartEmbeddedInAnotherFile) checkEquivalence(archive1, archive2); } } + +TEST_F(ZimArchive, checksumOfMultipartZim) +{ + for (const auto& testfile : getDataFilePath("small.zim", {"noTitleListingV0"})) { + const zim::Archive sourceArchive(testfile.path); + const auto archiveSize = sourceArchive.getFilesize(); + constexpr zim::size_type firstPartSize = 1234; // Deliberately not chunk-aligned. + ASSERT_LT(firstPartSize, archiveSize); + + const int fd = OPEN_READ_ONLY(testfile.path); + const std::vector parts = { + zim::FdInput(fd, 0, firstPartSize), + zim::FdInput(fd, firstPartSize, archiveSize - firstPartSize) + }; + const zim::Archive multipartArchive(parts); + close(fd); + + ASSERT_TRUE(multipartArchive.isMultiPart()); + EXPECT_TRUE(multipartArchive.check()); + } +} #endif // not _WIN32 #endif // WITH_TEST_DATA