Enforce canonicalization of premium/reserved list labels

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=193401336

java/google/registry/model/registry/label/DomainLabelEntry.java

@@ -14,7 +14,9 @@
 
 package google.registry.model.registry.label;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Strings.emptyToNull;
+import static google.registry.util.DomainNameUtils.canonicalizeDomainName;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 
 import com.googlecode.objectify.annotation.Id;
@@ -76,6 +78,10 @@ public abstract class DomainLabelEntry<T extends Comparable<?>, D extends Domain
     @Override
     public T build() {
       checkArgumentNotNull(emptyToNull(getInstance().label), "Label must be specified");
+      checkArgument(
+          getInstance().label.equals(canonicalizeDomainName(getInstance().label)),
+          "Label '%s' must be in puny-coded, lower-case form",
+          getInstance().label);
       checkArgumentNotNull(getInstance().getValue(), "Value must be specified");
       return super.build();
     }

java/google/registry/model/registry/label/PremiumList.java

@@ -248,6 +248,7 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
 
     /** A builder for constructing {@link PremiumListEntry} objects, since they are immutable. */
     public static class Builder extends DomainLabelEntry.Builder<PremiumListEntry, Builder> {
+
       public Builder() {}
 
       private Builder(PremiumListEntry instance) {

java/google/registry/model/registry/label/ReservedList.java

@@ -44,6 +44,7 @@ import com.googlecode.objectify.annotation.Embed;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Mapify;
 import com.googlecode.objectify.mapper.Mapper;
+import google.registry.model.Buildable;
 import google.registry.model.registry.Registry;
 import google.registry.model.registry.label.DomainLabelMetrics.MetricsReservedListMatch;
 import java.util.List;
@@ -72,7 +73,7 @@ public final class ReservedList
    */
   @Embed
   public static class ReservedListEntry
-      extends DomainLabelEntry<ReservationType, ReservedListEntry> {
+      extends DomainLabelEntry<ReservationType, ReservedListEntry> implements Buildable {
 
     ReservationType reservationType;
 
@@ -114,8 +115,12 @@ public final class ReservedList
         String label,
         ReservationType reservationType,
         @Nullable String restrictions,
-        String comment) {
+        @Nullable String comment) {
-      ReservedListEntry entry = new ReservedListEntry();
+      ReservedListEntry.Builder builder =
+          new ReservedListEntry.Builder()
+              .setLabel(label)
+              .setComment(comment)
+              .setReservationType(reservationType);
       if (restrictions != null) {
         checkArgument(
             reservationType == RESERVED_FOR_ANCHOR_TENANT
@@ -123,12 +128,10 @@ public final class ReservedList
             "Only anchor tenant and nameserver restricted reservations "
                 + "should have restrictions imposed");
         if (reservationType == RESERVED_FOR_ANCHOR_TENANT) {
-          entry.authCode = restrictions;
+          builder.setAuthCode(restrictions);
         } else if (reservationType == NAMESERVER_RESTRICTED) {
-          Set<String> allowedNameservers =
+          builder.setAllowedNameservers(
-              ImmutableSet.copyOf(Splitter.on(':').trimResults().split(restrictions));
+              ImmutableSet.copyOf(Splitter.on(':').trimResults().split(restrictions)));
-          checkNameserversAreValid(allowedNameservers);
-          entry.allowedNameservers = Joiner.on(',').join(allowedNameservers);
         }
       } else {
         checkArgument(
@@ -138,10 +141,7 @@ public final class ReservedList
             reservationType != NAMESERVER_RESTRICTED,
             "Nameserver restricted reservations must have at least one nameserver configured");
       }
-      entry.label = label;
+      return builder.build();
-      entry.comment = comment;
-      entry.reservationType = reservationType;
-      return entry;
     }
 
     private static void checkNameserversAreValid(Set<String> nameservers) {
@@ -166,6 +166,38 @@ public final class ReservedList
     public ImmutableSet<String> getAllowedNameservers() {
       return ImmutableSet.copyOf(Splitter.on(',').splitToList(allowedNameservers));
     }
+
+    @Override
+    public ReservedListEntry.Builder asBuilder() {
+      return new ReservedListEntry.Builder(clone(this));
+    }
+
+    /** A builder for constructing {@link ReservedListEntry} objects, since they are immutable. */
+    private static class Builder
+        extends DomainLabelEntry.Builder<ReservedListEntry, ReservedListEntry.Builder> {
+
+      public Builder() {}
+
+      private Builder(ReservedListEntry instance) {
+        super(instance);
+      }
+
+      ReservedListEntry.Builder setAllowedNameservers(Set<String> allowedNameservers) {
+        checkNameserversAreValid(allowedNameservers);
+        getInstance().allowedNameservers = Joiner.on(',').join(allowedNameservers);
+        return this;
+      }
+
+      ReservedListEntry.Builder setAuthCode(String authCode) {
+        getInstance().authCode = authCode;
+        return this;
+      }
+
+      ReservedListEntry.Builder setReservationType(ReservationType reservationType) {
+        getInstance().reservationType = reservationType;
+        return this;
+      }
+    }
   }
 
   @Override

java/google/registry/util/DomainNameUtils.java

@@ -41,7 +41,12 @@ public final class DomainNameUtils {
 
   /** Canonicalizes a domain name by lowercasing and converting unicode to punycode. */
   public static String canonicalizeDomainName(String label) {
-    return Idn.toASCII(Ascii.toLowerCase(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);
+    }
   }
 
   /**

javatests/google/registry/flows/domain/DomainCreateFlowTest.java

@@ -1669,7 +1669,8 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow,
 
   @Test
   public void testFailure_invalidPunycode() throws Exception {
-    doFailingDomainNameTest("xn--abcdefg.tld", InvalidPunycodeException.class);
+    // You don't want to know what this string (might?) mean.
+    doFailingDomainNameTest("xn--uxa129t5ap4f1h1bc3p.tld", InvalidPunycodeException.class);
   }
 
   @Test

javatests/google/registry/model/registry/label/PremiumListTest.java

@@ -25,8 +25,10 @@ import static google.registry.testing.JUnitBackports.assertThrows;
 
 import com.google.common.collect.ImmutableList;
 import google.registry.model.registry.Registry;
+import google.registry.model.registry.label.PremiumList.PremiumListEntry;
 import google.registry.model.registry.label.PremiumList.PremiumListRevision;
 import google.registry.testing.AppEngineRule;
+import org.joda.money.Money;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -98,4 +100,30 @@ public class PremiumListTest {
         .contains(
             "List 'tld' cannot contain duplicate labels. Dupes (with counts) were: [lol x 2]");
   }
+
+  @Test
+  public void testValidation_labelMustBeLowercase() {
+    Exception e =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                new PremiumListEntry.Builder()
+                    .setPrice(Money.parse("USD 399"))
+                    .setLabel("UPPER.tld")
+                    .build());
+    assertThat(e).hasMessageThat().contains("must be in puny-coded, lower-case form");
+  }
+
+  @Test
+  public void testValidation_labelMustBePunyCoded() {
+    Exception e =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                new PremiumListEntry.Builder()
+                    .setPrice(Money.parse("USD 399"))
+                    .setLabel("lower.みんな")
+                    .build());
+    assertThat(e).hasMessageThat().contains("must be in puny-coded, lower-case form");
+  }
 }

javatests/google/registry/model/registry/label/ReservedListTest.java

@@ -410,7 +410,7 @@ public class ReservedListTest {
         "reserved",
         "trombone,FULLY_BLOCKED  # yup",
         "oysters,MISTAKEN_PREMIUM #  this is a loooong comment",
-        "nullComment,ALLOWED_IN_SUNRISE  #");
+        "nullcomment,ALLOWED_IN_SUNRISE  #");
     assertThat(reservedList.getReservedListEntries()).hasSize(3);
 
     ReservedListEntry trombone = reservedList.getReservedListEntries().get("trombone");
@@ -423,8 +423,8 @@ public class ReservedListTest {
     assertThat(oysters.reservationType).isEqualTo(ReservationType.MISTAKEN_PREMIUM);
     assertThat(oysters.comment).isEqualTo("this is a loooong comment");
 
-    ReservedListEntry nullComment = reservedList.getReservedListEntries().get("nullComment");
+    ReservedListEntry nullComment = reservedList.getReservedListEntries().get("nullcomment");
-    assertThat(nullComment.label).isEqualTo("nullComment");
+    assertThat(nullComment.label).isEqualTo("nullcomment");
     assertThat(nullComment.reservationType).isEqualTo(ReservationType.ALLOWED_IN_SUNRISE);
     assertThat(nullComment.comment).isEmpty();
   }
@@ -582,4 +582,22 @@ public class ReservedListTest {
         .contains(
             "List 'blah' cannot contain duplicate labels. Dupes (with counts) were: [lol x 2]");
   }
+
+  @Test
+  public void testValidation_labelMustBeLowercase() {
+    Exception e =
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> ReservedListEntry.create("UPPER.tld", FULLY_BLOCKED, null, null));
+    assertThat(e).hasMessageThat().contains("must be in puny-coded, lower-case form");
+  }
+
+  @Test
+  public void testValidation_labelMustBePunyCoded() {
+    Exception e =
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> ReservedListEntry.create("lower.みんな", FULLY_BLOCKED, null, null));
+    assertThat(e).hasMessageThat().contains("must be in puny-coded, lower-case form");
+  }
 }