Allow double hyphens in 3rd&4th position in all domain operations (#2909)

This is a follow-up to PR #2908, which relaxed this restriction on bare TLDs
only, but now we also allow it systemwide on domains and hostnames as well.  The
rules against hyphens in these positions are still enforced on all parts of the
domain name except the last one. Correct handling of multi-part TLDs in this
regard is out of scope in this PR; a multi-part TLD that looked something like
".zz--foobar.foobar" would still fail validation. (But of course you cannot a
priori know just from looking at a 3-part string whether it might be a hostname
on a normal TLD, or a domain name on a 2-part TLD.)

This also has some annoying interactions with a trailing dot (indicating the
root), which need to be preserved, but otherwise don't affect how TLD validation
is handled.

BUG= http://b/471013082

config/nom_build.py

@@ -117,28 +117,19 @@ PROPERTIES = [
     Property('dbUser', 'Database user name for use in connection'),
     Property('dbPassword', 'Database password for use in connection'),
 
-    Property('publish_repo',
-             'Maven repository that hosts the Cloud SQL schema jar and the '
-             'registry server test jars. Such jars are needed for '
-             'server/schema integration tests. Please refer to <a '
-             'href="./integration/README.md">integration project</a> for more '
-             'information.'),
-    Property('baseSchemaTag',
-             'The nomulus version tag of the schema for use in the schema'
-             'deployment integration test (:db:schemaIncrementalDeployTest)'),
-    Property('schema_version',
-             'The nomulus version tag of the schema for use in a database'
-             'integration test.'),
-    Property('nomulus_version',
-             'The version of nomulus to test against in a database '
-             'integration test.'),
     Property('dot_path',
              'The path to "dot", part of the graphviz package that converts '
              'a BEAM pipeline to image. Setting this property to empty string '
              'will disable image generation.',
              '/usr/bin/dot'),
     Property('pipeline',
-             'The name of the Beam pipeline being staged.')
+             'The name of the Beam pipeline being staged.'),
+    Property('nomulus_env',
+             'For use by scripts. Normally not set manually.'),
+    Property('schema_env',
+             'For use by scripts. Normally not set manually.'),
+    Property('schemaTestArtifactsDir',
+             'For use by scripts. Normally not set manually.')
 ]
 
 GRADLE_FLAGS = [

core/build.gradle

@@ -646,23 +646,6 @@ artifacts {
   nomulus_test testUberJar
 }
 
-publishing {
-  repositories {
-    maven {
-      url project.publish_repo
-    }
-  }
-  publications {
-    nomulusTestsPublication(MavenPublication) {
-      groupId 'google.registry'
-      artifactId 'nomulus_test'
-      version project.nomulus_version
-      artifact nomulusFossJar
-      artifact testUberJar
-    }
-  }
-}
-
 task buildToolImage(dependsOn: nomulus, type: Exec) {
   commandLine 'docker', 'build', '-t', 'nomulus-tool', '.'
 }

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<String> 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<String> 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) {

core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java

@@ -124,6 +124,24 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
         .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<UpdateRegistrarCommand>
         .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();

db/build.gradle

@@ -126,22 +126,6 @@ artifacts {
   schema schemaJar
 }
 
-publishing {
-  repositories {
-    maven {
-      url project.publish_repo
-    }
-  }
-  publications {
-    sqlSchemaPublication(MavenPublication) {
-      groupId 'google.registry'
-      artifactId 'schema'
-      version project.schema_version
-      artifact schemaJar
-    }
-  }
-}
-
 // Adds flyway tasks such as: flywayInfo, flywayValidate, flywayMigrate (
 // deploying the schema in local repository), and flywayClean (dropping all data
 // in the database). The latter two commands are disallowed in environments
@@ -248,14 +232,10 @@ if (ext.isRestricted()) {
   }
 }
 
