Merge pull request #22654 from vbotbuildovich/backport-pr-22633-v23.3…

….x-466 [v23.3.x] [CORE-2144] schema_registry/protobuf: Disable protobuf logging
redpanda-data · Jul 30, 2024 · be57dd8 · be57dd8
2 parents af9f70d + 5e208af
commit be57dd8
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 4 deletions.
diff --git a/src/v/pandaproxy/schema_registry/protobuf.cc b/src/v/pandaproxy/schema_registry/protobuf.cc
@@ -231,10 +231,14 @@ class parser {
 
         // Attempt parse a .proto file
         if (!_parser.Parse(&t, &_fdp)) {
-            // base64 decode the schema
-            iobuf_istream is{base64_to_iobuf(schema.def().raw()())};
-            // Attempt parse as an encoded FileDescriptorProto.pb
-            if (!_fdp.ParseFromIstream(&is.istream())) {
+            try {
+                // base64 decode the schema
+                iobuf_istream is{base64_to_iobuf(schema.def().raw()())};
+                // Attempt parse as an encoded FileDescriptorProto.pb
+                if (!_fdp.ParseFromIstream(&is.istream())) {
+                    throw as_exception(error_collector.error());
+                }
+            } catch (const base64_decoder_exception&) {
                 throw as_exception(error_collector.error());
             }
         }

diff --git a/src/v/pandaproxy/schema_registry/test/compatibility_protobuf.cc b/src/v/pandaproxy/schema_registry/test/compatibility_protobuf.cc
@@ -189,6 +189,41 @@ SEASTAR_THREAD_TEST_CASE(test_protobuf_recursive_reference) {
                                   .get();
 }
 
+SEASTAR_THREAD_TEST_CASE(test_binary_protobuf) {
+    simple_sharded_store store;
+
+    BOOST_REQUIRE_NO_THROW(store.store
+                             .make_valid_schema(pps::canonical_schema{
+                               pps::subject{"com.redpanda.Payload.proto"},
+                               pps::canonical_schema_definition{
+                                 base64_raw_proto, pps::schema_type::protobuf}})
+                             .get());
+}
+
+SEASTAR_THREAD_TEST_CASE(test_invalid_binary_protobuf) {
+    simple_sharded_store store;
+
+    auto broken_base64_raw_proto = base64_raw_proto.substr(1);
+
+    auto schema = pps::canonical_schema{
+      pps::subject{"com.redpanda.Payload.proto"},
+      pps::canonical_schema_definition{
+        broken_base64_raw_proto, pps::schema_type::protobuf}};
+
+    BOOST_REQUIRE_EXCEPTION(
+      store.store
+        .make_valid_schema(pps::canonical_schema{
+          pps::subject{"com.redpanda.Payload.proto"},
+          pps::canonical_schema_definition{
+            broken_base64_raw_proto, pps::schema_type::protobuf}})
+        .get(),
+      pps::exception,
+      [](const pps::exception& e) {
+          std::cout << e.what();
+          return e.code() == pps::error_code::schema_invalid;
+      });
+}
+
 SEASTAR_THREAD_TEST_CASE(test_protobuf_well_known) {
     simple_sharded_store store;
 

diff --git a/src/v/pandaproxy/schema_registry/test/compatibility_protobuf.h b/src/v/pandaproxy/schema_registry/test/compatibility_protobuf.h
@@ -88,3 +88,70 @@ message A1 {
   }
 })",
   pps::schema_type::protobuf};
+
+// Binary encoded protobuf descriptor:
+//
+// syntax = "proto3";
+//
+// package com.redpanda;
+//
+// option go_package = "./;main";
+// option java_multiple_files = true;
+//
+// import "google/protobuf/timestamp.proto";
+//
+// message Payload {
+//     int32 val = 1;
+//     google.protobuf.Timestamp timestamp = 2;
+// }
+//
+// message A {
+//     message B {
+//         message C {
+//             message D {
+//                 message M00 {}
+//                 message M01 {}
+//                 message M02 {}
+//                 message M03 {}
+//                 message M04 {}
+//                 message M05 {}
+//                 message M06 {}
+//                 message M07 {}
+//                 message M08 {}
+//                 message M09 {}
+//                 message M10 {}
+//                 message M11 {}
+//                 message M12 {}
+//                 message M13 {}
+//                 message M14 {}
+//                 message M15 {}
+//                 message M16 {}
+//                 message M17 {}
+//                 message NestedPayload {
+//                     int32 val = 1;
+//                     google.protobuf.Timestamp timestamp = 2;
+//                 }
+//             }
+//         }
+//     }
+// }
+//
+// message CompressiblePayload {
+//     int32 val = 1;
+//     google.protobuf.Timestamp timestamp = 2;
+//     string message = 3;
+// }
+constexpr std::string_view base64_raw_proto{
+  "Cg1wYXlsb2FkLnByb3RvEgxjb20ucmVkcGFuZGEaH2dvb2dsZS9wcm90b2J1Zi90aW1lc3RhbXAu"
+  "cHJvdG8iRQoHUGF5bG9hZBILCgN2YWwYASABKAUSLQoJdGltZXN0YW1wGAIgASgLMhouZ29vZ2xl"
+  "LnByb3RvYnVmLlRpbWVzdGFtcCLgAQoBQRraAQoBQhrUAQoBQxrOAQoBRBoFCgNNMDAaBQoDTTAx"
+  "GgUKA00wMhoFCgNNMDMaBQoDTTA0GgUKA00wNRoFCgNNMDYaBQoDTTA3GgUKA00wOBoFCgNNMDka"
+  "BQoDTTEwGgUKA00xMRoFCgNNMTIaBQoDTTEzGgUKA00xNBoFCgNNMTUaBQoDTTE2GgUKA00xNxpL"
+  "Cg1OZXN0ZWRQYXlsb2FkEgsKA3ZhbBgBIAEoBRItCgl0aW1lc3RhbXAYAiABKAsyGi5nb29nbGUu"
+  "cHJvdG9idWYuVGltZXN0YW1wImIKE0NvbXByZXNzaWJsZVBheWxvYWQSCwoDdmFsGAEgASgFEi0K"
+  "CXRpbWVzdGFtcBgCIAEoCzIaLmdvb2dsZS5wcm90b2J1Zi5UaW1lc3RhbXASDwoHbWVzc2FnZRgD"
+  "IAEoCUILUAFaBy4vO21haW5iBnByb3RvMw=="};
+
+const pps::canonical_schema_definition base64_proto{
+  pps::canonical_schema_definition{
+    base64_raw_proto, pps::schema_type::protobuf}};
diff --git a/src/v/redpanda/application.cc b/src/v/redpanda/application.cc
@@ -135,6 +135,7 @@
 #include <seastar/util/defer.hh>
 #include <seastar/util/log.hh>
 
+#include <google/protobuf/stubs/logging.h>
 #include <sys/resource.h>
 #include <sys/utsname.h>
 
@@ -513,6 +514,12 @@ void application::initialize(
       .get();
     _cpu_profiler.invoke_on_all(&resources::cpu_profiler::start).get();
 
+    /*
+     * Disable the logger for protobuf; some interfaces don't allow a pluggable
+     * error collector.
+     */
+    google::protobuf::SetLogHandler(nullptr);
+
     /*
      * allocate per-core zstd decompression workspace and per-core
      * async_stream_zstd workspaces. it can be several megabytes in size, so do