Merge pull request #40 from petrs/cc_serialize_error_messages

Error messages of certificate are now serialized

6 files changed, 63 insertions, 24 deletions

diff --git a/sec_certs/certificate.py b/sec_certs/certificate.py
index 837e16bb..31675bf3 100644
--- a/sec_certs/certificate.py
+++ b/sec_certs/certificate.py
@@ -810,10 +810,12 @@ class CommonCriteriaCert(Certificate, ComplexSerializableType):
         report_pdf_path: Path
         st_txt_path: Path
         report_txt_path: Path
+        errors: Optional[List[str]]
 
         def __init__(self, st_link_ok: bool = True, report_link_ok: bool = True,
                      st_convert_ok: bool = True, report_convert_ok: bool = True,
-                     st_extract_ok: bool = True, report_extract_ok: bool = True):
+                     st_extract_ok: bool = True, report_extract_ok: bool = True,
+                     errors: Optional[List[str]] = None):
             self.st_link_ok = st_link_ok
             self.report_link_ok = report_link_ok
             self.st_convert_ok = st_convert_ok
@@ -821,10 +823,16 @@ class CommonCriteriaCert(Certificate, ComplexSerializableType):
             self.st_extract_ok = st_extract_ok
             self.report_extract_ok = report_extract_ok
 
+            if errors is None:
+                self.errors = []
+            else:
+                self.errors = errors
+
         def to_dict(self):
             return {'st_link_ok': self.st_link_ok, 'report_link_ok': self.report_link_ok,
                     'st_convert_ok': self.st_convert_ok, 'report_convert_ok': self.report_convert_ok,
-                    'st_extract_ok': self.st_extract_ok, 'report_extract_ok': self.report_extract_ok}
+                    'st_extract_ok': self.st_extract_ok, 'report_extract_ok': self.report_extract_ok,
+                    'errors': self.errors}
 
         @classmethod
         def from_dict(cls, dct: Dict[str, bool]):
@@ -1064,16 +1072,20 @@ class CommonCriteriaCert(Certificate, ComplexSerializableType):
     def download_pdf_report(cert: 'CommonCriteriaCert') -> 'CommonCriteriaCert':
         exit_code = helpers.download_file(cert.report_link, cert.state.report_pdf_path)
         if exit_code != requests.codes.ok:
-            logger.error(f'Cert dgst: {cert.dgst} failed to download report from {cert.report_link}, code: {exit_code}')
+            error_msg = f'failed to download report from {cert.report_link}, code: {exit_code}'
+            logger.error(f'Cert dgst: {cert.dgst} ' + error_msg)
             cert.state.report_link_ok = False
+            cert.state.errors.append(error_msg)
         return cert
 
     @staticmethod
     def download_pdf_target(cert: 'CommonCriteriaCert') -> 'CommonCriteriaCert':
         exit_code = helpers.download_file(cert.st_link, cert.state.st_pdf_path)
         if exit_code != requests.codes.ok:
-            logger.error(f'Cert dgst: {cert.dgst} failed to download ST from {cert.report_link}, code: {exit_code}')
+            error_msg = f'failed to download ST from {cert.report_link}, code: {exit_code}'
+            logger.error(f'Cert dgst: {cert.dgst}' + error_msg)
             cert.state.st_link_ok = False
+            cert.state.errors.append(error_msg)
         return cert
 
     def path_is_corrupted(self, local_path):
@@ -1083,16 +1095,20 @@ class CommonCriteriaCert(Certificate, ComplexSerializableType):
     def convert_report_pdf(cert: 'CommonCriteriaCert') -> 'CommonCriteriaCert':
         exit_code = helpers.convert_pdf_file(cert.state.report_pdf_path, cert.state.report_txt_path, ['-raw'])
         if exit_code != constants.RETURNCODE_OK:
-            logger.error(f'Cert dgst: {cert.dgst} failed to convert report pdf->txt')
+            error_msg = 'failed to convert report pdf->txt'
+            logger.error(f'Cert dgst: {cert.dgst}' + error_msg)
             cert.state.report_convert_ok = False
+            cert.state.errors.append(error_msg)
         return cert
 
     @staticmethod
     def convert_target_pdf(cert: 'CommonCriteriaCert') -> 'CommonCriteriaCert':
         exit_code = helpers.convert_pdf_file(cert.state.st_pdf_path, cert.state.st_txt_path, ['-raw'])
         if exit_code != constants.RETURNCODE_OK:
-            logger.error(f'Cert dgst: {cert.dgst} failed to convert security target pdf->txt')
+            error_msg = 'failed to convert security target pdf->txt'
+            logger.error(f'Cert dgst: {cert.dgst}' + error_msg)
             cert.state.st_convert_ok = False
