Address review comments

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

Author: TG1999

vulnerabilities/import_runner.py

@@ -59,24 +59,26 @@ def run(self) -> None:
         logger.info(f"Finished import for {importer_name}. Imported {count} advisories.")
 
     def do_import(self, advisories) -> None:
-        improver = AdvisoryBasedDefaultImprover(advisories=advisories)
-        logger.info(f"Running improver: {improver.qualified_name}")
-        improver_name = improver.qualified_name
+        advisory_importer = AdvisoryBasedDefaultImprover(advisories=advisories)
+        logger.info(f"Running improver: {advisory_importer.qualified_name}")
+        improver_name = advisory_importer.qualified_name
         advisories = []
-        for advisory in improver.interesting_advisories:
+        for advisory in advisory_importer.interesting_advisories:
             if advisory.date_imported:
                 continue
             logger.info(f"Processing advisory: {advisory!r}")
             try:
-                inferences = improver.get_inferences(advisory_data=advisory.to_advisory_data())
+                inferences = advisory_importer.get_inferences(
+                    advisory_data=advisory.to_advisory_data()
+                )
                 process_inferences(
                     inferences=inferences,
                     advisory=advisory,
                     improver_name=improver_name,
                 )
             except Exception as e:
                 logger.info(f"Failed to process advisory: {advisory!r} with error {e!r}")
-        logger.info("Finished improving using %s.", improver.__class__.qualified_name)
+        logger.info("Finished improving using %s.", advisory_importer.__class__.qualified_name)
 
     def process_advisories(
         self, advisory_datas: Iterable[AdvisoryData], importer_name: str
@@ -128,13 +130,13 @@ def process_advisories(
 def process_inferences(inferences: List[Inference], advisory: Advisory, improver_name: str):
     """
     Return number of inferences processed.
-    An atomic transaction that updates both the Advisory (e.g. date_improved)
+    An atomic transaction that updates both the Advisory (e.g. date_imported)
     and processes the given inferences to create or update corresponding
     database fields.
 
     This avoids failing the entire improver when only a single inference is
     erroneous. Also, the atomic transaction for every advisory and its
-    inferences makes sure that date_improved of advisory is consistent.
+    inferences makes sure that date_imported of advisory is consistent.
     """
     inferences_processed_count = 0
 

vulnerabilities/importer.py

@@ -252,13 +252,15 @@ class AdvisoryData:
     def __post_init__(self):
         if self.date_published and not self.date_published.tzinfo:
             logger.warning(f"AdvisoryData with no tzinfo: {self!r}")
-        self.clean_summary()
+        self.summary = self.clean_summary(self.summary)
 
-    def clean_summary(self):
+    def clean_summary(self, summary):
         # https://nvd.nist.gov/vuln/detail/CVE-2013-4314
         # https://github.com/cms-dev/cms/issues/888#issuecomment-516977572
-        if self.summary:
-            self.summary = self.summary.replace("\x00", "\uFFFD")
+        summary = summary.strip()
+        if summary:
+            summary = summary.replace("\x00", "\uFFFD")
+        return summary
 
     def to_dict(self):
         return {

vulnerabilities/improve_runner.py

@@ -60,13 +60,13 @@ def run(self) -> None:
 def process_inferences(inferences: List[Inference], advisory: Advisory, improver_name: str):
     """
     Return number of inferences processed.
-    An atomic transaction that updates both the Advisory (e.g. date_improved)
+    An atomic transaction that updates both the Advisory (e.g. date_imported)
     and processes the given inferences to create or update corresponding
     database fields.
 
     This avoids failing the entire improver when only a single inference is
     erroneous. Also, the atomic transaction for every advisory and its
-    inferences makes sure that date_improved of advisory is consistent.
+    inferences makes sure that date_imported of advisory is consistent.
     """
     inferences_processed_count = 0
 
@@ -150,7 +150,7 @@ def process_inferences(inferences: List[Inference], advisory: Advisory, improver
 
         inferences_processed_count += 1
 
-    advisory.date_improved = datetime.now(timezone.utc)
+    advisory.date_imported = datetime.now(timezone.utc)
     advisory.save()
     return inferences_processed_count
 

vulnerabilities/tests/test_import_runner.py

@@ -10,6 +10,7 @@
 from datetime import datetime
 from datetime import timezone
 
+import pytest
 from univers.version_range import VersionRange
 
 from vulnerabilities import models
@@ -43,6 +44,7 @@ def advisory_data(self):
         return ADVISORY_DATAS
 
 
+@pytest.mark.django_db(transaction=True)
 def test_import_runner(db):
     runner = ImportRunner(DummyImporter)
     runner.run()
@@ -51,18 +53,21 @@ def test_import_runner(db):
     assert advisory_datas == ADVISORY_DATAS
 
 
+@pytest.mark.django_db(transaction=True)
 def test_process_advisories_with_no_advisory(db):
     ImportRunner(DummyImporter).process_advisories([], "")
     assert 0 == models.Advisory.objects.count()
 
 
+@pytest.mark.django_db(transaction=True)
 def test_process_advisories_with_advisories(db):
     ImportRunner(DummyImporter).process_advisories(ADVISORY_DATAS, "test_importer")
     advisories = models.Advisory.objects.all()
     advisory_datas = [x.to_advisory_data() for x in advisories]
     assert advisory_datas == ADVISORY_DATAS
 
 
+@pytest.mark.django_db(transaction=True)
 def test_process_advisories_idempotency(db):
     ImportRunner(DummyImporter).process_advisories(ADVISORY_DATAS, "test_importer")
     ImportRunner(DummyImporter).process_advisories(ADVISORY_DATAS, "test_importer")
@@ -72,6 +77,7 @@ def test_process_advisories_idempotency(db):
     assert advisory_datas == ADVISORY_DATAS
 
 
+@pytest.mark.django_db(transaction=True)
 def test_process_advisories_idempotency_with_one_new_advisory(db):
     advisory_datas = ADVISORY_DATAS.copy()
     ImportRunner(DummyImporter).process_advisories(advisory_datas, "test_importer")
@@ -86,9 +92,17 @@ def test_process_advisories_idempotency_with_one_new_advisory(db):
     assert advisory_datas_in_db == advisory_datas
 
 
-def test_process_advisories_idempotency_with_different_importer_names(db):
+@pytest.mark.django_db(transaction=True)
+def test_process_advisories_idempotency_with_different_importer_names():
     ImportRunner(DummyImporter).process_advisories(ADVISORY_DATAS, "test_importer_one")
     ImportRunner(DummyImporter).process_advisories(ADVISORY_DATAS, "test_importer_two")
     advisories = models.Advisory.objects.all()
     advisory_datas = [x.to_advisory_data() for x in advisories]
     assert advisory_datas == ADVISORY_DATAS
+
+
+def test_advisory_summary_clean_up():
+    adv = AdvisoryData(
+        summary="The X509Extension in pyOpenSSL before 0.13.1 does not properly handle a '\x00' character in a domain name in the Subject Alternative Name field of an X.509 certificate, which allows man-in-the-middle attackers to spoof arbitrary SSL servers via a crafted certificate issued by a legitimate Certification Authority."
+    )
+    assert '\x00' not in adv.summary