fix: escape and batch the fixed packages rendering in the vulnerabilities tab #179

component_catalog/models.py

@@ -7,8 +7,10 @@
 #
 
 import logging
+import operator
 import re
 from contextlib import suppress
+from functools import reduce
 from urllib.parse import quote_plus
 
 from django.contrib.postgres.fields import ArrayField
@@ -47,6 +49,7 @@
 from packageurl.contrib import url2purl
 from packageurl.contrib.django.models import PackageURLMixin
 from packageurl.contrib.django.models import PackageURLQuerySetMixin
+from packageurl.contrib.django.utils import purl_to_lookups
 from packageurl.contrib.django.utils import without_empty_values
 
 from component_catalog.license_expression_dje import build_licensing
@@ -1761,6 +1764,18 @@ def has_download_url(self):
         """Return objects with download URL defined."""
         return self.filter(~models.Q(download_url=""))
 
+    def for_package_urls(self, package_urls):
+        """Return objects matching any of the provided Package URL strings."""
+        package_url_filters = [
+            models.Q(**lookups)
+            for package_url in package_urls
+            if (lookups := purl_to_lookups(package_url))
+        ]
+        if not package_url_filters:
+            return self.none()
+
+        return self.filter(reduce(operator.or_, package_url_filters))
+
     def annotate_sortable_identifier(self):
         """
         Annotate the QuerySet with a `sortable_identifier` value that combines

component_catalog/tests/test_models.py

@@ -2903,3 +2903,28 @@ def test_package_queryset_annotate_plain_package_url(self):
         qs = Package.objects.annotate_plain_package_url()
         self.assertEqual("pkg:pypi/django@5.0", qs.get(pk=package1.pk).plain_purl)
         self.assertEqual("", qs.get(pk=package2.pk).plain_purl)
+
+    def test_package_queryset_for_package_urls(self):
+        django_package = make_package(self.dataspace, package_url="pkg:pypi/django@5.0")
+        django_qualified_package = make_package(
+            self.dataspace, package_url="pkg:pypi/django@5.0?qualifier=true"
+        )
+        idna_package = make_package(self.dataspace, package_url="pkg:pypi/idna@3.7")
+        make_package(self.dataspace, package_url="pkg:pypi/django@4.2")
+        make_package(self.dataspace)
+
+        qs = Package.objects.for_package_urls(["pkg:pypi/django@5.0", "pkg:pypi/idna@3.7"])
+        expected = [django_package, django_qualified_package, idna_package]
+        self.assertQuerySetEqual(qs, expected, ordered=False)
+
+        qs = Package.objects.for_package_urls(["pkg:pypi/django@5.0?qualifier=true"])
+        self.assertQuerySetEqual(qs, [django_qualified_package])
+
+    def test_package_queryset_for_package_urls_invalid_and_empty(self):
+        idna_package = make_package(self.dataspace, package_url="pkg:pypi/idna@3.7")
+
+        self.assertQuerySetEqual(Package.objects.for_package_urls([]), [])
+        self.assertQuerySetEqual(Package.objects.for_package_urls(["invalid"]), [])
+
+        qs = Package.objects.for_package_urls(["invalid", "pkg:pypi/idna@3.7"])
+        self.assertQuerySetEqual(qs, [idna_package])

component_catalog/tests/test_views.py

@@ -16,10 +16,12 @@
 from django.contrib.auth import get_user_model
 from django.contrib.contenttypes.models import ContentType
 from django.contrib.sessions.backends.base import SessionBase
+from django.db import connection
 from django.http.response import Http404
 from django.test import TestCase
 from django.test import TransactionTestCase
 from django.test.client import RequestFactory
+from django.test.utils import CaptureQueriesContext
 from django.test.utils import override_settings
 from django.urls import reverse
 from django.utils import timezone
@@ -44,6 +46,7 @@
 from component_catalog.views import ComponentListView
 from component_catalog.views import PackageListView
 from component_catalog.views import PackageTabScanView
+from component_catalog.views import get_packages_for_package_url
 from dejacode_toolkit.scancodeio import get_webhook_url
 from dejacode_toolkit.vulnerablecode import VulnerableCode
 from dejacode_toolkit.vulnerablecode import get_plain_purls
@@ -3173,7 +3176,78 @@ def test_package_details_view_tab_vulnerabilities_fixed_by_packages(self):
         self.assertContains(response, fixing_package.get_absolute_url())
         # An unknown package offers an "Add Package" link instead.
         self.assertContains(response, "idna@9.9.9")
-        self.assertContains(response, "package_url=pkg:pypi/idna@9.9.9")
+        self.assertContains(response, "package_url=pkg%3Apypi%2Fidna%409.9.9")
+
+    def test_package_details_view_tab_vulnerabilities_fixed_by_packages_escaped(self):
+        self.vulnerability1.fixed_by_packages = ["pkg:pypi/idna@<img src=x onerror=alert(1)>"]
+        self.vulnerability1.save()
+
+        self.client.login(username=self.super_user.username, password="secret")
+        response = self.client.get(self.package1.details_url)
+
+        self.assertNotContains(response, "<img src=x onerror=alert(1)>")
+        self.assertContains(response, "pypi/idna@&lt;img src=x onerror=alert(1)&gt;")
+
+    def test_package_details_view_tab_vulnerabilities_fixed_by_packages_qualifiers(self):
+        self.vulnerability1.fixed_by_packages = [
+            "pkg:deb/debian/curl@7.50.3-1?arch=i386&distro=jessie"
+        ]
+        self.vulnerability1.save()
+
+        self.client.login(username=self.super_user.username, password="secret")
+        response = self.client.get(self.package1.details_url)
+
+        expected = (
+            "package_url=pkg%3Adeb%2Fdebian%2Fcurl%407.50.3-1%3Farch%3Di386%26distro%3Djessie"
+        )
+        self.assertContains(response, expected)
+
+    def test_package_details_view_tab_vulnerabilities_fixed_by_packages_num_queries(self):
+        make_package(self.dataspace, package_url="pkg:pypi/idna@3.7")
+        self.vulnerability1.fixed_by_packages = ["pkg:pypi/idna@3.7"]
+        self.vulnerability1.save()
+
+        self.client.login(username=self.super_user.username, password="secret")
+        # Warm up the caches so both measured requests start from the same state.
+        self.client.get(self.package1.details_url)
+        with CaptureQueriesContext(connection) as single_fixed_package_queries:
+            self.client.get(self.package1.details_url)
+
+        make_package(self.dataspace, package_url="pkg:pypi/idna@3.8")
+        make_package(self.dataspace, package_url="pkg:pypi/idna@3.9")
+        self.vulnerability1.fixed_by_packages = [
+            "pkg:pypi/idna@3.7",
+            "pkg:pypi/idna@3.8",
+            "pkg:pypi/idna@3.9",
+            "pkg:pypi/idna@9.9.9",
+        ]
+        self.vulnerability1.save()
+        with CaptureQueriesContext(connection) as multiple_fixed_packages_queries:
+            self.client.get(self.package1.details_url)
+
+        self.assertEqual(len(single_fixed_package_queries), len(multiple_fixed_packages_queries))
+
+    def test_package_views_get_packages_for_package_url(self):
+        django_package = make_package(self.dataspace, package_url="pkg:pypi/django@5.0")
+        django_qualified_package = make_package(
+            self.dataspace, package_url="pkg:pypi/django@5.0?qualifier=true"
+        )
+        django_old_package = make_package(self.dataspace, package_url="pkg:pypi/django@4.2")
+        packages = [django_package, django_qualified_package, django_old_package]
+
+        matched_packages = get_packages_for_package_url(packages, "pkg:pypi/django@5.0")
+        self.assertEqual([django_package, django_qualified_package], matched_packages)
+
+        matched_packages = get_packages_for_package_url(
+            packages, "pkg:pypi/django@5.0?qualifier=true"
+        )
+        self.assertEqual([django_qualified_package], matched_packages)
+
+        matched_packages = get_packages_for_package_url(packages, "pkg:pypi/django")
+        self.assertEqual(packages, matched_packages)
+
+        self.assertEqual([], get_packages_for_package_url(packages, "invalid"))
+        self.assertEqual([], get_packages_for_package_url([], "pkg:pypi/django@5.0"))
 
     def test_package_details_view_tab_vulnerabilities_curation_badges(self):
         DataspaceConfiguration.objects.create(

component_catalog/views.py

@@ -34,6 +34,7 @@
 from django.utils.html import escape
 from django.utils.html import format_html
 from django.utils.html import mark_safe
+from django.utils.http import urlencode
 from django.utils.text import normalize_newlines
 from django.utils.translation import gettext
 from django.utils.translation import gettext_lazy as _
@@ -46,6 +47,7 @@
 
 from crispy_forms.utils import render_crispy_form
 from packageurl import PackageURL
+from packageurl.contrib.django.utils import purl_to_lookups
 
 from component_catalog.filters import ComponentFilterSet
 from component_catalog.filters import PackageFilterSet
@@ -253,6 +255,19 @@ def include_type(view_instance):
     return view_instance.dataspace.show_type_in_component_list_view
 
 
+def get_packages_for_package_url(packages, package_url):
+    """Return the `packages` matching `package_url`, same as `for_package_url`."""
+    lookups = purl_to_lookups(package_url)
+    if not lookups:
+        return []
+
+    return [
+        package
+        for package in packages
+        if all(getattr(package, field_name) == value for field_name, value in lookups.items())
+    ]
+
+
 class TabVulnerabilityMixin:
     template = "component_catalog/tabs/tab_vulnerabilities.html"
 
@@ -267,9 +282,20 @@ def tab_vulnerabilities(self):
             f"{len(vulnerabilities_qs)}</span>"
         )
 
+        fixed_by_package_urls = {
+            package_url
+            for vulnerability in vulnerabilities_qs
+            for package_url in vulnerability.fixed_by_packages
+        }
+        fixed_packages = list(
+            Package.objects.scope(self.object.dataspace)
+            .for_package_urls(fixed_by_package_urls)
+            .only_rendering_fields()
+        )
+
         vulnerabilities = []
         for vulnerability in vulnerabilities_qs:
-            fixed_packages_html = self.get_fixed_packages_html(vulnerability, self.object.dataspace)
+            fixed_packages_html = self.get_fixed_packages_html(vulnerability, fixed_packages)
             vulnerability.fixed_packages_html = fixed_packages_html
             vulnerabilities.append(vulnerability)
 
@@ -283,7 +309,7 @@ def tab_vulnerabilities(self):
             "label": mark_safe(label),
         }
 
-    def get_fixed_packages_html(self, vulnerability, dataspace):
+    def get_fixed_packages_html(self, vulnerability, fixed_packages):
         if not vulnerability.fixed_by_packages:
             return
 
@@ -292,7 +318,7 @@ def get_fixed_packages_html(self, vulnerability, dataspace):
 
         fixed_packages_values = []
         for purl in fixed_packages_sorted:
-            package_instances = Package.objects.scope(dataspace).for_package_url(purl)
+            package_instances = get_packages_for_package_url(fixed_packages, purl)
 
             for package in package_instances:
                 display_value = package.get_html_link(href=package.get_absolute_url())
@@ -301,15 +327,18 @@ def get_fixed_packages_html(self, vulnerability, dataspace):
             if not package_instances:
                 # Warning: do not add spaces between HTML elements as this content
                 # is displayed in a <pre>
-                display_value = (
-                    f"{purl.replace('pkg:', '')}"
-                    f'<a href="{add_package_url}?package_url={purl}"'
-                    f'   class="ms-1" target="_blank">'
-                    f'<span data-bs-toggle="tooltip" title="Add Package"'
-                    f'      data-boundary="viewport">'
-                    f'<i class="fas fa-plus-circle"></i>'
-                    f"</span>"
-                    f"</a>"
+                display_value = format_html(
+                    "{}"
+                    '<a href="{}?{}"'
+                    '   class="ms-1" target="_blank">'
+                    '<span data-bs-toggle="tooltip" title="Add Package"'
+                    '      data-boundary="viewport">'
+                    '<i class="fas fa-plus-circle"></i>'
+                    "</span>"
+                    "</a>",
+                    purl.replace("pkg:", ""),
+                    add_package_url,
+                    urlencode({"package_url": purl}),
                 )
                 fixed_packages_values.append(display_value)