From 7951799595c1d3ebd96e67267487c3a392b49797 Mon Sep 17 00:00:00 2001 From: mcilwain Date: Wed, 25 Oct 2017 08:19:47 -0700 Subject: [PATCH] Add validation to ROID suffixes I could've sworn we were already doing this, but apparently not? Anyway, ROID suffixes have a number of requirements on them that weren't being enforced, so this enforces them. All existing production data is compliant with these requirements; the only existing bad data we have is in alpha and sandbox. ROID suffixes are now required to match the regex ^[A-Z0-9_]{1,8}$ See also https://tools.ietf.org/html/rfc5730 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=173400001 --- .../registry/model/registry/Registry.java | 7 +++++ .../registry/model/registry/RegistryTest.java | 26 ++++++++++++++++++- .../registry/testing/DatastoreHelper.java | 9 ++++--- .../registry/tools/CreateTldCommandTest.java | 5 ++-- 4 files changed, 39 insertions(+), 8 deletions(-) diff --git a/java/google/registry/model/registry/Registry.java b/java/google/registry/model/registry/Registry.java index 0ae1783c7..fcd7175f8 100644 --- a/java/google/registry/model/registry/Registry.java +++ b/java/google/registry/model/registry/Registry.java @@ -68,6 +68,7 @@ import java.util.Collection; import java.util.Map.Entry; import java.util.Optional; import java.util.Set; +import java.util.regex.Pattern; import javax.annotation.Nullable; import org.joda.money.CurrencyUnit; import org.joda.money.Money; @@ -852,7 +853,13 @@ public class Registry extends ImmutableObject implements Buildable { return this; } + private static final Pattern ROID_SUFFIX_PATTERN = Pattern.compile("^[A-Z0-9_]{1,8}$"); + public Builder setRoidSuffix(String roidSuffix) { + checkArgument( + ROID_SUFFIX_PATTERN.matcher(roidSuffix).matches(), + "ROID suffix must be in format %s", + ROID_SUFFIX_PATTERN.pattern()); getInstance().roidSuffix = roidSuffix; return this; } diff --git a/javatests/google/registry/model/registry/RegistryTest.java b/javatests/google/registry/model/registry/RegistryTest.java index 3cc9e42c0..6208cfde9 100644 --- a/javatests/google/registry/model/registry/RegistryTest.java +++ b/javatests/google/registry/model/registry/RegistryTest.java @@ -17,7 +17,6 @@ package google.registry.model.registry; import static com.google.common.collect.Iterables.transform; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; -import static com.google.common.truth.Truth8.assertThat; import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.registry.label.ReservedListTest.GET_NAME_FUNCTION; @@ -29,6 +28,8 @@ import static google.registry.util.DateTimeUtils.END_OF_TIME; import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.joda.money.CurrencyUnit.EUR; import static org.joda.money.CurrencyUnit.USD; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.expectThrows; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; @@ -488,4 +489,27 @@ public class RegistryTest extends EntityTestCase { .setEapFeeSchedule(ImmutableSortedMap.of(START_OF_TIME, Money.zero(EUR))) .build(); } + + @Test + public void testFailure_roidSuffixTooLong() { + IllegalArgumentException e = + expectThrows( + IllegalArgumentException.class, + () -> Registry.get("tld").asBuilder().setRoidSuffix("123456789")); + assertThat(e).hasMessageThat().isEqualTo("ROID suffix must be in format ^[A-Z0-9_]{1,8}$"); + } + + @Test + public void testFailure_roidSuffixNotUppercased() { + assertThrows( + IllegalArgumentException.class, + () -> Registry.get("tld").asBuilder().setRoidSuffix("abcd")); + } + + @Test + public void testFailure_roidSuffixContainsInvalidCharacters() { + assertThrows( + IllegalArgumentException.class, + () -> Registry.get("tld").asBuilder().setRoidSuffix("ABC-DEF")); + } } diff --git a/javatests/google/registry/testing/DatastoreHelper.java b/javatests/google/registry/testing/DatastoreHelper.java index 06a2dcc0c..304bed87f 100644 --- a/javatests/google/registry/testing/DatastoreHelper.java +++ b/javatests/google/registry/testing/DatastoreHelper.java @@ -418,10 +418,11 @@ public class DatastoreHelper { } public static void createTld(String tld, ImmutableSortedMap tldStates) { - createTld( - tld, - Ascii.toUpperCase(tld.replaceFirst(ACE_PREFIX_REGEX, "").replace('.', '_')), - tldStates); + // Coerce the TLD string into a valid ROID suffix. + String roidSuffix = + Ascii.toUpperCase(tld.replaceFirst(ACE_PREFIX_REGEX, "").replace('.', '_')) + .replace('-', '_'); + createTld(tld, roidSuffix.length() > 8 ? roidSuffix.substring(0, 8) : roidSuffix, tldStates); } public static void createTld( diff --git a/javatests/google/registry/tools/CreateTldCommandTest.java b/javatests/google/registry/tools/CreateTldCommandTest.java index c31ccc806..60f8c1c76 100644 --- a/javatests/google/registry/tools/CreateTldCommandTest.java +++ b/javatests/google/registry/tools/CreateTldCommandTest.java @@ -16,7 +16,6 @@ package google.registry.tools; import static com.google.common.collect.Iterables.transform; import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth8.assertThat; import static google.registry.model.registry.label.ReservedListTest.GET_NAME_FUNCTION; import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.persistPremiumList; @@ -78,13 +77,13 @@ public class CreateTldCommandTest extends CommandTestCase { @Test public void testFailure_multipleArguments() throws Exception { thrown.expect(IllegalArgumentException.class, "Can't create more than one TLD at a time"); - runCommandForced("--roid_suffix=blah", "--dns_writers=VoidDnsWriter", "xn--q9jyb4c", "test"); + runCommandForced("--roid_suffix=BLAH", "--dns_writers=VoidDnsWriter", "xn--q9jyb4c", "test"); } @Test public void testFailure_multipleDuplicateArguments() throws Exception { thrown.expect(IllegalArgumentException.class, "Can't create more than one TLD at a time"); - runCommandForced("--roid_suffix=blah", "--dns_writers=VoidDnsWriter", "test", "test"); + runCommandForced("--roid_suffix=BLAH", "--dns_writers=VoidDnsWriter", "test", "test"); } @Test