Skip to content

Commit

Permalink
Properly format and safely escape to CSV in DecodeEventLogger (#1092)
Browse files Browse the repository at this point in the history
* Add failing test for toCSV()

* Add missing " to timeslot value

* Add failing test for quotes and commas in details

* Separate comma and quote tests

* Escape toCSV cells using utility

* Use CSVFormatter instead of escape utils

Allows us to control exactly how we want to format a CSV-valid row.

* Never have a null cell

Instead of using a stream map, opting for an explicit handling so the logger can explode when an unexpected null shows up. Prevents the situation where things are mysteriously blank, forcing developers to handle known null scenarios.
  • Loading branch information
f3ndot committed Nov 15, 2021
1 parent 0bc11a0 commit cb83a8e
Show file tree
Hide file tree
Showing 3 changed files with 155 additions and 22 deletions.
2 changes: 2 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,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'

Expand Down Expand Up @@ -92,6 +93,7 @@ dependencies {
implementation 'org.apache.commons:commons-compress:1.20'
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.1.0'
Expand Down
54 changes: 32 additions & 22 deletions src/main/java/io/github/dsheirer/module/log/DecodeEventLogger.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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());
}
}
121 changes: 121 additions & 0 deletions src/test/java/io/github/dsheirer/module/log/DecodeEventLoggerTest.java
Original file line number Diff line number Diff line change
@@ -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);
}
}

0 comments on commit cb83a8e

Please sign in to comment.