PS-10347 [8.0]: Ensure strict JSON compliance and robust field separation in audit_log_read()/AuditJsonHandler

plugin/audit_log_filter/json_reader/audit_json_handler.cc

@@ -14,6 +14,7 @@
    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA */
 
 #include "plugin/audit_log_filter/json_reader/audit_json_handler.h"
+#include <cstring>  // std::strcpy
 #include "plugin/audit_log_filter/audit_log_reader.h"
 
 namespace audit_log_filter::json_reader {
@@ -40,7 +41,8 @@ AuditJsonHandler::AuditJsonHandler(
       m_out_buff_size{out_buff_size},
       m_used_buff_size{0},
       m_printed_events_count{0},
-      m_reading_start_reached{false} {}
+      m_reading_start_reached{false},
+      m_is_first_field{false} {}
 
 char *AuditJsonHandler::get_result_buffer_ptr() noexcept {
   return m_out_buff.get();
@@ -62,7 +64,10 @@ void AuditJsonHandler::iterative_parse_init() noexcept {
 }
 
 void AuditJsonHandler::iterative_parse_close(bool with_null_tag) noexcept {
-  if (m_used_buff_size > 2 && !with_null_tag) {
+  // Remove the trailing ",\n" from the last event, if present,
+  // to ensure valid JSON array closing.
+  if (m_used_buff_size >= 2 && (m_current_buff - 2)[0] == ',' &&
+      (m_current_buff - 1)[0] == '\n' && !with_null_tag) {
     m_current_buff -= 2;
     m_used_buff_size -= 2;
   }
@@ -72,60 +77,74 @@ void AuditJsonHandler::iterative_parse_close(bool with_null_tag) noexcept {
   write_out_buff(closing_tag.c_str(), closing_tag.length());
 }
 
+// --- Value Handlers ---
+
 bool AuditJsonHandler::Null() { return true; }
 
 bool AuditJsonHandler::Bool(bool value) {
-  m_event_str << (value ? "true" : "false") << ", ";
+  m_event_str << (value ? "true" : "false");
   return true;
 }
 
 bool AuditJsonHandler::Int(int value) {
-  update_bookmark(value);
-  m_event_str << value << ", ";
+  update_bookmark(static_cast<uint64_t>(value));
+  m_event_str << value;
   return true;
 }
 
 bool AuditJsonHandler::Uint(unsigned value) {
-  update_bookmark(value);
-  m_event_str << value << ", ";
+  update_bookmark(static_cast<uint64_t>(value));
+  m_event_str << value;
   return true;
 }
 
 bool AuditJsonHandler::Int64(int64_t value) {
-  update_bookmark(value);
-  m_event_str << value << ", ";
+  update_bookmark(static_cast<uint64_t>(value));
+  m_event_str << value;
   return true;
 }
 
 bool AuditJsonHandler::Uint64(uint64_t value) {
   update_bookmark(value);
-  m_event_str << value << ", ";
+  m_event_str << value;
   return true;
 }
 
 bool AuditJsonHandler::Double(double value) {
-  m_event_str << value << ", ";
+  m_event_str << value;
   return true;
 }
 
-bool AuditJsonHandler::String(const char *value,
-                              rapidjson::SizeType length [[maybe_unused]],
+bool AuditJsonHandler::String(const char *value, rapidjson::SizeType length,
                               bool copy [[maybe_unused]]) {
-  update_bookmark(value);
-  m_event_str << "\"" << value << "\", ";
+  std::string s_value(value, length);
+  update_bookmark(s_value);
+  m_event_str << "\"" << s_value << "\"";
   return true;
 }
 
+// --- Structure Handlers ---
+
 bool AuditJsonHandler::StartObject() {
   ++m_obj_level;
   m_event_str << "{";
+  // Reset flag: the first key encountered will not have a comma separator
+  m_is_first_field = true;
   return true;
 }
 
-bool AuditJsonHandler::Key(const char *str,
-                           rapidjson::SizeType length [[maybe_unused]],
+bool AuditJsonHandler::Key(const char *str, rapidjson::SizeType length,
                            bool copy [[maybe_unused]]) {
-  m_current_key_name = str;
+  m_current_key_name.assign(str, length);
+
+  // Manage comma separation and state before appending the key.
+  if (m_is_first_field) {
+    m_is_first_field = false;  // This is the first field, so unset the flag
+  } else {
+    m_event_str << ", ";  // Prepend separator if a previous field exists
+  }
+
+  // Append key and colon
   m_event_str << "\"" << str << "\": ";
   return true;
 }
@@ -136,20 +155,24 @@ bool AuditJsonHandler::EndObject(rapidjson::SizeType memberCount
     --m_obj_level;
   }
 
-  if (m_obj_level > 0) {
-    // remove comma after last event
-    m_event_str.seekp(-2, std::ios_base::end);
-  }
-
+  // Append the closing brace.
   m_event_str << "}";
 
+  // If closing an inner object (m_obj_level is now > 0), or the top-level
+  // object, ensure the m_is_first_field is false, so the next Key() call
+  // (if any, in the parent) correctly prepends a comma.
+  m_is_first_field = false;
+
+  // Handle the completed top-level event object.
   if (m_obj_level == 0) {
     if (!check_reading_start_reached()) {
       clear_current_event();
       return true;
     }
 
+    // Add the inter-event separator (comma and newline).
     m_event_str << ",\n";
+
     const auto event_length = m_event_str.str().length();
     const auto max_array_length =
         m_reader_context->batch_reader_args->max_array_length;

plugin/audit_log_filter/json_reader/audit_json_handler.h

@@ -90,6 +90,7 @@ class AuditJsonHandler
   ulong m_used_buff_size;
   ulong m_printed_events_count;
   bool m_reading_start_reached;
+  bool m_is_first_field;
 
   LogBookmark m_current_event_bookmark;
 };