From c24f09febc71cefcb52f0a76a5684557eeda8276 Mon Sep 17 00:00:00 2001 From: Ben McIlwain Date: Mon, 22 Dec 2025 16:34:55 -0500 Subject: [PATCH] Don't call canonicalizeHostname() on nomulus command TLD args (#2908) The canonicalizeHostname() helper method is only suitable for use with domain names or host names. It does not work on bare TLDs, because a bare TLD can have hyphens in the third and fourth position without necessarily being an IDN. Note that the configure TLD command already correctly allows TLDs with such names to be created. Note that we are still enforcing that the TLDs to be added exist, so they have to pass all TLD naming requirements that are enforced on creating TLDs, and we are still lowercasing the TLD names passed as arguments here (though we're no longer punycoding them, although arguably that's not super useful on command-line params anyway). BUG= http://b/471013082 --- .../tools/CreateOrUpdateRegistrarCommand.java | 14 +++------ .../tools/UpdateRegistrarCommandTest.java | 31 +++++++++++++++++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java index db52912de..b07f3ef17 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java @@ -19,12 +19,12 @@ import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.base.Verify.verify; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; -import static google.registry.util.DomainNameUtils.canonicalizeHostname; import static google.registry.util.RegistrarUtils.normalizeRegistrarName; import static java.nio.charset.StandardCharsets.US_ASCII; import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameter; +import com.google.common.base.Ascii; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import google.registry.flows.certs.CertificateChecker; @@ -305,20 +305,16 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand { if (!allowedTlds.isEmpty()) { checkArgument( addAllowedTlds.isEmpty(), "Can't specify both --allowedTlds and --addAllowedTlds"); - ImmutableSet.Builder allowedTldsBuilder = new ImmutableSet.Builder<>(); - for (String allowedTld : allowedTlds) { - allowedTldsBuilder.add(canonicalizeHostname(allowedTld)); - } - builder.setAllowedTlds(allowedTldsBuilder.build()); + builder.setAllowedTlds( + allowedTlds.stream().map(Ascii::toLowerCase).collect(toImmutableSet())); } if (!addAllowedTlds.isEmpty()) { ImmutableSet.Builder allowedTldsBuilder = new ImmutableSet.Builder<>(); if (oldRegistrar != null) { allowedTldsBuilder.addAll(oldRegistrar.getAllowedTlds()); } - for (String allowedTld : addAllowedTlds) { - allowedTldsBuilder.add(canonicalizeHostname(allowedTld)); - } + allowedTldsBuilder.addAll( + addAllowedTlds.stream().map(Ascii::toLowerCase).collect(toImmutableSet())); builder.setAllowedTlds(allowedTldsBuilder.build()); } if (ipAllowList != null) { diff --git a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java index 9f7265306..796a356db 100644 --- a/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java @@ -124,6 +124,24 @@ class UpdateRegistrarCommandTest extends CommandTestCase .containsExactly("xn--q9jyb4c", "foobar"); } + @Test + void testSuccess_allowedTlds_tldNameWithHyphens() throws Exception { + persistRdapAbuseContact(); + createTlds("zz--main-1611", "foobar"); + persistResource( + loadRegistrar("NewRegistrar") + .asBuilder() + .setAllowedTlds(ImmutableSet.of("foobar")) + .build()); + runCommandInEnvironment( + RegistryToolEnvironment.PRODUCTION, + "--allowed_tlds=zz--main-1611,foobar", + "--force", + "NewRegistrar"); + assertThat(loadRegistrar("NewRegistrar").getAllowedTlds()) + .containsExactly("zz--main-1611", "foobar"); + } + @Test void testSuccess_addAllowedTlds() throws Exception { persistRdapAbuseContact(); @@ -142,6 +160,19 @@ class UpdateRegistrarCommandTest extends CommandTestCase .containsExactly("xn--q9jyb4c", "foo", "bar"); } + @Test + void testSuccess_addAllowedTlds_tldNameWithHyphens() throws Exception { + persistRdapAbuseContact(); + createTlds("foo", "bar", "zz--main-1611"); + runCommandInEnvironment( + RegistryToolEnvironment.PRODUCTION, + "--add_allowed_tlds=foo,bar", + "--force", + "NewRegistrar"); + assertThat(loadRegistrar("NewRegistrar").getAllowedTlds()) + .containsExactly("foo", "bar", "zz--main-1611"); + } + @Test void testSuccess_addAllowedTldsWithDupes() throws Exception { persistRdapAbuseContact();