From 4ce790d29ea55f2f7534767036690961c8674e4a Mon Sep 17 00:00:00 2001 From: Ben McIlwain Date: Wed, 17 Feb 2021 12:09:19 -0500 Subject: [PATCH] Fix obscure bug when checking restore prices of duplicate domain names (#968) * Fix obscure bug when checking restore prices of duplicate domain names There were instances of "java.lang.IllegalArgumentException: Multiple entries with same key" in the logs, caused by attempting to construct an ImmutableMap containing duplicate keys. It turns out this was happening in the domain check flow when the following conditions were all simultaneously met: 1. The older v06 fee extension is used 2. The same domain name is being queried multiple times in a single check command (which is valid per the spec but doesn't actually make any sense) 3. Said domain exists 4. The cost of a restore (an uncommon operation) is being checked When all of those conditions were met, an error was being thrown when the dupe-containing list of domain names was used as the keys of a new Map. This fixes that bug by calling .distinct() first. Give enough registrars enough typewriters ... BUG=179052195 --- .../flows/domain/DomainCheckFlow.java | 1 + .../flows/domain/DomainCheckFlowTest.java | 28 ++++ ..._check_fee_reserved_dupes_response_v12.xml | 147 +++++++++++++++++ .../domain_check_fee_reserved_dupes_v06.xml | 116 ++++++++++++++ .../domain_check_fee_reserved_dupes_v12.xml | 33 ++++ ..._check_fee_reserved_response_dupes_v06.xml | 149 ++++++++++++++++++ 6 files changed, 474 insertions(+) create mode 100644 core/src/test/resources/google/registry/flows/domain/domain_check_fee_reserved_dupes_response_v12.xml create mode 100644 core/src/test/resources/google/registry/flows/domain/domain_check_fee_reserved_dupes_v06.xml create mode 100644 core/src/test/resources/google/registry/flows/domain/domain_check_fee_reserved_dupes_v12.xml create mode 100644 core/src/test/resources/google/registry/flows/domain/domain_check_fee_reserved_response_dupes_v06.xml diff --git a/core/src/main/java/google/registry/flows/domain/DomainCheckFlow.java b/core/src/main/java/google/registry/flows/domain/DomainCheckFlow.java index d7fec83ca..2511cbbd4 100644 --- a/core/src/main/java/google/registry/flows/domain/DomainCheckFlow.java +++ b/core/src/main/java/google/registry/flows/domain/DomainCheckFlow.java @@ -301,6 +301,7 @@ public final class DomainCheckFlow implements Flow { feeCheck.getItems().stream() .filter(fc -> fc.getCommandName() == CommandName.RESTORE) .map(FeeCheckCommandExtensionItem::getDomainName) + .distinct() .collect(toImmutableList()); } else if (feeCheck.getItems().stream() .anyMatch(fc -> fc.getCommandName() == CommandName.RESTORE)) { diff --git a/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java index d65d8977e..e1ba51ce2 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCheckFlowTest.java @@ -887,6 +887,20 @@ class DomainCheckFlowTest extends ResourceCheckFlowTestCase + + + Command completed successfully + + + + + reserved.tld + Reserved + + + allowedinsunrise.tld + In use + + + allowedinsunrise.tld + In use + + + premiumcollision.tld + Cannot be delegated + + + + + + USD + + + reserved.tld + + + 1 + reserved + + + + + reserved.tld + + + 1 + 11.00 + + + + + reserved.tld + + + 1 + 11.00 + + + + + reserved.tld + + + 1 + 17.00 + + + + + allowedinsunrise.tld + + + 1 + reserved + + + + + allowedinsunrise.tld + + + 1 + 11.00 + + + + + allowedinsunrise.tld + + + 1 + 11.00 + + + + + allowedinsunrise.tld + + + 1 + 17.00 + + + + + premiumcollision.tld + + + 1 + reserved + + + + + premiumcollision.tld + + + 1 + 70.00 + premium + + + + + premiumcollision.tld + + + 1 + 70.00 + premium + + + + + premiumcollision.tld + + + 1 + 17.00 + + + + + + ABC-12345 + server-trid + + + diff --git a/core/src/test/resources/google/registry/flows/domain/domain_check_fee_reserved_dupes_v06.xml b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_reserved_dupes_v06.xml new file mode 100644 index 000000000..3700f42fe --- /dev/null +++ b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_reserved_dupes_v06.xml @@ -0,0 +1,116 @@ + + + + + reserved.tld + allowedinsunrise.tld + allowedinsunrise.tld + premiumcollision.tld + + + + + custom + + + + reserved.tld + USD + create + 1 + + + reserved.tld + USD + renew + 1 + + + reserved.tld + USD + transfer + 1 + + + reserved.tld + USD + restore + 1 + + + allowedinsunrise.tld + USD + create + 1 + + + allowedinsunrise.tld + USD + renew + 1 + + + allowedinsunrise.tld + USD + transfer + 1 + + + allowedinsunrise.tld + USD + restore + 1 + + + allowedinsunrise.tld + USD + create + 1 + + + allowedinsunrise.tld + USD + renew + 1 + + + allowedinsunrise.tld + USD + transfer + 1 + + + allowedinsunrise.tld + USD + restore + 1 + + + premiumcollision.tld + USD + create + 1 + + + premiumcollision.tld + USD + renew + 1 + + + premiumcollision.tld + USD + transfer + 1 + + + premiumcollision.tld + USD + restore + 1 + + + + ABC-12345 + + diff --git a/core/src/test/resources/google/registry/flows/domain/domain_check_fee_reserved_dupes_v12.xml b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_reserved_dupes_v12.xml new file mode 100644 index 000000000..cee487122 --- /dev/null +++ b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_reserved_dupes_v12.xml @@ -0,0 +1,33 @@ + + + + + reserved.tld + allowedinsunrise.tld + allowedinsunrise.tld + premiumcollision.tld + + + + + custom + + + USD + + 1 + + + 1 + + + 1 + + + 1 + + + + ABC-12345 + + diff --git a/core/src/test/resources/google/registry/flows/domain/domain_check_fee_reserved_response_dupes_v06.xml b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_reserved_response_dupes_v06.xml new file mode 100644 index 000000000..55247107e --- /dev/null +++ b/core/src/test/resources/google/registry/flows/domain/domain_check_fee_reserved_response_dupes_v06.xml @@ -0,0 +1,149 @@ + + + + Command completed successfully + + + + + reserved.tld + Reserved + + + allowedinsunrise.tld + In use + + + allowedinsunrise.tld + In use + + + premiumcollision.tld + Cannot be delegated + + + + + + + reserved.tld + USD + create + 1 + reserved + + + reserved.tld + USD + renew + 1 + 11.00 + + + reserved.tld + USD + transfer + 1 + 11.00 + + + reserved.tld + USD + restore + 1 + 17.00 + + + allowedinsunrise.tld + USD + create + 1 + reserved + + + allowedinsunrise.tld + USD + renew + 1 + 11.00 + + + allowedinsunrise.tld + USD + transfer + 1 + 11.00 + + + allowedinsunrise.tld + USD + restore + 1 + 17.00 + + + allowedinsunrise.tld + USD + create + 1 + reserved + + + allowedinsunrise.tld + USD + renew + 1 + 11.00 + + + allowedinsunrise.tld + USD + transfer + 1 + 11.00 + + + allowedinsunrise.tld + USD + restore + 1 + 17.00 + + + premiumcollision.tld + USD + create + 1 + reserved + + + premiumcollision.tld + USD + renew + 1 + 70.00 + premium + + + premiumcollision.tld + USD + transfer + 1 + 70.00 + premium + + + premiumcollision.tld + USD + restore + 1 + 17.00 + + + + + ABC-12345 + server-trid + + +