Properly format and safely escape to CSV in DecodeEventLogger

build.gradle

@@ -51,6 +51,7 @@ dependencies {
     testImplementation 'junit:junit:4.12'
     testImplementation 'org.assertj:assertj-core:3.8.0'
     testImplementation("org.junit.jupiter:junit-jupiter-api:5.7.0")
+    testImplementation 'org.mockito:mockito-core:3.+'
     testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine'
 
     //Jitpack imports
@@ -80,6 +81,7 @@ dependencies {
     implementation 'commons-io:commons-io:2.7'
     implementation 'org.apache.commons:commons-lang3:3.8.1'
     implementation 'org.apache.commons:commons-math3:3.6.1'
+    implementation 'org.apache.commons:commons-csv:1.9.0'
     implementation 'org.apache.mina:mina-core:2.1.3'
     implementation 'org.apache.mina:mina-http:2.1.3'
     implementation 'org.controlsfx:controlsfx:11.0.2'

src/main/java/io/github/dsheirer/module/log/DecodeEventLogger.java

@@ -33,10 +33,13 @@
 import io.github.dsheirer.module.decode.event.IDecodeEventListener;
 import io.github.dsheirer.preference.TimestampFormat;
 import io.github.dsheirer.sample.Listener;
+import org.apache.commons.csv.CSVFormat;
+import org.apache.commons.csv.QuoteMode;
 
 import java.nio.file.Path;
 import java.text.DecimalFormat;
 import java.text.SimpleDateFormat;
+import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
 
@@ -47,6 +50,16 @@ public class DecodeEventLogger extends EventLogger implements IDecodeEventListen
     private AliasList mAliasList;
     private AliasModel mAliasModel;
 
+    /**
+     * The CSV format that SDR Trunk will use to write decode event logs
+     * <p>
+     * It uses the standard, default CSV format (RFC 4180 and permitting empty lines) but *always* quoting cells since
+     * that's what SDR Trunk has done previously when hand-crafting CSV rows.
+     */
+    private final CSVFormat mCsvFormat = CSVFormat.Builder.create(CSVFormat.DEFAULT)
+            .setQuoteMode(QuoteMode.ALL)
+            .build();
+
     public DecodeEventLogger(AliasModel aliasModel, Path logDirectory, String fileNameSuffix, long frequency)
     {
         super(logDirectory, fileNameSuffix, frequency);
@@ -83,25 +96,23 @@ public static String getCSVHeader()
 
     private String toCSV(IDecodeEvent event)
     {
+        List<Object> cells = new ArrayList<>();
 
-        StringBuilder sb = new StringBuilder();
-
-        sb.append("\"").append(mTimestampFormat.format(new Date(event.getTimeStart()))).append("\"");
-        sb.append(",\"").append(event.getDuration() > 0 ? event.getDuration() : "").append("\"");
-        sb.append(",\"").append(event.getProtocol()).append("\"");
+        cells.add(mTimestampFormat.format(new Date(event.getTimeStart())));
+        cells.add(event.getDuration() > 0 ? event.getDuration() : "");
+        cells.add(event.getProtocol());
 
         String description = event.getEventDescription();
-
-        sb.append(",\"").append(description != null ? description : "").append("\"");
+        cells.add(description != null ? description : "");
 
         List<Identifier> fromIdentifiers = event.getIdentifierCollection().getIdentifiers(Role.FROM);
         if(fromIdentifiers != null && !fromIdentifiers.isEmpty())
         {
-            sb.append(",\"").append(fromIdentifiers.get(0)).append("\"");
+            cells.add(fromIdentifiers.get(0));
         }
         else
         {
-            sb.append(",\"\"");
+            cells.add("");
         }
 
         List<Identifier> toIdentifiers = event.getIdentifierCollection().getIdentifiers(Role.TO);
@@ -116,48 +127,47 @@ private String toCSV(IDecodeEvent event)
             {
                 String mystring = (!mAliasList.getAliases(toIdentifiers.get(0)).isEmpty()) ?
                     mAliasList.getAliases(toIdentifiers.get(0)).get(0).toString() : "";
-                sb.append(",\"").append(mystring).append(" (").append(toIdentifiers.get(0)).append(")\"");
+                cells.add(mystring + " (" + toIdentifiers.get(0) + ")");
             }
             else
             {
-                sb.append(",\"\"");
+                cells.add("");
             }
         }
         else
         {
-            sb.append(",\"\"");
+            cells.add("");
         }
 
         IChannelDescriptor descriptor = event.getChannelDescriptor();
-
-        sb.append(",\"").append(descriptor != null ? descriptor : "").append("\"");
+        cells.add(descriptor != null ? descriptor : "");
 
         Identifier frequency = event.getIdentifierCollection()
             .getIdentifier(IdentifierClass.CONFIGURATION, Form.CHANNEL_FREQUENCY, Role.ANY);
 
         if(frequency instanceof FrequencyConfigurationIdentifier)
         {
-            sb.append(",\"").append(mFrequencyFormat
-                .format(((FrequencyConfigurationIdentifier)frequency).getValue() / 1e6d)).append("\"");
+            cells.add(mFrequencyFormat
+                    .format(((FrequencyConfigurationIdentifier)frequency).getValue() / 1e6d));
+
         }
         else
         {
-            sb.append(",\"\"");
+            cells.add("");
         }
 
         if(event.hasTimeslot())
         {
-            sb.append(",\"TS:").append(event.getTimeslot());
+            cells.add("TS:" + event.getTimeslot());
         }
         else
         {
-            sb.append(",\"\"");
+            cells.add("");
         }
 
         String details = event.getDetails();
+        cells.add(details != null ? details : "");
 
-        sb.append(",\"").append(details != null ? details : "").append("\"");
-
-        return sb.toString();
+        return mCsvFormat.format(cells.toArray());
     }
 }

src/test/java/io/github/dsheirer/module/log/DecodeEventLoggerTest.java

@@ -0,0 +1,121 @@
+package io.github.dsheirer.module.log;
+
+import io.github.dsheirer.alias.Alias;
+import io.github.dsheirer.alias.AliasModel;
+import io.github.dsheirer.channel.IChannelDescriptor;
+import io.github.dsheirer.identifier.IdentifierCollection;
+import io.github.dsheirer.identifier.Role;
+import io.github.dsheirer.identifier.configuration.AliasListConfigurationIdentifier;
+import io.github.dsheirer.identifier.configuration.DecoderTypeConfigurationIdentifier;
+import io.github.dsheirer.identifier.configuration.FrequencyConfigurationIdentifier;
+import io.github.dsheirer.module.decode.DecoderType;
+import io.github.dsheirer.module.decode.event.DecodeEvent;
+import io.github.dsheirer.module.decode.event.IDecodeEvent;
+import io.github.dsheirer.module.decode.p25.identifier.channel.APCO25Channel;
+import io.github.dsheirer.module.decode.p25.identifier.talkgroup.APCO25Talkgroup;
+import io.github.dsheirer.protocol.Protocol;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.*;
+
+class DecodeEventLoggerTest {
+    IChannelDescriptor channelDescriptor = APCO25Channel.create(98765, 1);
+
+    APCO25Talkgroup fromIdentifier = new APCO25Talkgroup(123, Role.FROM);
+    APCO25Talkgroup toIdentifier = new APCO25Talkgroup(456, Role.TO);
+    FrequencyConfigurationIdentifier freqIdentifier = new FrequencyConfigurationIdentifier(859562500L);
+    DecoderTypeConfigurationIdentifier decoderIdentifier = new DecoderTypeConfigurationIdentifier(DecoderType.P25_PHASE1);
+    AliasListConfigurationIdentifier aliasListIdentifier = new AliasListConfigurationIdentifier("My Alias List");
+    AliasModel aliasModel = new AliasModel();
+
+    Path logDirectory = Paths.get(System.getProperty("java.io.tmpdir"), "sdr_trunk_tests");
+
+    @BeforeEach
+    void setUp() {
+        aliasModel.addAliasList(aliasListIdentifier.getValue());
+    }
+
+    @AfterEach
+    void tearDown() {
+    }
+
+    IdentifierCollection buildIdentifierCollection() {
+        return new IdentifierCollection(Arrays.asList(
+                fromIdentifier,
+                toIdentifier,
+                decoderIdentifier,
+                freqIdentifier,
+                aliasListIdentifier
+        ));
+    }
+
+    DecodeEvent.DecodeEventBuilder decodeEventBuilder() {
+        return DecodeEvent.builder(1634428994000L)
+                .channel(channelDescriptor)
+                .identifiers(buildIdentifierCollection())
+                .duration(111)
+                .protocol(Protocol.APCO25)
+                .eventDescription("DATA_PACKET")
+                .timeslot(2)
+                .details("Some details");
+    }
+
+    @Test
+    void test_receive_writesToCsv() {
+        IDecodeEvent decodeEvent = decodeEventBuilder().build();
+
+        DecodeEventLogger decodeEventLogger = new DecodeEventLogger(aliasModel, logDirectory, "_foo.txt", 859562500);
+        DecodeEventLogger spy = spy(decodeEventLogger);
+
+        doNothing().when(spy).write(anyString());
+
+        spy.receive(decodeEvent);
+
+        String expectedToCsvString =
+                "\"2021:10:16:20:03:14\",\"111\",\"APCO-25\",\"DATA_PACKET\",\"123\",\" (456)\",\"98765-1\",\"859.562500\",\"TS:2\",\"Some details\"";
+        verify(spy).write(expectedToCsvString);
+    }
+
+    @Test
+    void test_receive_withQuotesInDetails_writesToCsv() {
+        IDecodeEvent decodeEvent = decodeEventBuilder()
+                .details("Some details now with \"quotes\"!")
+                .build();
+
+        DecodeEventLogger decodeEventLogger = new DecodeEventLogger(aliasModel, logDirectory, "_foo.txt", 859562500);
+        DecodeEventLogger spy = spy(decodeEventLogger);
+
+        doNothing().when(spy).write(anyString());
+
+        spy.receive(decodeEvent);
+
+        String expectedToCsvString =
+                "\"2021:10:16:20:03:14\",\"111\",\"APCO-25\",\"DATA_PACKET\",\"123\",\" (456)\",\"98765-1\",\"859.562500\",\"TS:2\",\"Some details now with \"\"quotes\"\"!\"";
+        verify(spy).write(expectedToCsvString);
+    }
+
+    @Test
+    void test_receive_withCommasInDetails_writesToCsv() {
+        IDecodeEvent decodeEvent = decodeEventBuilder()
+                .details("Some details now with, to an extent, commas!")
+                .build();
+
+        DecodeEventLogger decodeEventLogger = new DecodeEventLogger(aliasModel, logDirectory, "_foo.txt", 859562500);
+        DecodeEventLogger spy = spy(decodeEventLogger);
+
+        doNothing().when(spy).write(anyString());
+
+        spy.receive(decodeEvent);
+
+        String expectedToCsvString =
+                "\"2021:10:16:20:03:14\",\"111\",\"APCO-25\",\"DATA_PACKET\",\"123\",\" (456)\",\"98765-1\",\"859.562500\",\"TS:2\",\"Some details now with, to an extent, commas!\"";
+        verify(spy).write(expectedToCsvString);
+    }
+}