schema_registry/json: fix is_array_superset

in the case of tuples with different size of elements, excess elements needs to be compatible with the other schema "additionalItems". previously, this property was cheked only for excess elements in newer
vbotbuildovich · Aug 28, 2024 · b6201a1 · b6201a1
1 parent f81507b
commit b6201a1
Show file tree

Hide file tree

Showing 2 changed files with 113 additions and 12 deletions.
diff --git a/src/v/pandaproxy/schema_registry/json.cc b/src/v/pandaproxy/schema_registry/json.cc
@@ -1015,21 +1015,32 @@ bool is_array_superset(
         return false;
     }
 
-    // no mismatching elements. two possible cases
-    // 1. older_tuple_schema.Size() >= newer_tuple_schema.Size() ->
-    // compatible (implies newer_it==end())
-    // 2. older_tuple_schema.Size() <  newer_tuple_schema.Size() -> check
-    // excess elements with older["additionalItems"]
+    // no mismatching elements, and they don't have the same size.
+    // To be compatible, excess elements needs to be compatible with the other
+    // "additionalItems" schema
 
     auto older_additional_schema = get_object_or_empty(
       ctx.older, older, get_additional_items_kw(ctx.older.dialect()));
+    if (!std::all_of(
+          newer_it, newer_tuple_schema.end(), [&](json::Value const& n) {
+              return is_superset(ctx, older_additional_schema, n);
+          })) {
+        // newer has excess elements that are not compatible with
+        // older["additionalItems"]
+        return false;
+    }
 
-    // check that all excess schemas are compatible with
-    // older["additionalItems"]
-    return std::all_of(
-      newer_it, newer_tuple_schema.end(), [&](json::Value const& n) {
-          return is_superset(ctx, older_additional_schema, n);
-      });
+    auto newer_additional_schema = get_object_or_empty(
+      ctx.newer, newer, get_additional_items_kw(ctx.newer.dialect()));
+    if (!std::all_of(
+          older_it, older_tuple_schema.end(), [&](json::Value const& o) {
+              return is_superset(ctx, o, newer_additional_schema);
+          })) {
+        // older has excess elements that are not compatible with
+        // newer["additionalItems"]
+        return false;
+    }
+    return true;
 }
 
 bool is_object_properties_superset(

diff --git a/src/v/pandaproxy/schema_registry/test/test_json_schema.cc b/src/v/pandaproxy/schema_registry/test/test_json_schema.cc
@@ -555,7 +555,14 @@ static constexpr auto compatibility_test_cases = std::to_array<
     .reader_schema
     = R"({"type": "array", "additionalItems": {"type": "integer"}, "items": [{"type":"boolean"}]})",
     .writer_schema
-    = R"({"type": "array", "additionalItems": {"type": "integer"}, "items": [{"type":"boolean"}, {"type": "boolean"}]})",
+    = R"({"type": "array", "additionalItems": false, "items": [{"type":"boolean"}, {"type": "number"}]})",
+    .reader_is_compatible_with_writer = false,
+  },
+  {
+    .reader_schema
+    = R"({"type": "array", "additionalItems": {"type": "number"}, "items": [{"type":"boolean"}, {"type": "integer"}]})",
+    .writer_schema
+    = R"({"type": "array", "additionalItems": {"type": "number"}, "items": [{"type":"boolean"}]})",
     .reader_is_compatible_with_writer = false,
   },
   // combinators: "not" is required on both schema
@@ -865,6 +872,89 @@ static constexpr auto compatibility_test_cases = std::to_array<
   })",
     .reader_is_compatible_with_writer = true,
   },
+  // array checks: excess elements are absorbed by additionalItems
+  {
+    .reader_schema = R"(
+{
+  "type": "array",
+  "items": [
+    {
+      "type": "boolean"
+    }
+  ],
+  "additionalItems": {
+    "type": "number"
+  }
+})",
+    .writer_schema = R"(
+{
+  "type": "array",
+  "items": [
+    {
+      "type": "boolean"
+    },
+    {
+      "type": "integer"
+    }
+  ],
+  "additionalItems": false
+})",
+    .reader_is_compatible_with_writer = true,
+  },
+  {
+    .reader_schema = R"(
+{
+  "type": "array",
+  "items": [
+    {
+      "type": "boolean"
+    },
+    {
+      "type": "number"
+    }
+  ],
+  "additionalItems": {"type": "number"}
+})",
+    .writer_schema = R"(
+{
+  "type": "array",
+  "items": [
+    {
+      "type": "boolean"
+    }
+  ],
+  "additionalItems": {
+    "type": "integer"
+  }
+})",
+    .reader_is_compatible_with_writer = true,
+  },
+  {
+    .reader_schema = R"(
+{
+  "type": "array",
+  "items": [
+    {
+      "type": "boolean"
+    },
+    {
+      "type": "number"
+    }
+  ],
+  "additionalItems": false
+})",
+    .writer_schema = R"(
+{
+  "type": "array",
+  "items": [
+    {
+      "type": "boolean"
+    }
+  ],
+  "additionalItems": false
+})",
+    .reader_is_compatible_with_writer = true,
+  },
   // array checks: prefixItems/items are compatible across drafts
   {
     .reader_schema = R"({