Feature event log: better escaping in events

platform/platform-impl/src/com/intellij/internal/statistic/eventLog/LogEvents.kt

@@ -8,13 +8,12 @@ package com.intellij.internal.statistic.eventLog
 import com.intellij.util.containers.ContainerUtil
 import java.util.*
 
-open class LogEvent(val session: String, val bucket: String,
-                    recorderId: String,
-                    recorderVersion: String,
-                    type: String) {
+open class LogEvent(session: String, bucket: String, recorderId: String, recorderVersion: String, type: String) {
+  val session = escape(session)
+  val bucket = escape(bucket)
   val time = System.currentTimeMillis()
-  val recorder: LogEventRecorder = LogEventRecorder(removeTabsOrSpaces(recorderId), recorderVersion)
-  val action: LogEventAction = LogEventAction(removeTabsOrSpaces(type))
+  val recorder: LogEventRecorder = LogEventRecorder(escape(recorderId), escape(recorderVersion))
+  val action: LogEventAction = LogEventAction(escape(type))
 
   fun shouldMerge(next: LogEvent): Boolean {
     if (session != next.session) return false
@@ -26,10 +25,6 @@ open class LogEvent(val session: String, val bucket: String,
     return true
   }
 
-  private fun removeTabsOrSpaces(str : String) : String {
-    return str.replace(" ", "_").replace("\t", "_")
-  }
-
   override fun equals(other: Any?): Boolean {
     if (this === other) return true
     if (javaClass != other?.javaClass) return false
@@ -82,7 +77,9 @@ class LogEventAction(val id: String) {
     if (data.isEmpty()) {
       data = ContainerUtil.newHashMap()
     }
-    data.put(key, value)
+
+    val escapedValue = if (value is String) escape(value) else value
+    data.put(escape(key), escapedValue)
   }
 
   override fun equals(other: Any?): Boolean {
@@ -99,4 +96,8 @@ class LogEventAction(val id: String) {
   override fun hashCode(): Int {
     return 31 * id.hashCode() + data.hashCode()
   }
+}
+
+private fun escape(str: String): String {
+  return str.replace(" ", "_").replace("\t", "_").replace("\"", "")
 }

platform/platform-tests/testSrc/com/intellij/internal/statistics/FeatureEventLogSerializationTest.kt

@@ -32,6 +32,25 @@ class FeatureEventLogSerializationTest {
     testEventSerialization(LogEvent("session-id", "-1", "recorder-id", "1", "event\ttype"))
   }
 
+  @Test
+  fun testEventActionWithQuotes() {
+    testEventSerialization(LogEvent("session-id", "-1", "recorder-id", "1", "event\"type"))
+  }
+
+  @Test
+  fun testEventActionWithTagInDataKey() {
+    val event = LogEvent("session-id", "-1", "recorder-id", "1", "event-type")
+    event.action.addData("my key", "value")
+    testEventSerialization(event, "my_key")
+  }
+
+  @Test
+  fun testEventActionWithTagInDataValue() {
+    val event = LogEvent("session-id", "-1", "recorder-id", "1", "event-type")
+    event.action.addData("key", "my value")
+    testEventSerialization(event, "key")
+  }
+
   @Test
   fun testEventContent() {
     val events = ArrayList<LogEvent>()
@@ -66,30 +85,32 @@ class FeatureEventLogSerializationTest {
     assert(json.get("time").isJsonPrimitive)
 
     assert(json.get("session").isJsonPrimitive)
-    assert(noTabsOrSpaces(json.get("session").asString))
+    assert(noTabsOrSpacesOrQuotes(json.get("session").asString))
 
     assert(json.get("bucket").isJsonPrimitive)
-    assert(noTabsOrSpaces(json.get("bucket").asString))
+    assert(noTabsOrSpacesOrQuotes(json.get("bucket").asString))
 
     assert(json.get("recorder").isJsonObject)
     assert(json.getAsJsonObject("recorder").get("id").isJsonPrimitive)
     assert(json.getAsJsonObject("recorder").get("version").isJsonPrimitive)
-    assert(noTabsOrSpaces(json.getAsJsonObject("recorder").get("id").asString))
-    assert(noTabsOrSpaces(json.getAsJsonObject("recorder").get("version").asString))
+    assert(noTabsOrSpacesOrQuotes(json.getAsJsonObject("recorder").get("id").asString))
+    assert(noTabsOrSpacesOrQuotes(json.getAsJsonObject("recorder").get("version").asString))
 
     assert(json.get("action").isJsonObject)
     assert(json.getAsJsonObject("action").get("id").isJsonPrimitive)
     assert(json.getAsJsonObject("action").get("data").isJsonObject)
-    assert(noTabsOrSpaces(json.getAsJsonObject("action").get("id").asString))
+    assert(noTabsOrSpacesOrQuotes(json.getAsJsonObject("action").get("id").asString))
 
     val obj = json.getAsJsonObject("action").get("data").asJsonObject
     for (option in dataOptions) {
+      assert(noTabsOrSpacesOrQuotes(option))
       assert(obj.get(option).isJsonPrimitive)
+      assert(noTabsOrSpacesOrQuotes(obj.get(option).asString))
     }
   }
 
-  private fun noTabsOrSpaces(str : String) : Boolean {
-    return str.indexOf(" ") == -1 && str.indexOf("\t") == -1
+  private fun noTabsOrSpacesOrQuotes(str : String) : Boolean {
+    return str.indexOf(" ") == -1 && str.indexOf("\t") == -1 && str.indexOf("\"") == -1
   }
 
   @Suppress("unused")