From aa6076d78081b0b4ddb3dd27762f65ef3a24155f Mon Sep 17 00:00:00 2001
From: David Davis <daviddavis@redhat.com>
Date: Tue, 26 Nov 2019 15:39:38 -0500
Subject: [PATCH] Add functions to validate file path overlaps

fixes #5559
https://pulp.plan.io/issues/5559
---
 CHANGES/5559.feature               |  1 +
 pulpcore/app/models/publication.py | 52 ++++++++++++++++++++++++++++++
 pulpcore/app/models/repository.py  | 23 +++++++++++--
 3 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 CHANGES/5559.feature

diff --git a/CHANGES/5559.feature b/CHANGES/5559.feature
new file mode 100644
index 00000000000..3eaea9d4ce3
--- /dev/null
+++ b/CHANGES/5559.feature
@@ -0,0 +1 @@
+Add checks to prevent artifacts from having overlapping base paths in repo versions and publications.
diff --git a/pulpcore/app/models/publication.py b/pulpcore/app/models/publication.py
index 4173ec5327d..9b1d1eccbc3 100644
--- a/pulpcore/app/models/publication.py
+++ b/pulpcore/app/models/publication.py
@@ -1,4 +1,7 @@
+from gettext import gettext as _
+
 from django.db import IntegrityError, models, transaction
+from django.db.models import Q
 
 from .base import MasterModel, Model
 from .content import Artifact, Content, ContentArtifact
@@ -107,6 +110,49 @@ def delete(self, **kwargs):
             CreatedResource.objects.filter(object_id=self.pk).delete()
             super().delete(**kwargs)
 
+    def validate_path_overlap(self):
+        """
+        Validate whether any artifact relative paths overlap (e.g. a/b and a/b/c). This checks
+        published artifacts, published metadata, and repo version content (if this publication is a
+        pass_through publication).
+
+        Plugins can override this method if they wish to provide their own validate_path_overlap
+        functionality or if they wish to skip this validation entirely.
+
+        Raises:
+            ValueError: If two artifact relative paths overlap
+        """
+        q = Q()
+        for pa in self.published_artifact.all():
+            q |= Q(relative_path__startswith=f"{pa.relative_path}/")
+
+        for pm in self.published_metadata.all():
+            q |= Q(relative_path__startswith=f"{pm.relative_path}/")
+
+        if self.pass_through:
+            c_artifacts = ContentArtifact.objects. \
+                filter(content__pk__in=self.repository_version.content)
+            for ca in c_artifacts:
+                q |= Q(relative_path__startswith=f"{ca.relative_path}/")
+
+            conflicts = c_artifacts.filter(q).values_list("content__pk", flat=True)
+            if conflicts:
+                content_pks = [str(c) for c in conflicts]
+                raise ValueError(_("Cannot create publication due to file that overlaps path for "
+                                   "Content: {content}").format(content=(", ").join(content_pks)))
+
+        pa_conflicts = self.published_artifact.filter(q).values_list("pk", flat=True)
+        if pa_conflicts:
+            pa_pks = [str(pk) for pk in pa_conflicts]
+            raise ValueError(_("Cannot create publication due to file that overlaps path for "
+                               "PublishedArtifact: {pa}").format(pa=(", ").join(pa_pks)))
+
+        pm_conflicts = self.published_metadata.filter(q).values_list("pk", flat=True)
+        if pm_conflicts:
+            pm_pks = [str(pk) for pk in pm_conflicts]
+            raise ValueError(_("Cannot create publication due to file that overlaps path for "
+                               "PublishedMetadata: {pm}").format(pm=(", ").join(pm_pks)))
+
     def __enter__(self):
         """
         Enter context.
@@ -125,6 +171,12 @@ def __exit__(self, exc_type, exc_val, exc_tb):
             exc_val (Exception): (optional) Instance of exception raised.
             exc_tb (types.TracebackType): (optional) stack trace.
         """
+        try:
+            self.validate_path_overlap()
+        except ValueError:
+            self.delete()
+            raise
+
         if not exc_val:
             self.complete = True
             self.save()
diff --git a/pulpcore/app/models/repository.py b/pulpcore/app/models/repository.py
index 0ca1195d32f..bc31262cfac 100644
--- a/pulpcore/app/models/repository.py
+++ b/pulpcore/app/models/repository.py
@@ -8,6 +8,7 @@
 
 import django
 from django.db import models, transaction
+from django.db.models import Q
 from django.urls import reverse
 
 from pulpcore.app.util import get_view_name_for_model
@@ -15,7 +16,7 @@
 from pulpcore.exceptions import ResourceImmutableError
 
 from .base import MasterModel, Model
-from .content import Artifact, Content
+from .content import Artifact, Content, ContentArtifact
 from .task import CreatedResource, Task
 
 
@@ -128,7 +129,7 @@ def finalize_new_version(self, new_version):
         Returns:
 
         """
-        pass
+        new_version.validate_path_overlap()
 
     def latest_version(self):
         """
@@ -654,6 +655,24 @@ def _compute_counts(self):
                     counts_list.append(count_obj)
             RepositoryVersionContentDetails.objects.bulk_create(counts_list)
 
+    def validate_path_overlap(self):
+        """
+        Validate whether any artifact relative paths overlap (e.g. a/b and a/b/c)
+
+        Raises:
+            ValueError: If two artifact relative paths overlap
+        """
+        q = Q()
+        c_artifacts = ContentArtifact.objects.filter(content__pk__in=self.content)
+        for ca in c_artifacts:
+            q |= Q(relative_path__startswith=f"{ca.relative_path}/")
+
+        conflicts = c_artifacts.filter(q).values_list("content__pk", flat=True)
+        if conflicts:
+            content_pks = [str(pk) for pk in conflicts]
+            raise ValueError(_("Cannot create repo version due to file that overlaps path for "
+                               "Content: {content}").format(content=(", ".join(content_pks))))
+
     def __enter__(self):
         """
         Create the repository version