diff --git a/java/google/registry/model/registry/label/DomainLabelEntry.java b/java/google/registry/model/registry/label/DomainLabelEntry.java index ae6f38d68..72e10d1a9 100644 --- a/java/google/registry/model/registry/label/DomainLabelEntry.java +++ b/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, 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(); } diff --git a/java/google/registry/model/registry/label/PremiumList.java b/java/google/registry/model/registry/label/PremiumList.java index 1002f0b70..40cda1ab8 100644 --- a/java/google/registry/model/registry/label/PremiumList.java +++ b/java/google/registry/model/registry/label/PremiumList.java @@ -248,6 +248,7 @@ public final class PremiumList extends BaseDomainLabelList { + public Builder() {} private Builder(PremiumListEntry instance) { diff --git a/java/google/registry/model/registry/label/ReservedList.java b/java/google/registry/model/registry/label/ReservedList.java index 6d2601c7d..c08b0aa96 100644 --- a/java/google/registry/model/registry/label/ReservedList.java +++ b/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 { + extends DomainLabelEntry implements Buildable { ReservationType reservationType; @@ -114,8 +115,12 @@ public final class ReservedList String label, ReservationType reservationType, @Nullable String restrictions, - String comment) { - ReservedListEntry entry = new ReservedListEntry(); + @Nullable String comment) { + 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 allowedNameservers = - ImmutableSet.copyOf(Splitter.on(':').trimResults().split(restrictions)); - checkNameserversAreValid(allowedNameservers); - entry.allowedNameservers = Joiner.on(',').join(allowedNameservers); + builder.setAllowedNameservers( + ImmutableSet.copyOf(Splitter.on(':').trimResults().split(restrictions))); } } 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; - entry.comment = comment; - entry.reservationType = reservationType; - return entry; + return builder.build(); } private static void checkNameserversAreValid(Set nameservers) { @@ -166,6 +166,38 @@ public final class ReservedList public ImmutableSet 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 { + + public Builder() {} + + private Builder(ReservedListEntry instance) { + super(instance); + } + + ReservedListEntry.Builder setAllowedNameservers(Set 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 diff --git a/java/google/registry/util/DomainNameUtils.java b/java/google/registry/util/DomainNameUtils.java index 554ab6676..6a380642e 100644 --- a/java/google/registry/util/DomainNameUtils.java +++ b/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); + } } /** diff --git a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java index 68c43a6f1..5d989acd0 100644 --- a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java @@ -1669,7 +1669,8 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase + 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"); + } } diff --git a/javatests/google/registry/model/registry/label/ReservedListTest.java b/javatests/google/registry/model/registry/label/ReservedListTest.java index e1b4857ed..5c0c37d68 100644 --- a/javatests/google/registry/model/registry/label/ReservedListTest.java +++ b/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"); - assertThat(nullComment.label).isEqualTo("nullComment"); + ReservedListEntry nullComment = reservedList.getReservedListEntries().get("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"); + } }