Standardize hostname handling in URS command (#1886)

This commit is contained in:
Ben McIlwain 2022-12-19 16:22:52 -05:00 committed by GitHub
parent f9d1945787
commit 6a1fd3491f
7 changed files with 73 additions and 21 deletions

View file

@ -40,8 +40,8 @@ import google.registry.model.transfer.DomainTransferData;
import google.registry.model.transfer.TransferData;
import google.registry.model.transfer.TransferStatus;
import google.registry.persistence.VKey;
import java.util.Collection;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import java.util.function.Function;
import java.util.function.Supplier;
@ -183,7 +183,7 @@ public final class EppResourceUtils {
* @param now the logical time of the check
*/
public static <T extends EppResource> ImmutableSet<String> checkResourcesExist(
Class<T> clazz, List<String> uniqueIds, final DateTime now) {
Class<T> clazz, Collection<String> uniqueIds, final DateTime now) {
return ForeignKeyUtils.load(clazz, uniqueIds, now).keySet();
}

View file

@ -35,11 +35,12 @@ import google.registry.model.domain.Domain;
import google.registry.model.domain.secdns.DomainDsData;
import google.registry.model.eppcommon.StatusValue;
import google.registry.model.host.Host;
import google.registry.tools.params.NameserversParameter;
import google.registry.tools.soy.DomainRenewSoyInfo;
import google.registry.tools.soy.UniformRapidSuspensionSoyInfo;
import google.registry.util.Clock;
import google.registry.util.DomainNameUtils;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
@ -69,9 +70,12 @@ final class UniformRapidSuspensionCommand extends MutatingEppToolCommand {
@Parameter(
names = {"-h", "--hosts"},
description = "Comma-delimited set of fully qualified host names to replace the current hosts"
+ " on the domain.")
private List<String> newHosts = new ArrayList<>();
description =
"Comma-delimited set of fully qualified host names to replace the current hosts"
+ " on the domain.",
converter = NameserversParameter.class,
validateWith = NameserversParameter.class)
private Set<String> newHosts = new HashSet<>();
@Parameter(
names = {"-s", "--dsdata"},
@ -126,14 +130,10 @@ final class UniformRapidSuspensionCommand extends MutatingEppToolCommand {
protected void initMutatingEppToolCommand() {
superuser = true;
DateTime now = clock.nowUtc();
ImmutableList<String> newCanonicalHosts =
newHosts.stream().map(DomainNameUtils::canonicalizeHostname).collect(toImmutableList());
ImmutableSet<String> newHostsSet = ImmutableSet.copyOf(newCanonicalHosts);
Optional<Domain> domainOpt = loadByForeignKey(Domain.class, domainName, now);
checkArgumentPresent(domainOpt, "Domain '%s' does not exist or is deleted", domainName);
Domain domain = domainOpt.get();
Set<String> missingHosts =
difference(newHostsSet, checkResourcesExist(Host.class, newCanonicalHosts, now));
Set<String> missingHosts = difference(newHosts, checkResourcesExist(Host.class, newHosts, now));
checkArgument(missingHosts.isEmpty(), "Hosts do not exist: %s", missingHosts);
checkArgument(
locksToPreserve.isEmpty() || undo,
@ -187,9 +187,9 @@ final class UniformRapidSuspensionCommand extends MutatingEppToolCommand {
"domainName",
domainName,
"hostsToAdd",
difference(newHostsSet, existingNameservers),
difference(newHosts, existingNameservers),
"hostsToRemove",
difference(existingNameservers, newHostsSet),
difference(existingNameservers, newHosts),
"statusesToApply",
statusesToApply,
"statusesToRemove",

View file

@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.re2j.Matcher;
import com.google.re2j.Pattern;
import google.registry.util.DomainNameUtils;
import java.util.Set;
import java.util.stream.Stream;
@ -50,12 +51,9 @@ public final class NameserversParameter extends ParameterConverterValidator<Set<
if (Strings.isNullOrEmpty(value)) {
return ImmutableSet.of();
}
return Splitter.on(',')
.trimResults()
.omitEmptyStrings()
.splitToList(value)
.stream()
return Splitter.on(',').trimResults().omitEmptyStrings().splitToList(value).stream()
.flatMap(NameserversParameter::splitNameservers)
.map(DomainNameUtils::canonicalizeHostname)
.collect(toImmutableSet());
}

View file

@ -1006,7 +1006,6 @@ public final class DatabaseHelper {
*
* <p>This was coded for testing RDE since its queries depend on the associated entries.
*
*
* @see #persistResource(ImmutableObject)
*/
public static <R extends EppResource> R persistEppResource(final R resource) {

View file

@ -57,6 +57,23 @@ class CreateDomainCommandTest extends EppToolCommandTestCase<CreateDomainCommand
eppVerifier.verifySent("domain_create_complete.xml");
}
@Test
void testSuccess_completeWithCanonicalization() throws Exception {
runCommandForced(
"--client=NewRegistrar",
"--period=1",
"--nameservers=NS1.zdns.google,ns2.ZDNS.google,ns3.zdns.gOOglE,ns4.zdns.google",
"--registrant=crr-admin",
"--admins=crr-admin",
"--techs=crr-tech",
"--password=2fooBAR",
"--ds_records=1 2 2 9F86D081884C7D659A2FEAA0C55AD015A3BF4F1B2B0B822CD15D6C15B0F00A08,4 5 1"
+ " A94A8FE5CCB19BA61C4C0873D391E987982FBBD3",
"--ds_records=60485 5 2 D4B7D520E7BB5F0F67674A0CCEB1E3E0614B93C4F9E99B8383F6A1E4469DA50A",
"example.tld");
eppVerifier.verifySent("domain_create_complete.xml");
}
@Test
void testSuccess_completeWithSquareBrackets() throws Exception {
runCommandForced(
@ -74,6 +91,23 @@ class CreateDomainCommandTest extends EppToolCommandTestCase<CreateDomainCommand
eppVerifier.verifySent("domain_create_complete.xml");
}
@Test
void testSuccess_completeWithSquareBracketsAndCanonicalization() throws Exception {
runCommandForced(
"--client=NewRegistrar",
"--period=1",
"--nameservers=NS[1-4].zdns.google",
"--registrant=crr-admin",
"--admins=crr-admin",
"--techs=crr-tech",
"--password=2fooBAR",
"--ds_records=1 2 2 9F86D081884C7D659A2FEAA0C55AD015A3BF4F1B2B0B822CD15D6C15B0F00A08,4 5 1"
+ " A94A8FE5CCB19BA61C4C0873D391E987982FBBD3",
"--ds_records=60485 5 2 D4B7D520E7BB5F0F67674A0CCEB1E3E0614B93C4F9E99B8383F6A1E4469DA50A",
"example.tld");
eppVerifier.verifySent("domain_create_complete.xml");
}
@Test
void testSuccess_minimal() throws Exception {
// Test that each optional field can be omitted. Also tests the auto-gen password.

View file

@ -163,6 +163,27 @@ class UniformRapidSuspensionCommandTest
assertInStdout("--restore_client_hold");
}
@Test
void testCommand_bracketNameserverNotationWithCanonicalization() throws Exception {
persistDomainWithHosts(defaultDomain, defaultDsData, ns1, ns2);
runCommandForced(
"--domain_name=evil.tld",
"--hosts=URS[1-2].example.com",
"--dsdata=1 1 1 A94A8FE5CCB19BA61C4C0873D391E987982FBBD3",
"--renew_one_year=false");
eppVerifier
.expectRegistrarId("CharlestonRoad")
.expectSuperuser()
.verifySent("uniform_rapid_suspension.xml")
.verifyNoMoreSent();
assertInStdout("uniform_rapid_suspension --undo");
assertInStdout("--domain_name evil.tld");
assertInStdout("--hosts ns1.example.com,ns2.example.com");
assertInStdout("--dsdata 1 2 3 DEAD,4 5 6 BEEF");
assertNotInStdout("--locks_to_preserve");
assertNotInStdout("--restore_client_hold");
}
@Test
void testUndo_removesLocksReplacesHostsAndDsData() throws Exception {
persistDomainWithHosts(defaultDomain, defaultDsData, urs1, urs2);

View file

@ -93,10 +93,10 @@ class UpdateDomainCommandTest extends EppToolCommandTestCase<UpdateDomainCommand
}
@Test
void testSuccess_completeWithSquareBrackets() throws Exception {
void testSuccess_completeWithSquareBracketsAndCanonicalization() throws Exception {
runCommandForced(
"--client=NewRegistrar",
"--add_nameservers=ns[1-2].zdns.google",
"--add_nameservers=NS[1-2].zdns.google",
"--add_admins=crr-admin2",
"--add_techs=crr-tech2",
"--add_statuses=serverDeleteProhibited",