diff --git a/core/src/main/java/google/registry/tmch/ClaimsListParser.java b/core/src/main/java/google/registry/tmch/ClaimsListParser.java index 0f71b585b..adc7efe1f 100644 --- a/core/src/main/java/google/registry/tmch/ClaimsListParser.java +++ b/core/src/main/java/google/registry/tmch/ClaimsListParser.java @@ -16,18 +16,21 @@ package google.registry.tmch; import static com.google.common.base.Preconditions.checkArgument; +import com.google.common.base.Joiner; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableMap; import google.registry.model.tmch.ClaimsList; +import java.io.IOException; +import java.io.StringReader; import java.util.List; +import org.apache.commons.csv.CSVFormat; +import org.apache.commons.csv.CSVParser; +import org.apache.commons.csv.CSVRecord; import org.joda.time.DateTime; /** * Claims List (MarksDB DNL CSV) Parser. * - *

This is a quick and dirty CSV parser made specifically for the DNL CSV format defined in the - * TMCH specification. It doesn't support any fancy CSV features like quotes. - * * @see * TMCH functional specifications - DNL List file */ @@ -38,7 +41,7 @@ public class ClaimsListParser { * *

Please note that this does not insert the object into the DB. */ - public static ClaimsList parse(List lines) { + public static ClaimsList parse(List lines) throws IOException { ImmutableMap.Builder builder = new ImmutableMap.Builder<>(); // First line: , @@ -51,26 +54,16 @@ public class ClaimsListParser { checkArgument(version == 1, String.format( "Line 1: Expected version 1, found %d", version)); - // Second line contains headers: DNL,lookup-key,insertion-datetime - List secondLine = Splitter.on(',').splitToList(lines.get(1)); - checkArgument(secondLine.size() == 3, String.format( - "Line 2: Expected 3 elements, found %d", secondLine.size())); - checkArgument("DNL".equals(secondLine.get(0)), String.format( - "Line 2: Expected header \"DNL\", found \"%s\"", secondLine.get(0))); - checkArgument("lookup-key".equals(secondLine.get(1)), String.format( - "Line 2: Expected header \"lookup-key\", found \"%s\"", secondLine.get(1))); - checkArgument("insertion-datetime".equals(secondLine.get(2)), String.format( - "Line 2: Expected header \"insertion-datetime\", found \"%s\"", secondLine.get(2))); - - // Subsequent lines: ,, - for (int i = 2; i < lines.size(); i++) { - List currentLine = Splitter.on(',').splitToList(lines.get(i)); - checkArgument(currentLine.size() == 3, String.format( - "Line %d: Expected 3 elements, found %d", i + 1, currentLine.size())); - - String label = currentLine.get(0); - String lookupKey = currentLine.get(1); - DateTime.parse(currentLine.get(2)); // This is the insertion time, currently unused. + // Note: we have to skip the first line because it contains the version metadata + CSVParser csv = + CSVFormat.Builder.create(CSVFormat.DEFAULT) + .setHeader() + .setSkipHeaderRecord(true) + .build() + .parse(new StringReader(Joiner.on('\n').join(lines.subList(1, lines.size())))); + for (CSVRecord record : csv) { + String label = record.get("DNL"); + String lookupKey = record.get("lookup-key"); builder.put(label, lookupKey); } diff --git a/core/src/main/java/google/registry/tmch/Marksdb.java b/core/src/main/java/google/registry/tmch/Marksdb.java index f2e4fc738..5b9ab1640 100644 --- a/core/src/main/java/google/registry/tmch/Marksdb.java +++ b/core/src/main/java/google/registry/tmch/Marksdb.java @@ -36,7 +36,6 @@ import java.security.GeneralSecurityException; import java.security.Security; import java.security.SignatureException; import java.util.Arrays; -import java.util.List; import java.util.Optional; import javax.annotation.Tainted; import javax.inject.Inject; @@ -125,7 +124,8 @@ public final class Marksdb { } } - List fetchSignedCsv(Optional loginAndPassword, String csvPath, String sigPath) + ImmutableList fetchSignedCsv( + Optional loginAndPassword, String csvPath, String sigPath) throws IOException, GeneralSecurityException, PGPException { checkArgument( loginAndPassword.isPresent(), "Cannot fetch from MarksDB without login credentials"); diff --git a/core/src/main/java/google/registry/tmch/SmdrlCsvParser.java b/core/src/main/java/google/registry/tmch/SmdrlCsvParser.java index 2b76c8468..a370f1608 100644 --- a/core/src/main/java/google/registry/tmch/SmdrlCsvParser.java +++ b/core/src/main/java/google/registry/tmch/SmdrlCsvParser.java @@ -17,55 +17,52 @@ package google.registry.tmch; import static com.google.common.base.Preconditions.checkArgument; import static org.joda.time.DateTimeZone.UTC; +import com.google.common.base.Joiner; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableMap; import google.registry.model.smd.SignedMarkRevocationList; +import java.io.IOException; +import java.io.StringReader; import java.util.List; +import org.apache.commons.csv.CSVFormat; +import org.apache.commons.csv.CSVParser; +import org.apache.commons.csv.CSVRecord; import org.joda.time.DateTime; /** * Signed Mark Data Revocation List (SMDRL) CSV Parser * - *

This is a quick and dirty CSV parser made specifically for the SMDRL CSV format defined in - * the TMCH specification. It doesn't support any fancy CSV features like quotes. - * * @see * TMCH functional specifications - SMD Revocation List */ public final class SmdrlCsvParser { /** Converts the lines from the DNL CSV file into a data structure. */ - public static SignedMarkRevocationList parse(List lines) { + public static SignedMarkRevocationList parse(List lines) throws IOException { ImmutableMap.Builder revokes = new ImmutableMap.Builder<>(); // First line: , List firstLine = Splitter.on(',').splitToList(lines.get(0)); - checkArgument(firstLine.size() == 2, String.format( - "Line 1: Expected 2 elements, found %d", firstLine.size())); + checkArgument( + firstLine.size() == 2, + String.format("Line 1: Expected 2 elements, found %d", firstLine.size())); int version = Integer.parseInt(firstLine.get(0)); - checkArgument(version == 1, String.format( - "Line 1: Expected version 1, found %d", version)); + checkArgument(version == 1, String.format("Line 1: Expected version 1, found %d", version)); DateTime creationTime = DateTime.parse(firstLine.get(1)).withZone(UTC); - // Second line contains headers: smd-id,insertion-datetime - List secondLine = Splitter.on(',').splitToList(lines.get(1)); - checkArgument(secondLine.size() == 2, String.format( - "Line 2: Expected 2 elements, found %d", secondLine.size())); - checkArgument("smd-id".equals(secondLine.get(0)), String.format( - "Line 2: Expected header \"smd-id\", found \"%s\"", secondLine.get(0))); - checkArgument("insertion-datetime".equals(secondLine.get(1)), String.format( - "Line 2: Expected header \"insertion-datetime\", found \"%s\"", secondLine.get(1))); + // Note: we have to skip the first line because it contains the version metadata + CSVParser csv = + CSVFormat.Builder.create(CSVFormat.DEFAULT) + .setHeader() + .setSkipHeaderRecord(true) + .build() + .parse(new StringReader(Joiner.on('\n').join(lines.subList(1, lines.size())))); - // Subsequent lines: , - for (int i = 2; i < lines.size(); i++) { - List currentLine = Splitter.on(',').splitToList(lines.get(i)); - checkArgument(currentLine.size() == 2, String.format( - "Line %d: Expected 2 elements, found %d", i + 1, currentLine.size())); - String smdId = currentLine.get(0); - DateTime revokedTime = DateTime.parse(currentLine.get(1)); + for (CSVRecord record : csv) { + String smdId = record.get("smd-id"); + DateTime revokedTime = DateTime.parse(record.get("insertion-datetime")); revokes.put(smdId, revokedTime); } - return SignedMarkRevocationList.create(creationTime, revokes.build()); } } diff --git a/core/src/main/java/google/registry/tmch/TmchDnlAction.java b/core/src/main/java/google/registry/tmch/TmchDnlAction.java index 2d7954d83..1dd325318 100644 --- a/core/src/main/java/google/registry/tmch/TmchDnlAction.java +++ b/core/src/main/java/google/registry/tmch/TmchDnlAction.java @@ -16,6 +16,7 @@ package google.registry.tmch; import static google.registry.request.Action.Method.POST; +import com.google.common.collect.ImmutableList; import com.google.common.flogger.FluentLogger; import google.registry.keyring.api.KeyModule.Key; import google.registry.model.tmch.ClaimsList; @@ -24,7 +25,6 @@ import google.registry.request.Action; import google.registry.request.auth.Auth; import java.io.IOException; import java.security.GeneralSecurityException; -import java.util.List; import java.util.Optional; import javax.inject.Inject; import org.bouncycastle.openpgp.PGPException; @@ -49,13 +49,14 @@ public final class TmchDnlAction implements Runnable { /** Synchronously fetches latest domain name list and saves it to Cloud SQL. */ @Override public void run() { - List lines; + ClaimsList claims; try { - lines = marksdb.fetchSignedCsv(marksdbDnlLoginAndPassword, DNL_CSV_PATH, DNL_SIG_PATH); + ImmutableList lines = + marksdb.fetchSignedCsv(marksdbDnlLoginAndPassword, DNL_CSV_PATH, DNL_SIG_PATH); + claims = ClaimsListParser.parse(lines); } catch (GeneralSecurityException | IOException | PGPException e) { throw new RuntimeException(e); } - ClaimsList claims = ClaimsListParser.parse(lines); ClaimsListDao.save(claims); logger.atInfo().log( "Inserted %,d claims into the DB(s), created at %s.", diff --git a/core/src/main/java/google/registry/tmch/TmchSmdrlAction.java b/core/src/main/java/google/registry/tmch/TmchSmdrlAction.java index 837bf1ad8..ece110366 100644 --- a/core/src/main/java/google/registry/tmch/TmchSmdrlAction.java +++ b/core/src/main/java/google/registry/tmch/TmchSmdrlAction.java @@ -16,6 +16,7 @@ package google.registry.tmch; import static google.registry.request.Action.Method.POST; +import com.google.common.collect.ImmutableList; import com.google.common.flogger.FluentLogger; import google.registry.keyring.api.KeyModule.Key; import google.registry.model.smd.SignedMarkRevocationList; @@ -23,7 +24,6 @@ import google.registry.request.Action; import google.registry.request.auth.Auth; import java.io.IOException; import java.security.GeneralSecurityException; -import java.util.List; import java.util.Optional; import javax.inject.Inject; import org.bouncycastle.openpgp.PGPException; @@ -48,13 +48,14 @@ public final class TmchSmdrlAction implements Runnable { /** Synchronously fetches latest signed mark revocation list and saves it to the database. */ @Override public void run() { - List lines; + SignedMarkRevocationList smdrl; try { - lines = marksdb.fetchSignedCsv(marksdbSmdrlLoginAndPassword, SMDRL_CSV_PATH, SMDRL_SIG_PATH); + ImmutableList lines = + marksdb.fetchSignedCsv(marksdbSmdrlLoginAndPassword, SMDRL_CSV_PATH, SMDRL_SIG_PATH); + smdrl = SmdrlCsvParser.parse(lines); } catch (GeneralSecurityException | IOException | PGPException e) { throw new RuntimeException(e); } - SignedMarkRevocationList smdrl = SmdrlCsvParser.parse(lines); smdrl.save(); logger.atInfo().log( "Inserted %,d smd revocations into the database, created at %s.", diff --git a/core/src/test/java/google/registry/tmch/SmdrlCsvParserTest.java b/core/src/test/java/google/registry/tmch/SmdrlCsvParserTest.java index edad1b6ce..1971352d7 100644 --- a/core/src/test/java/google/registry/tmch/SmdrlCsvParserTest.java +++ b/core/src/test/java/google/registry/tmch/SmdrlCsvParserTest.java @@ -79,7 +79,7 @@ class SmdrlCsvParserTest { } @Test - void testOneRow() { + void testOneRow() throws Exception { SignedMarkRevocationList smdrl = SmdrlCsvParser.parse( ImmutableList.of( @@ -93,7 +93,7 @@ class SmdrlCsvParserTest { } @Test - void testEmpty() { + void testEmpty() throws Exception { SignedMarkRevocationList smdrl = SmdrlCsvParser.parse( ImmutableList.of("1,2014-11-24T23:30:04.3Z", "smd-id,insertion-datetime")); @@ -128,7 +128,9 @@ class SmdrlCsvParserTest { "1,2013-11-24T23:30:04.3Z", "lol,cat", "0000001681375789102250-65535,2013-08-09T12:00:00.0Z"))); - assertThat(thrown).hasMessageThat().contains("header"); + assertThat(thrown) + .hasMessageThat() + .isEqualTo("Mapping for smd-id not found, expected one of [lol, cat]"); } @Test @@ -142,6 +144,6 @@ class SmdrlCsvParserTest { "1,2013-11-24T23:30:04.3Z", "smd-id,insertion-datetime", "0000001681375789102250-65535,haha,2013-08-09T12:00:00.0Z"))); - assertThat(thrown).hasMessageThat().contains("elements"); + assertThat(thrown).hasMessageThat().isEqualTo("Invalid format: \"haha\""); } }