fix: keep a self-referencing trait map from escaping trackEvent as an Error

Serialising traits and metadata with valueToTree at buffer time let a
map or list that contains itself throw a raw StackOverflowError out of
trackEvent. The older flush-time serialisation had reported the same
input as a JsonMappingException, so this was a regression from moving
serialisation earlier.

Each event's traits and metadata are now written with
writeValueAsString, which reports every cycle as a checked
JsonMappingException, and are held as RawValue that Jackson emits
verbatim in the batch. The offending event is dropped and logged; the
rest are unaffected. Nothing catches Error, and the class Javadoc now
says so. Buffered JSON text is also more compact to hold than a tree.

Author: Zaimwa9

src/main/java/com/flagsmith/threads/EventProcessor.java

@@ -1,8 +1,9 @@
 package com.flagsmith.threads;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.util.RawValue;
 import com.flagsmith.FlagsmithLogger;
 import com.flagsmith.MapperFactory;
 import com.flagsmith.Versions;
@@ -39,8 +40,9 @@
  * Buffers experimentation events and ships them to the Flagsmith events API.
  *
  * <p>Events are flushed on a fixed interval, when the buffer fills up, and on {@link #close()}.
- * Exposure events are deduplicated within a flush window. Nothing thrown here ever reaches caller
- * code: every failure is logged instead.
+ * Exposure events are deduplicated within a flush window. No exception thrown here reaches caller
+ * code: every failure is logged instead, including traits or metadata that cannot be serialised,
+ * which drop only their own event. {@link Error}s such as {@code OutOfMemoryError} are not caught.
  */
 public class EventProcessor {
 
@@ -342,20 +344,24 @@ private void bufferEvent(String event, String featureName, String identifier, Ob
       eventMetadata.put(SDK_VERSION_KEY, Versions.getVersion());
       final Object experimentId = eventMetadata.get(EXPERIMENT_ID_KEY);
 
-      // Traits and metadata are caller objects of any shape. Turning them into JSON trees here,
-      // rather than at flush time, means a value Jackson cannot serialise drops this one event
-      // (logged below) instead of failing the whole batch it would later be sent in. It is also
-      // a deep copy, so a caller mutating a nested map afterwards cannot change a buffered event.
-      ObjectMapper mapper = MapperFactory.getMapper();
+      // Traits and metadata are caller objects of any shape. Serialising them here, rather than
+      // at flush time, means a value Jackson cannot serialise drops this one event (logged
+      // below) instead of failing the whole batch it would later be sent in. The JSON text is
+      // also a deep copy, so a caller mutating a nested map afterwards cannot change a buffered
+      // event, and is more compact to hold than the objects or a JSON tree.
+      //
+      // It must be writeValueAsString, not valueToTree: on a map or list that contains itself,
+      // valueToTree lets a raw StackOverflowError escape, where writeValueAsString reports it as
+      // a JsonMappingException that the catch below handles.
       Map<String, Object> eventTraits = eventTraits(traits);
 
       Map<String, Object> eventPayload = new LinkedHashMap<>();
       eventPayload.put("event", event);
       eventPayload.put("feature_name", featureName);
       eventPayload.put("identifier", identifier);
       eventPayload.put("value", stringValue);
-      eventPayload.put("traits", eventTraits == null ? null : mapper.valueToTree(eventTraits));
-      eventPayload.put("metadata", mapper.valueToTree(eventMetadata));
+      eventPayload.put("traits", eventTraits == null ? null : toJson(eventTraits));
+      eventPayload.put("metadata", toJson(eventMetadata));
       eventPayload.put("timestamp", System.currentTimeMillis());
 
       boolean isFull;
@@ -379,11 +385,16 @@ private void bufferEvent(String event, String featureName, String identifier, Ob
       if (isFull) {
         flush();
       }
-    } catch (RuntimeException e) {
+    } catch (JsonProcessingException | RuntimeException e) {
       logger.error("Failed to buffer event " + event + ".", e);
     }
   }
 
+  /** JSON text Jackson writes out verbatim when the batch is serialised. */
+  private static RawValue toJson(Object value) throws JsonProcessingException {
+    return new RawValue(MapperFactory.getMapper().writeValueAsString(value));
+  }
+
   private void logClosed(String event) {
     logger.info("Not buffering event {}: the event processor is closed.", event);
   }

src/test/java/com/flagsmith/threads/EventProcessorTest.java

@@ -19,6 +19,7 @@
 import static org.mockito.Mockito.when;
 
 import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.util.RawValue;
 import com.flagsmith.FlagsmithLogger;
 import com.flagsmith.MapperFactory;
 import com.flagsmith.config.FlagsmithConfig;
@@ -117,6 +118,12 @@ private void flushAndWait(EventProcessor processor) {
     processor.flush().get(WAIT_SECONDS, TimeUnit.SECONDS);
   }
 
+  /** Parse a buffered event's pre-serialised traits or metadata. */
+  @SneakyThrows
+  private static JsonNode json(Object buffered) {
+    return MapperFactory.getMapper().readTree(((RawValue) buffered).rawValue().toString());
+  }
+
   @Test
   public void trackEvent_buffersStringifiedValueSdkVersionAndTimestamp() {
     EventProcessor processor = newProcessor(1000, 0);
@@ -140,9 +147,9 @@ public void trackEvent_buffersStringifiedValueSdkVersionAndTimestamp() {
     assertNull(event.get("feature_name"));
     assertEquals("user-123", event.get("identifier"));
     assertEquals("49.0", event.get("value"));
-    assertEquals(MapperFactory.getMapper().valueToTree(traits), event.get("traits"));
+    assertEquals(MapperFactory.getMapper().valueToTree(traits), json(event.get("traits")));
 
-    JsonNode eventMetadata = (JsonNode) event.get("metadata");
+    JsonNode eventMetadata = json(event.get("metadata"));
     assertEquals("checkout", eventMetadata.get("source").asText());
     assertNotNull(eventMetadata.get("sdk_version"));
 
@@ -487,7 +494,7 @@ public void trackExposureEvent_unwrapsTraitConfigsAndDropsTransientTraits() {
 
     processor.trackExposureEvent("checkout_cta", "user-1", "treatment", traits, null);
 
-    JsonNode buffered = (JsonNode) processor.bufferedEvents().get(0).get("traits");
+    JsonNode buffered = json(processor.bufferedEvents().get(0).get("traits"));
     assertEquals(2, buffered.size());
     assertEquals("premium", buffered.get("plan").asText());
     assertEquals("gold", buffered.get("tier").asText());
@@ -513,9 +520,9 @@ public void trackEvent_copiesTraitsAndMetadataDeeplyAtBufferTime() {
     Map<String, Object> event = processor.bufferedEvents().get(0);
     assertEquals(
         MapperFactory.getMapper().valueToTree(Collections.singletonMap("plan", "premium")),
-        event.get("traits"));
+        json(event.get("traits")));
     assertEquals("payment",
-        ((JsonNode) event.get("metadata")).get("context").get("step").asText());
+        json(event.get("metadata")).get("context").get("step").asText());
   }
 
   @Test
@@ -542,6 +549,25 @@ public void trackEvent_dropsOnlyTheEventWhoseValuesCannotBeSerialised() {
     assertEquals("user-4", events.get(1).get("identifier").asText());
   }
 
+  @Test
+  public void trackEvent_dropsAnEventWhoseTraitsOrMetadataContainThemselves() {
+    EventProcessor processor = newProcessor(1000, 0);
+    Map<String, Object> cyclic = new HashMap<>();
+    cyclic.put("self", cyclic);
+    List<Object> cyclicList = new ArrayList<>();
+    cyclicList.add(cyclicList);
+
+    // A caller bug, but it must stay out of caller code: serialising these recurses without
+    // end, and it has to surface as a dropped event, not a StackOverflowError.
+    processor.trackEvent("purchase", "user-1", "1", cyclic, null);
+    processor.trackEvent("purchase", "user-2", "2", null,
+        Collections.singletonMap("list", cyclicList));
+    processor.trackEvent("purchase", "user-3", "3", null, null);
+
+    assertEquals(1, processor.bufferedEvents().size());
+    assertEquals("user-3", processor.bufferedEvents().get(0).get("identifier"));
+  }
+
   @Test
   @SneakyThrows
   public void start_flushesOnTheTimer() {