Skip to content

Commit

Permalink
[CHECK] Do not continue with other checks if integrity check fails.
Browse files Browse the repository at this point in the history 
It make no sense to check content in a zim archive if the zim archive
itself is broken and we can't read it.
  • Loading branch information
@mgautierfr
mgautierfr committed Jun 10, 2024
1 parent d6c6a1b commit 20dcc12
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 56 deletions.
3 changes: 2 additions & 1 deletion src/zimcheck/checks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ void test_checksum(zim::Archive& archive, ErrorLogger& reporter) {
}
}

void test_integrity(const std::string& filename, ErrorLogger& reporter) {
bool test_integrity(const std::string& filename, ErrorLogger& reporter) {
reporter.infoMsg("[INFO] Verifying ZIM-archive structure integrity...");
zim::IntegrityCheckList checks;
checks.set(); // enable all checks (including checksum)
Expand All @@ -249,6 +249,7 @@ void test_integrity(const std::string& filename, ErrorLogger& reporter) {
if (!result) {
reporter.infoMsg(" [ERROR] ZIM file's low level structure is invalid");
}
return result;
}


Expand Down
2 changes: 1 addition & 1 deletion src/zimcheck/checks.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class ErrorLogger {


void test_checksum(zim::Archive& archive, ErrorLogger& reporter);
void test_integrity(const std::string& filename, ErrorLogger& reporter);
bool test_integrity(const std::string& filename, ErrorLogger& reporter);
void test_metadata(const zim::Archive& archive, ErrorLogger& reporter);
void test_favicon(const zim::Archive& archive, ErrorLogger& reporter);
void test_mainpage(const zim::Archive& archive, ErrorLogger& reporter);
Expand Down
111 changes: 57 additions & 54 deletions src/zimcheck/zimcheck.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,70 +266,73 @@ int zimcheck (const std::vector<const char*>& args)
error.infoMsg("[INFO] Zimcheck version is " + std::string(VERSION));

//Test 0: Low-level ZIM-file structure integrity checks
if(enabled_tests.isEnabled(TestType::INTEGRITY))
test_integrity(filename, error);
bool valid;
if(enabled_tests.isEnabled(TestType::INTEGRITY)) {
valid = test_integrity(filename, error);
}


// Does it make sense to do the other checks if the integrity
// check fails?
zim::Archive archive( filename );
error.addInfo("file_uuid", stringify(archive.getUuid()));
if (valid) {
zim::Archive archive( filename );
error.addInfo("file_uuid", stringify(archive.getUuid()));

//Test 1: Internal Checksum
if(enabled_tests.isEnabled(TestType::CHECKSUM)) {
if ( enabled_tests.isEnabled(TestType::INTEGRITY) ) {
error.infoMsg(
"[INFO] Avoiding redundant checksum test"
" (already performed by the integrity check)."
);
} else {
test_checksum(archive, error);
//Test 1: Internal Checksum
if(enabled_tests.isEnabled(TestType::CHECKSUM)) {
if ( enabled_tests.isEnabled(TestType::INTEGRITY) ) {
error.infoMsg(
"[INFO] Avoiding redundant checksum test"
" (already performed by the integrity check)."
);
} else {
test_checksum(archive, error);
}
}
}

//Test 2: Metadata Entries:
//The file is searched for the compulsory metadata entries.
if(enabled_tests.isEnabled(TestType::METADATA))
test_metadata(archive, error);
//Test 2: Metadata Entries:
//The file is searched for the compulsory metadata entries.
if(enabled_tests.isEnabled(TestType::METADATA))
test_metadata(archive, error);

//Test 3: Test for Favicon.
if(enabled_tests.isEnabled(TestType::FAVICON))
test_favicon(archive, error);
//Test 3: Test for Favicon.
if(enabled_tests.isEnabled(TestType::FAVICON))
test_favicon(archive, error);


//Test 4: Main Page Entry
if(enabled_tests.isEnabled(TestType::MAIN_PAGE))
test_mainpage(archive, error);
//Test 4: Main Page Entry
if(enabled_tests.isEnabled(TestType::MAIN_PAGE))
test_mainpage(archive, error);

/* Now we want to avoid to loop on the tests but on the article.
*
* If we loop of the tests we will have :
*
* for (test: tests) {
* for(article: articles) {
* data = article->getData();
* ...
* }
* }
*
* And so we will get several the data of an article (and so decompression and so).
* By looping on the articles first, we have :
*
* for (article: articles) {
* data = article->getData();
* for (test: tests) {
* ...
* }
* }
*/
/* Now we want to avoid to loop on the tests but on the article.
*
* If we loop of the tests we will have :
*
* for (test: tests) {
* for(article: articles) {
* data = article->getData();
* ...
* }
* }
*
* And so we will get several the data of an article (and so decompression and so).
* By looping on the articles first, we have :
*
* for (article: articles) {
* data = article->getData();
* for (test: tests) {
* ...
* }
* }
*/

if ( enabled_tests.isEnabled(TestType::URL_INTERNAL) ||
enabled_tests.isEnabled(TestType::URL_EXTERNAL) ||
enabled_tests.isEnabled(TestType::REDUNDANT) ||
enabled_tests.isEnabled(TestType::EMPTY) )
test_articles(archive, error, progress, enabled_tests, thread_count);
if ( enabled_tests.isEnabled(TestType::URL_INTERNAL) ||
enabled_tests.isEnabled(TestType::URL_EXTERNAL) ||
enabled_tests.isEnabled(TestType::REDUNDANT) ||
enabled_tests.isEnabled(TestType::EMPTY) )
test_articles(archive, error, progress, enabled_tests, thread_count);

if ( enabled_tests.isEnabled(TestType::REDIRECT))
test_redirect_loop(archive, error);
if ( enabled_tests.isEnabled(TestType::REDIRECT))
test_redirect_loop(archive, error);
}

const bool overallStatus = error.overallStatus();
error.addInfo("status", overallStatus);
Expand Down

0 comments on commit 20dcc12

Please sign in to comment.