-if (project.baseSchemaTag != '') {
-  repositories {
-    maven {
-      url project.publish_repo
-    }
-  }
+if (project.schemaTestArtifactsDir != '') {
   dependencies {
-    integration "google.registry:schema:${project.baseSchemaTag}"
+    // For schemaIncrementalDeployTest, which only uses sandbox schema
+    integration files("${project.schemaTestArtifactsDir}/schema.sandbox.jar")
   }
 
   // Checks if Flyway scripts can be deployed to an existing database with

gradle.properties

@@ -22,9 +22,8 @@ dbServer=
 dbName=postgres
 dbUser=
 dbPassword=
-publish_repo=
-baseSchemaTag=
-schema_version=
-nomulus_version=
 dot_path=/usr/bin/dot
 pipeline=
+nomulus_env=
+schema_env=
+schemaTestArtifactsDir=

integration/README.md

@@ -1,50 +1,38 @@
 ## Summary
 
-This project runs cross-version server/schema integration tests with arbitrary
-version pairs. It may be used by presubmit tests and continuous-integration
-tests, or as a gating test during release and/or deployment.
+This subproject provides two integration tests that ensure schema safety:
 
-## Maven Dependencies
+*  One test checks for edits to Flyway scripts already deployed to Sandbox
+   Production. Such edits will cause schema deployment failure.
+*  Another test runs cross-version server/schema integration tests between a
+   pull request and the deployed release in Sandbox or Production. If a pull
+   request fails this test, it either contains schema changes not compatible
+   with Sandbox/Production binaries, or binaries not compatible with the
+   current schema in Sandbox/Production. This test may be include in presubmit
+   testing.
 
-This release process is expected to publish the following Maven dependencies to
-a well-known repository:
+## Test Artifacts
 
-*   google.registry:schema, which contains the schema DDL scripts. This is done
-    by the ':db:publish' task.
-*   google.registry:nomulus_test, which contains the nomulus classes and
-    dependencies needed for the integration tests. This is done by the
-    ':core:publish' task.
+To support the tests above, each release generates the following test artifacts:
 
-After each deployment in sandbox or production, the deployment process is
-expected to save the version tag of the binary or schema along with the
-environment. These tags will be made available to test runners.
+*  schema.jar: The flyway scripts.
+*  nomulus-public.jar:  The open-source java classes.
+*  nomulus-tests-alldeps.jar: Uber jar with schema test classes and all
+   third-party dependencies.
+
+After each deployment in sandbox or production, the deployment process copies
+these artifacts to a well-known location, and appends the environment tag to
+the file names.
 
 ## Usage
 
-The ':integration:sqlIntegrationTest' task is the test runner. It uses the
-following properties:
+Use the convenience scripts in the `integration` folder to run the tests. 
 
-*   nomulus_version: a Registry server release tag, or 'local' if the code in
-    the local Git tree should be used.
-*   schema_version: a schema release tag, or 'local' if the code in the local
-    Git tree should be used.
-*   publish_repo: the Maven repository where release jars may be found. This is
-    required if neither of the above is 'local'.
+```bash
+./integration/run_schema_check.sh -p domain-registry-dev
 
-Given a program 'fetch_version_tag' that retrieves the currently deployed
-version tag of SQL schema or server binary in a particular environment (which as
-mentioned earlier are saved by the deployment process), the following code
-snippet checks if the current PR or local clone has schema changes, and if yes,
-tests the production server's version with the new schema.
-
-```shell
-current_prod_schema=$(fetch_version_tag schema production)
-current_prod_server=$(fetch_version_tag server production)
-schema_changes=$(git diff ${current_prod_schema} --name-only \
-  ./db/src/main/resources/sql/flyway/ | wc -l)
-[[ schema_changes -gt  0 ]] && ./gradlew :integration:sqlIntegrationTest \
-  -Ppublish_repo=${REPO} -Pschema_version=local \
-  -Pnomulus_version=current_prod_server
+./integration/run_compatibility_tests.sh -p domain-registry-dev -s sql
+./integration/run_compatibility_tests.sh -p domain-registry-dev -s nomulus
 ```
 
 ## Implementation Notes

integration/build.gradle

@@ -18,22 +18,16 @@
 import static com.google.common.base.Preconditions.checkArgument
 import static com.google.common.base.Strings.isNullOrEmpty
 
-if (schema_version == '' || nomulus_version == '') {
+if (schema_env == '' || nomulus_env == '') {
   return
 }
 
 def USE_LOCAL = 'local'
 
-if (schema_version != USE_LOCAL || nomulus_version != USE_LOCAL) {
+if (schema_env != USE_LOCAL || nomulus_env != USE_LOCAL) {
   checkArgument(
-      !isNullOrEmpty(publish_repo),
-      'The publish_repo is required when remote jars are needed.')
-
-  repositories {
-    maven {
-      url project.publish_repo
-    }
-  }
+      !isNullOrEmpty(schemaTestArtifactsDir),
+      'The schemaTestArtifactsDir is required when deployed jars are needed.')
 }
 
 def testUberJarName = ''
@@ -41,25 +35,25 @@ def testUberJarName = ''
 // Might need to add this back if we re-add nebula-lint
 // gradleLint.ignore('unused-dependency') {
 dependencies {
-  if (schema_version == USE_LOCAL) {
+  if (schema_env == USE_LOCAL) {
     testRuntimeOnly project(path: ':db', configuration: 'schema')
   } else {
-    testRuntimeOnly "google.registry:schema:${schema_version}"
+    testRuntimeOnly files("${project.schemaTestArtifactsDir}/schema.${schema_env}.jar")
   }
-  if (nomulus_version == USE_LOCAL) {
+  if (nomulus_env == USE_LOCAL) {
     testRuntimeOnly project(path: ':core', configuration: 'nomulus_test')
     testUberJarName = 'nomulus-tests-alldeps.jar'
   } else {
-    testRuntimeOnly "google.registry:nomulus_test:${nomulus_version}:public"
-    testRuntimeOnly "google.registry:nomulus_test:${nomulus_version}:alldeps"
-    testUberJarName = "nomulus_test-${nomulus_version}-alldeps.jar"
+    testRuntimeOnly files("${project.schemaTestArtifactsDir}/nomulus-public.${nomulus_env}.jar")
+    testRuntimeOnly files("${project.schemaTestArtifactsDir}/nomulus-tests-alldeps.${nomulus_env}.jar")
+    testUberJarName = "nomulus-tests-alldeps.${nomulus_env}.jar"
   }
 }
 // }
 
 configurations.testRuntimeOnly.transitive = false
 
-def unpackedTestDir = "${projectDir}/build/unpackedTests/${nomulus_version}"
+def unpackedTestDir = "${projectDir}/build/unpackedTests/${nomulus_env}"
 
 // Extracts SqlIntegrationTestSuite.class to a temp folder. Gradle's test
 // runner only looks for runnable tests on a regular file system. However,
@@ -78,7 +72,7 @@ task extractSqlIntegrationTestSuite (type: Copy) {
   into unpackedTestDir
   includeEmptyDirs = false
 
-  if (nomulus_version == USE_LOCAL) {
+  if (nomulus_env == USE_LOCAL) {
     dependsOn ':core:testUberJar'
   }
 }

integration/run_compatibility_tests.sh

@@ -56,37 +56,50 @@ function runTest() {
   local deployed_system=${1}
   local version=${2}
   local dev_project=${3}
+  local env=${4}
 
-  local changes=$(getChangeCountSinceVersion ${deployed_system} ${version})
-  if [[ ${changes} = 0 ]]; then
-    echo "No relevant changes in ${deployed_system} since ${version}"
-    return 0
+#  local changes=$(getChangeCountSinceVersion ${deployed_system} ${version})
+#  if [[ ${changes} = 0 ]]; then
+#    echo "No relevant changes in ${deployed_system} since ${version}"
+#    return 0
+#  fi
+#
+#  echo "Found relevant changes in ${deployed_system} since ${version}"
+
+  if [[ -n "${SCHEMA_TEST_ARTIFACTS_DIR}" ]]; then
+    echo "Using schema test jars downloaded to ${SCHEMA_TEST_ARTIFACTS_DIR}"
+  else
+    SCHEMA_TEST_ARTIFACTS_DIR=$(mktemp -d)
+    echo "Created working dir ${SCHEMA_TEST_ARTIFACTS_DIR} for downloaded test jars."
+    trap 'rm -rf ${SCHEMA_TEST_ARTIFACTS_DIR}' EXIT
+    gcloud storage cp --verbosity=none \
+        "gs://${DEV_PROJECT}-deployed-tags/schema-test-artifacts/*.jar" \
+        "${SCHEMA_TEST_ARTIFACTS_DIR}"
   fi
 
-  echo "Found relevant changes in ${deployed_system} since ${version}"
-
-  local nomulus_version
-  local schema_version
+  local nomulus_env
+  local schema_env
 
   if [[ ${deployed_system} = "sql" ]]; then
-    schema_version=${version}
-    nomulus_version="local"
+    schema_env=${env}
+    nomulus_env="local"
   else
-    nomulus_version=${version}
-    schema_version="local"
+    nomulus_env=${env}
+    schema_env="local"
   fi
 
-  echo "Running test with -Pnomulus_version=${nomulus_version}" \
-      "-Pschema_version=${schema_version}"
+  echo "Running test with -Pnomulus_env=${nomulus_env}" \
+      "-Pschema_env=${schema_env}" \
+      "-PschemaTestArtifactsDir=${SCHEMA_TEST_ARTIFACTS_DIR}" \
 
   # The https scheme in the Maven repo URL below is required for Kokoro. See
   # ./run_schema_check.sh for more information.
   (cd ${SCRIPT_DIR}/..; \
       ./gradlew :integration:sqlIntegrationTest \
           -PdevProject=${dev_project} \
-          -Pnomulus_version=${nomulus_version} \
-          -Pschema_version=${schema_version} \
-          -Ppublish_repo=https://storage.googleapis.com/${dev_project}-deployed-tags/maven)
+          -Pnomulus_env=${nomulus_env} \
+          -Pschema_env=${schema_env} \
+          -PschemaTestArtifactsDir=${SCHEMA_TEST_ARTIFACTS_DIR})
 }
 
 set -e
@@ -128,19 +141,16 @@ if [[ -z "${ENV}" ]]; then
   SANDBOX_VERSION=$(fetchVersion ${DEPLOYED_SYSTEM} sandbox ${DEV_PROJECT})
   PROD_VERSION=$(fetchVersion ${DEPLOYED_SYSTEM} production ${DEV_PROJECT})
   if [[ ${SANDBOX_VERSION} = ${PROD_VERSION} ]]; then
-    VERSIONS=(${PROD_VERSION})
     echo "- sandbox and production at ${PROD_VERSION}"
+    runTest ${DEPLOYED_SYSTEM} ${SANDBOX_VERSION} ${DEV_PROJECT} sandbox
   else
-    VERSIONS=(${PROD_VERSION} ${SANDBOX_VERSION})
     echo "- sandbox at ${SANDBOX_VERSION}"
+    runTest ${DEPLOYED_SYSTEM} ${SANDBOX_VERSION} ${DEV_PROJECT} sandbox
     echo "- production at ${PROD_VERSION}"
+    runTest ${DEPLOYED_SYSTEM} ${PROD_VERSION} ${DEV_PROJECT} production
   fi
 else
   TARGET_VERSION=$(fetchVersion ${DEPLOYED_SYSTEM} ${ENV} ${DEV_PROJECT})
-  VERSIONS=(${TARGET_VERSION})
-    echo "- ${ENV} at ${TARGET_VERSION}"
+  echo "- ${ENV} at ${TARGET_VERSION}"
+  runTest ${DEPLOYED_SYSTEM} ${TARGET_VERSION} ${DEV_PROJECT} ${ENV}
 fi
-
-for v in "${VERSIONS[@]}"; do
-  runTest ${DEPLOYED_SYSTEM} ${v} ${DEV_PROJECT}
-done

integration/run_schema_check.sh

@@ -67,13 +67,17 @@ fi
 sandbox_tag=$(fetchVersion sql sandbox ${DEV_PROJECT})
 echo "Checking Flyway scripts against schema in Sandbox (${sandbox_tag})."
 
-# The URL of the Maven repo on GCS for the publish_repo parameter must use the
-# https scheme (https://storage.googleapis.com/{BUCKET}/{PATH}) in order to work
-# with Kokoro. Gradle's alternative gcs scheme does not work on Kokoro: a GCP
-# credential with proper scopes for GCS access is required even for public
-# buckets, however, Kokoro VM instances are not set up with such credentials.
-# Incidentally, gcs can be used on Cloud Build.
+if [[ -n "${SCHEMA_TEST_ARTIFACTS_DIR}" ]]; then
+  echo "Using schema test jars downloaded to ${SCHEMA_TEST_ARTIFACTS_DIR}"
+else
+  SCHEMA_TEST_ARTIFACTS_DIR=$(mktemp -d)
+  echo "Created working dir ${SCHEMA_TEST_ARTIFACTS_DIR} for downloaded test jars."
+  trap 'rm -rf ${SCHEMA_TEST_ARTIFACTS_DIR}' EXIT
+  gcloud storage cp --verbosity=none \
+      "gs://${DEV_PROJECT}-deployed-tags/schema-test-artifacts/*.jar" \
+      "${SCHEMA_TEST_ARTIFACTS_DIR}"
+fi
+
 (cd ${SCRIPT_DIR}/..; \
     ./gradlew :db:schemaIncrementalDeployTest \
-        -PbaseSchemaTag=${sandbox_tag} \
-        -Ppublish_repo=https://storage.googleapis.com/${DEV_PROJECT}-deployed-tags/maven)
+        -PschemaTestArtifactsDir="${SCHEMA_TEST_ARTIFACTS_DIR}")

release/cloudbuild-nomulus.yaml

@@ -154,8 +154,7 @@ steps:
       google/registry/beam/resave_all_epp_resources_pipeline_metadata.json \
       google.registry.beam.wipeout.WipeOutContactHistoryPiiPipeline \
       google/registry/beam/wipe_out_contact_history_pii_pipeline_metadata.json
-# Tentatively build and publish Cloud SQL schema jar here, before the schema release process is
-# finalized. Also publish nomulus:core jars that are needed for server/schema compatibility tests.
+# Build and upload the schema jar as well as other artifacts needed by the schema tests.
 - name: 'gcr.io/${PROJECT_ID}/builder:latest'
   entrypoint: /bin/bash
   # Set home for Gradle caches. Must be consistent with previous steps above
@@ -165,20 +164,8 @@ steps:
   - -c
   - |
     set -e
-    ./gradlew \
-      :db:publish \
-      -PmavenUrl=gcs://domain-registry-maven-repository/maven \
-      -PpluginsUrl=gcs://domain-registry-maven-repository/plugins \
-      -Ppublish_repo=gcs://${PROJECT_ID}-deployed-tags/maven \
-      -Pschema_version=${TAG_NAME}
-    ./gradlew \
-      :core:publish \
-      -PmavenUrl=gcs://domain-registry-maven-repository/maven \
-      -PpluginsUrl=gcs://domain-registry-maven-repository/plugins \
-      -Ppublish_repo=gcs://${PROJECT_ID}-deployed-tags/maven \
-      -Pnomulus_version=${TAG_NAME}
-    # Upload jars for use by schema deployment and schema compatibility test.
-    # The jars published to GCS will be unaccessible to Kokoro once public access is removed.
+    ./gradlew :db:schemaJar
+    ./gradlew :core:nomulusFossJar :core:testUberJar
     cp db/build/libs/schema.jar output/
     cp core/build/libs/nomulus-public.jar output/
     cp core/build/libs/nomulus-tests-alldeps.jar output/

util/src/main/java/google/registry/util/DomainNameUtils.java

@@ -19,9 +19,11 @@ import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 
 import com.google.common.base.Ascii;
 import com.google.common.base.Joiner;
+import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.net.InternetDomainName;
+import java.util.List;
 
 /** Utility methods related to domain names. */
 public final class DomainNameUtils {
@@ -39,14 +41,34 @@ public final class DomainNameUtils {
             .equals(potentialParent.parts());
   }
 
-  /** Canonicalizes a hostname/domain name by lowercasing and converting unicode to punycode. */
+  /**
+   * Canonicalizes a hostname/domain name by lowercasing and converting Unicode to punycode.
+   *
+   * <p>This applies slightly stricter rules to all labels other than the TLD part (all other labels
+   * are not allowed to have hyphens in the third and fourth position except when using
+   * ACE-formatted Punycode). This restriction is not enforced on the last label (so multi-part TLDs
+   * still cannot have said characters except on the last part).
+   */
   public static String canonicalizeHostname(String label) {
     String labelLowercased = Ascii.toLowerCase(label);
-    try {
-      return Idn.toASCII(labelLowercased);
-    } catch (IllegalArgumentException e) {
-      throw new IllegalArgumentException(String.format("Error ASCIIfying label '%s'", label), e);
+    String finalChar = "";
+    if (labelLowercased.endsWith(".")) {
+      labelLowercased = labelLowercased.substring(0, labelLowercased.length() - 1);
+      finalChar = ".";
     }
+    List<String> parts = Splitter.on('.').splitToList(labelLowercased);
+    // If the hostname only has one part, just canonicalize that.
+    if (parts.size() == 1) {
+      return Idn.toASCII(parts.getFirst()) + finalChar;
+    }
+    // If the hostname has multiple parts, apply stricter validation to all labels but the last
+    // one (which relaxes the hyphens in third and fourth positions rule).
+    StringBuilder sb = new StringBuilder();
+    for (int i = 0; i < parts.size() - 1; i++) {
+      sb.append(Idn.toASCII(parts.get(i))).append('.');
+    }
+    sb.append(Idn.tldToASCII(parts.getLast())).append(finalChar);
+    return sb.toString();
   }
 
   /**

util/src/main/java/google/registry/util/Idn.java

@@ -14,9 +14,15 @@
 
 package google.registry.util;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
 import com.ibm.icu.text.IDNA;
+import com.ibm.icu.text.IDNA.Error;
 import com.ibm.icu.text.IDNA.Info;
+import java.util.Set;
 
 /**
  * A partial API-compatible replacement for {@link java.net.IDN} that replaces <a
@@ -51,11 +57,29 @@ public final class Idn {
     StringBuilder result = new StringBuilder();
     UTS46_INSTANCE.nameToASCII(name, result, info);
     if (info.hasErrors()) {
-      throw new IllegalArgumentException("Errors: " + Joiner.on(',').join(info.getErrors()));
+      throw new IllegalArgumentException(
+          String.format(
+              "Errors ASCIIfying label %s: %s", name, Joiner.on(',').join(info.getErrors())));
     }
     return result.toString();
   }
 
+  /**
+   * Translates a TLD string from Unicode to Punycoded ASCII.
+   *
+   * <p>Unlike {@link #toASCII}, this method does NOT enforce the restriction that hyphens may only
+   * be present on the third and fourth characters for "xn--" ACE-formatted domains.
+   */
+  public static String tldToASCII(String name) {
+    Info info = new Info();
+    StringBuilder result = new StringBuilder();
+    UTS46_INSTANCE.nameToASCII(name, result, info);
+    Set<Error> errors = Sets.difference(info.getErrors(), ImmutableSet.of(Error.HYPHEN_3_4));
+    checkArgument(
+        errors.isEmpty(), "Errors ASCIIfying label %s: %s", name, Joiner.on(',').join(errors));
+    return result.toString();
+  }
+
   /**
    * Translates a string from ASCII Compatible Encoding (ACE) to Unicode, as defined by the
    * ToUnicode operation of <a href="http://www.ietf.org/rfc/rfc3490.txt">RFC 3490</a>.

util/src/test/java/google/registry/util/DomainNameUtilsTest.java

@@ -45,13 +45,37 @@ class DomainNameUtilsTest {
         .isEqualTo("119.63.227.45-ns1.jhz-tt.uk");
   }
 
+  @Test
+  void testCanonicalizeHostname_retainsTrailingDot() {
+    assertThat(canonicalizeHostname("みんな.みんな.")).isEqualTo("xn--q9jyb4c.xn--q9jyb4c.");
+    assertThat(canonicalizeHostname("BAR.foo.みんな.")).isEqualTo("bar.foo.xn--q9jyb4c.");
+    assertThat(canonicalizeHostname("cat.lol.")).isEqualTo("cat.lol.");
+  }
+
   @Test
   void testCanonicalizeHostname_throwsOn34HyphenRule() {
     IllegalArgumentException thrown =
         assertThrows(
             IllegalArgumentException.class,
             () -> canonicalizeHostname("119.63.227.45--ns1.jhz-tt.uk"));
-    assertThat(thrown).hasCauseThat().hasMessageThat().contains("HYPHEN_3_4");
+    assertThat(thrown).hasMessageThat().contains("HYPHEN_3_4");
+  }
+
+  @Test
+  void testCanonicalizeHostname_throwsOn34HyphenRule_withTrailingDot() {
+    IllegalArgumentException thrown =
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> canonicalizeHostname("119.63.227.45--ns1.jhz-tt.uk."));
+    assertThat(thrown).hasMessageThat().contains("HYPHEN_3_4");
+  }
+
+  @Test
+  void testCanonicalizeHostname_allows34HyphenOnTld() {
+    assertThat(canonicalizeHostname("foobar.zz--main-2262")).isEqualTo("foobar.zz--main-2262");
+    assertThat(canonicalizeHostname("foobar.zz--main-2262.")).isEqualTo("foobar.zz--main-2262.");
+    assertThat(canonicalizeHostname("みんな.45--foo")).isEqualTo("xn--q9jyb4c.45--foo");
+    assertThat(canonicalizeHostname("みんな.45--foo.")).isEqualTo("xn--q9jyb4c.45--foo.");
   }
 
   @Test