From c9aed194615640e00750814320839bcf3e2a0f9b Mon Sep 17 00:00:00 2001
From: Elia Migliore <elia.migliore@aiven.io>
Date: Mon, 18 Sep 2023 13:33:52 +0200
Subject: [PATCH] fix: prevent from accepting schema that are not uniquely
 identifiable from the current parser

---
 karapace/protobuf/dependency.py             | 13 ++++++--
 karapace/protobuf/schema.py                 |  4 +++
 tests/unit/protobuf/test_protobuf_schema.py | 34 +++++++++++++++++++++
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/karapace/protobuf/dependency.py b/karapace/protobuf/dependency.py
index c04bc332f..90118092a 100644
--- a/karapace/protobuf/dependency.py
+++ b/karapace/protobuf/dependency.py
@@ -11,13 +11,22 @@
 from typing import List
 
 
+class FieldNotUniquelyIdentifiableException(Exception):
+    pass
+
+
 class ProtobufDependencyVerifier:
     def __init__(self) -> None:
         self.declared_types: List[str] = []
         self.used_types: List[str] = []
         self.import_path: List[str] = []
 
-    def add_declared_type(self, full_name: str) -> None:
+    def add_declared_type(self, full_name: str, uniquely: bool = False) -> None:
+        if uniquely and full_name in self.declared_types:
+            raise FieldNotUniquelyIdentifiableException(
+                f"{full_name} is not currently identifiable from the parser, "
+                f"validating this message lead to break the schema evolution!"
+            )
         self.declared_types.append(full_name)
 
     def add_used_type(self, parent: str, element_type: str) -> None:
@@ -42,7 +51,7 @@ def is_type_declared(
         delimiter,
         father_child_type,
         used_type_with_scope,
-    ):
+    ) -> bool:
         return (
             used_type in declared_index
             or (delimiter != -1 and used_type_with_scope in declared_index)
diff --git a/karapace/protobuf/schema.py b/karapace/protobuf/schema.py
index 576db1182..001feeb18 100644
--- a/karapace/protobuf/schema.py
+++ b/karapace/protobuf/schema.py
@@ -161,6 +161,10 @@ def _process_nested_type(
     ):
         verifier.add_declared_type(package_name + "." + parent_name + "." + element_type.name)
         verifier.add_declared_type(parent_name + "." + element_type.name)
+        first_ancestral = parent_name.find(".")
+        if first_ancestral != -1:
+            # adding the first father and the type name, this should be unique to identify which is which.
+            verifier.add_declared_type(parent_name[:first_ancestral] + "." + element_type.name, uniquely=True)
 
         if isinstance(element_type, MessageElement):
             for one_of in element_type.one_ofs:
diff --git a/tests/unit/protobuf/test_protobuf_schema.py b/tests/unit/protobuf/test_protobuf_schema.py
index 25d6cf9e6..4a655e504 100644
--- a/tests/unit/protobuf/test_protobuf_schema.py
+++ b/tests/unit/protobuf/test_protobuf_schema.py
@@ -2,7 +2,9 @@
 Copyright (c) 2023 Aiven Ltd
 See LICENSE for details
 """
+from _pytest.python_api import raises
 from karapace.protobuf.compare_result import CompareResult
+from karapace.protobuf.dependency import FieldNotUniquelyIdentifiableException
 from karapace.protobuf.kotlin_wrapper import trim_margin
 from karapace.protobuf.location import Location
 from karapace.protobuf.schema import ProtobufSchema
@@ -355,3 +357,35 @@ def test_protobuf_self_referencing_schema():
         """
 
     assert isinstance(ValidatedTypedSchema.parse(SchemaType.PROTOBUF, proto3).schema, ProtobufSchema)
+
+
+def test_illegal_redefine_objects_in_same_scope():
+    proto1 = """\
+            syntax = "proto3";
+
+            package fancy.company.in.party.v1;
+            message AnotherMessage {
+              enum BamFancyEnum {
+                // Hei! This is a comment!
+                MY_AWESOME_FIELD = 0;
+              }
+              message WowANestedMessage {
+                message DeeplyNestedMsg {
+                  enum BamFancyEnum {
+                  // Hei! This is a comment!
+                     MY_AWESOME_FIELD = 0;
+                  }
+                  message AnotherLevelOfNesting {
+                     BamFancyEnum im_tricky_im_referring_to_the_previous_enum = 1;
+                  }
+                }
+              }
+            }
+            """
+    with raises(FieldNotUniquelyIdentifiableException) as e:
+        assert isinstance(ValidatedTypedSchema.parse(SchemaType.PROTOBUF, proto1).schema, ProtobufSchema)
+
+    assert (
+        e.value.args[0] == "AnotherMessage.BamFancyEnum is not currently identifiable from the parser, "
+        "validating this message lead to break the schema evolution!"
+    )