+            cert.state.errors.append(error_msg)
         return cert
 
     @staticmethod
@@ -1100,6 +1116,7 @@ class CommonCriteriaCert(Certificate, ComplexSerializableType):
         response, cert.pdf_data.st_metadata = helpers.extract_pdf_metadata(cert.state.st_pdf_path)
         if response != constants.RETURNCODE_OK:
             cert.state.st_extract_ok = False
+            cert.state.errors.append(response)
         return cert
 
     @staticmethod
@@ -1107,17 +1124,22 @@ class CommonCriteriaCert(Certificate, ComplexSerializableType):
         response, cert.pdf_data.report_metadata = helpers.extract_pdf_metadata(cert.state.report_pdf_path)
         if response != constants.RETURNCODE_OK:
             cert.state.report_extract_ok = False
+            cert.state.errors.append(response)
         return cert
 
     @staticmethod
     def extract_st_pdf_frontpage(cert: 'CommonCriteriaCert') -> 'CommonCriteriaCert':
         cert.pdf_data.st_frontpage = dict()
 
-        response_bsi, cert.pdf_data.st_frontpage['bsi'] = helpers.search_only_headers_bsi(cert.state.st_txt_path)
         response_anssi, cert.pdf_data.st_frontpage['anssi'] = helpers.search_only_headers_anssi(cert.state.st_txt_path)
+        response_bsi, cert.pdf_data.st_frontpage['bsi'] = helpers.search_only_headers_bsi(cert.state.st_txt_path)
 
-        if response_anssi != constants.RETURNCODE_OK or response_bsi != constants.RETURNCODE_OK:
+        if response_anssi != constants.RETURNCODE_OK:
+            cert.state.st_extract_ok = False
+            cert.state.errors.append(response_anssi)
+        if response_bsi != constants.RETURNCODE_OK:
             cert.state.st_extract_ok = False
+            cert.state.errors.append(response_bsi)
 
         return cert
 
@@ -1127,8 +1149,12 @@ class CommonCriteriaCert(Certificate, ComplexSerializableType):
         response_bsi, cert.pdf_data.report_frontpage['bsi'] = helpers.search_only_headers_bsi(cert.state.report_txt_path)
         response_anssi, cert.pdf_data.report_frontpage['anssi'] = helpers.search_only_headers_anssi(cert.state.report_txt_path)
 
-        if response_anssi != constants.RETURNCODE_OK or response_bsi != constants.RETURNCODE_OK:
+        if response_anssi != constants.RETURNCODE_OK:
+            cert.state.report_extract_ok = False
+            cert.state.errors.append(response_anssi)
+        if response_bsi != constants.RETURNCODE_OK:
             cert.state.report_extract_ok = False
+            cert.state.errors.append(response_bsi)
 
         return cert
 
@@ -1144,5 +1170,6 @@ class CommonCriteriaCert(Certificate, ComplexSerializableType):
         response, cert.pdf_data.st_keywords = helpers.extract_keywords(cert.state.st_txt_path)
         if response != constants.RETURNCODE_OK:
             cert.state.st_extract_ok = False
+            cert.state.errors.append(response)
         return cert
 
diff --git a/sec_certs/constants.py b/sec_certs/constants.py
index 7f63d7e9..18c9998c 100644
--- a/sec_certs/constants.py
+++ b/sec_certs/constants.py
@@ -2,8 +2,8 @@ from enum import Enum
 
 N_THREADS = 8
 RESPONSE_OK = 200
-RETURNCODE_OK = 0
-RETURNCODE_NOK = -1
+RETURNCODE_OK = 'ok'
+RETURNCODE_NOK = 'nok'
 REQUEST_TIMEOUT = 5
 
 MIN_CORRECT_CERT_SIZE = 5000
diff --git a/sec_certs/dataset.py b/sec_certs/dataset.py
index 56cd3a1b..1f5189a4 100644
--- a/sec_certs/dataset.py
+++ b/sec_certs/dataset.py
@@ -461,10 +461,12 @@ class CCDataset(Dataset, ComplexSerializableType):
 
     def _download_targets(self, fresh=True):
         self.targets_pdf_dir.mkdir(parents=True, exist_ok=True)
+
         if fresh is True:
             certs_to_process = self.certs.values()
         else:
             certs_to_process = [x for x in self.certs.values() if not x.state.st_link_ok]
+
         cert_processing.process_parallel(CommonCriteriaCert.download_pdf_target, certs_to_process, constants.N_THREADS)
 
     def download_all_pdfs(self, fresh: bool = True):
