Add verification that domain labels aren't multi-level domains (#553)

* Add verification that domain labels aren't multi-level domains

In addition, I did a bit of test refactoring because previously, the
CreateOrUpdateReserveListCommandTestCase test cases weren't actually
testing the proper things -- they were failing with
IllegalArgumentExceptions, but not the right ones.

* Change test name and use IDN library

* Handle numeric labels

String like "0" or "2018" are valid labels but not valid domain names

* Use IDN validation with a dummy TLD
This commit is contained in:
gbrodman 2020-04-15 11:54:40 -04:00 committed by GitHub
parent 580a3b6981
commit 3d88ba4e1b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 47 additions and 12 deletions

View file

@ -19,6 +19,7 @@ import static com.google.common.base.Strings.emptyToNull;
import static google.registry.util.DomainNameUtils.canonicalizeDomainName;
import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
import com.google.common.net.InternetDomainName;
import com.googlecode.objectify.annotation.Id;
import google.registry.model.Buildable.GenericBuilder;
import google.registry.model.ImmutableObject;
@ -83,6 +84,13 @@ public abstract class DomainLabelEntry<T extends Comparable<?>, D extends Domain
"Label '%s' must be in puny-coded, lower-case form",
getInstance().label);
checkArgumentNotNull(getInstance().getValue(), "Value must be specified");
// Verify that the label creates a valid SLD if we add a TLD to the end of it.
// We require that the label is not already a full domain name including a dot.
// Domain name validation is tricky, so let InternetDomainName handle it for us.
checkArgument(
InternetDomainName.from(getInstance().label + ".tld").parts().size() == 2,
"Label %s must not be a multi-level domain name",
getInstance().label);
return super.build();
}
}

View file

@ -61,18 +61,43 @@ public abstract class CreateOrUpdateReservedListCommandTestCase<
@Test
public void testFailure_fileDoesntExist() {
assertThat(
assertThrows(
ParameterException.class,
() ->
runCommandForced(
"--name=xn--q9jyb4c-blah", "--input=" + reservedTermsPath + "-nonexistent"));
"--name=xn--q9jyb4c_common-reserved",
"--input=" + reservedTermsPath + "-nonexistent")))
.hasMessageThat()
.contains("-i not found");
}
@Test
public void testFailure_fileDoesntParse() {
assertThat(
assertThrows(
IllegalArgumentException.class,
() -> runCommandForced("--name=xn--q9jyb4c-blork", "--input=" + invalidReservedTermsPath));
() ->
runCommandForced(
"--name=xn--q9jyb4c_common-reserved",
"--input=" + invalidReservedTermsPath)))
.hasMessageThat()
.contains("No enum constant");
}
@Test
public void testFailure_invalidLabel_includesFullDomainName() throws Exception {
Files.asCharSink(new File(invalidReservedTermsPath), UTF_8)
.write("example.tld,FULLY_BLOCKED\n\n");
assertThat(
assertThrows(
IllegalArgumentException.class,
() ->
runCommandForced(
"--name=xn--q9jyb4c_common-reserved",
"--input=" + invalidReservedTermsPath)))
.hasMessageThat()
.isEqualTo("Label example.tld must not be a multi-level domain name");
}
google.registry.schema.tld.ReservedList createCloudSqlReservedList(

View file

@ -26,12 +26,18 @@ import com.google.common.collect.ImmutableMap;
import google.registry.model.registry.label.ReservedList;
import google.registry.schema.tld.ReservedList.ReservedEntry;
import google.registry.schema.tld.ReservedListDao;
import org.junit.Before;
import org.junit.Test;
/** Unit tests for {@link UpdateReservedListCommand}. */
public class UpdateReservedListCommandTest extends
CreateOrUpdateReservedListCommandTestCase<UpdateReservedListCommand> {
@Before
public void setup() {
populateInitialReservedListInDatastore(true);
}
private void populateInitialReservedListInDatastore(boolean shouldPublish) {
persistResource(
new ReservedList.Builder()
@ -63,7 +69,6 @@ public class UpdateReservedListCommandTest extends
@Test
public void testSuccess_lastUpdateTime_updatedCorrectly() throws Exception {
populateInitialReservedListInDatastore(true);
ReservedList original = ReservedList.get("xn--q9jyb4c_common-reserved").get();
runCommandForced("--input=" + reservedTermsPath);
ReservedList updated = ReservedList.get("xn--q9jyb4c_common-reserved").get();
@ -90,7 +95,6 @@ public class UpdateReservedListCommandTest extends
}
private void runSuccessfulUpdateTest(String... args) throws Exception {
populateInitialReservedListInDatastore(true);
runCommandForced(args);
assertThat(ReservedList.get("xn--q9jyb4c_common-reserved")).isPresent();
ReservedList reservedList = ReservedList.get("xn--q9jyb4c_common-reserved").get();
@ -114,7 +118,6 @@ public class UpdateReservedListCommandTest extends
@Test
public void testSaveToCloudSql_succeeds() throws Exception {
populateInitialReservedListInDatastore(true);
populateInitialReservedListInCloudSql(true);
runCommandForced("--name=xn--q9jyb4c_common-reserved", "--input=" + reservedTermsPath);
verifyXnq9jyb4cInDatastore();
@ -126,7 +129,6 @@ public class UpdateReservedListCommandTest extends
// Note that, during the dual-write phase, we always save the reserved list to Cloud SQL without
// checking if there is a list with same name. This is to backfill the existing list in Cloud
// Datastore when we update it.
populateInitialReservedListInDatastore(true);
runCommandForced("--name=xn--q9jyb4c_common-reserved", "--input=" + reservedTermsPath);
verifyXnq9jyb4cInDatastore();
assertThat(ReservedListDao.checkExists("xn--q9jyb4c_common-reserved")).isTrue();