diff --git a/sec_certs/helpers.py b/sec_certs/helpers.py
index 22d2e272..7a3bc78a 100644
--- a/sec_certs/helpers.py
+++ b/sec_certs/helpers.py
@@ -173,8 +173,11 @@ def repair_pdf(file: Path):
 
 
 def convert_pdf_file(pdf_path: Path, txt_path: Path, options):
-    return subprocess.run(['pdftotext', *options, pdf_path, txt_path], stdout=subprocess.DEVNULL,
-                          stderr=subprocess.DEVNULL, timeout=60).returncode
+    response = subprocess.run(['pdftotext', *options, pdf_path, txt_path], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, timeout=60).returncode
+    if response == 0:
+        return constants.RETURNCODE_OK
+    else:
+        return constants.RETURNCODE_NOK
 
 
 def extract_pdf_metadata(filepath: Path):
@@ -192,8 +195,9 @@ def extract_pdf_metadata(filepath: Path):
                 metadata[key] = str(val)
 
     except Exception as e:
-        logger.error(f'Failed to read metadata of {filepath}, error: {e}')
-        return constants.RETURNCODE_NOK, None
+        error_msg = f'Failed to read metadata of {filepath}, error: {e}'
+        logger.error(error_msg)
+        return error_msg, None
 
     return constants.RETURNCODE_OK, metadata
 
@@ -368,8 +372,9 @@ def search_only_headers_anssi(filepath: Path):
                 items_found[constants.TAG_CERT_LAB] = extract_certificates.normalize_match_string(match_groups[index_next_item])
                 index_next_item += 1
     except Exception as e:
-        logger.error(f'Failed to parse ANSSI frontpage headers from {filepath}; {e}')
-        return constants.RETURNCODE_NOK, None
+        error_msg = f'Failed to parse ANSSI frontpage headers from {filepath}; {e}'
+        logger.error(error_msg)
+        return error_msg, None
 
     # if True:
     #     print('# hits for rule')
@@ -468,8 +473,9 @@ def search_only_headers_bsi(filepath: Path):
         # print('Total no hits files: {}'.format(len(files_without_match)))
         # print('\n**********************************')
     except Exception as e:
-        logger.error(f'Failed to parse BSI headers from frontpage: {filepath}; {e}')
-        return constants.RETURNCODE_NOK, None
+        error_msg = f'Failed to parse BSI headers from frontpage: {filepath}; {e}'
+        logger.error(error_msg)
+        return error_msg, None
 
     return constants.RETURNCODE_OK, items_found
 
@@ -477,6 +483,7 @@ def extract_keywords(filepath: Path) -> Tuple[int, Optional[Dict[str, str]]]:
     try:
         result = extract_certificates.parse_cert_file(filepath, cc_search_rules, -1, extract_certificates.LINE_SEPARATOR)[0]
     except Exception as e:
-        logger.error(f'Failed to parse keywords from: {filepath}; {e}')
-        return constants.RETURNCODE_NOK, None
+        error_msg = f'Failed to parse keywords from: {filepath}; {e}'
+        logger.error(error_msg)
+        return error_msg, None
     return constants.RETURNCODE_OK, result
\ No newline at end of file
diff --git a/test/data/test_cc_oop/fictional_cert.json b/test/data/test_cc_oop/fictional_cert.json
index 33a1aafb..db00b77c 100644
--- a/test/data/test_cc_oop/fictional_cert.json
+++ b/test/data/test_cc_oop/fictional_cert.json
@@ -39,7 +39,8 @@
         "st_convert_ok": true,
         "report_convert_ok": true,
         "st_extract_ok": true,
-        "report_extract_ok": true
+        "report_extract_ok": true,
+        "errors": []
     },
     "pdf_data": {
         "_type": "PdfData",
diff --git a/test/data/test_cc_oop/toy_dataset.json b/test/data/test_cc_oop/toy_dataset.json
index aaafb53c..2ad2cb35 100644
--- a/test/data/test_cc_oop/toy_dataset.json
+++ b/test/data/test_cc_oop/toy_dataset.json
@@ -42,7 +42,8 @@
                 "st_convert_ok": true,
                 "report_convert_ok": true,
                 "st_extract_ok": true,
-                "report_extract_ok": true
+                "report_extract_ok": true,
+                "errors": []
             },
             "pdf_data": {
                 "_type": "PdfData",
@@ -85,7 +86,8 @@
                 "st_convert_ok": true,
                 "report_convert_ok": true,
                 "st_extract_ok": true,
-                "report_extract_ok": true
+                "report_extract_ok": true,
+                "errors": []
             },
             "pdf_data": {
                 "_type": "PdfData",