mirrors/nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus synced 2026-07-12 02:52:38 +00:00
Code Issues Releases Wiki Activity

Enforce nonnegative costs for TLDs (#3139)

Browse Source 
Note: we remove the checks in the setters because we check them all in
the build method instead (and the setters are only called in tests).
This commit is contained in:
gbrodman
2026-07-10 12:49:17 -04:00
committed by GitHub
parent 5067b3d339
commit b8a51cacc4
6 changed files with 149 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/main/java/google/registry/model/tld/Tld.java
+52 -67
View File
@@ -814,6 +814,55 @@ public class Tld extends ImmutableObject implements Buildable, UnsafeSerializabl
return new Builder(clone(this));
}
/** Checks the validity of the TLD object, for use during building or deserializing. */
public void validateState() {
checkArgument(tldStr != null, "No registry TLD specified");
// Check for canonical form by converting to an InternetDomainName and then back.
checkArgument(
InternetDomainName.isValid(tldStr)
&& tldStr.equals(InternetDomainName.from(tldStr).toString()),
"Cannot create registry for TLD that is not a valid, canonical domain name");
// Check the validity of all TimedTransitionProperties to ensure that they have values for
// START_INSTANT. The setters above have already checked this for new values, but also check
// here to catch cases where we loaded an invalid TimedTransitionProperty from the database
// and cloned it into a new builder, to block re-building a Tld in an invalid state.
tldStateTransitions.checkValidity();
createBillingCostTransitions.checkValidity();
renewBillingCostTransitions.checkValidity();
eapFeeSchedule.checkValidity();
// All costs must be in the expected currency.
checkArgumentNotNull(getCurrency(), "Currency must be set");
Predicate<Money> currencyCheck =
(Money money) -> money.getCurrencyUnit().equals(currency) && money.isPositiveOrZero();
checkArgument(
currencyCheck.test(getRestoreBillingCost()),
"Restore cost is negative or in the wrong currency");
checkArgument(
currencyCheck.test(getServerStatusChangeBillingCost()),
"Server status change cost is negative or in the wrong currency");
checkArgument(
currencyCheck.test(getRegistryLockOrUnlockBillingCost()),
"Registry lock/unlock cost is negative or in the wrong currency");
checkArgument(
getRenewBillingCostTransitions().values().stream().allMatch(currencyCheck),
"Some renew cost(s) are negative or in the wrong currency");
checkArgument(
getCreateBillingCostTransitions().values().stream().allMatch(currencyCheck),
"Some create cost(s) are negative or in the wrong currency");
checkArgument(
eapFeeSchedule.toValueMap().values().stream().allMatch(currencyCheck),
"Some EAP fee cost(s) are negative or in the wrong currency'");
checkArgumentNotNull(
pricingEngineClassName, "All registries must have a configured pricing engine");
checkArgument(
dnsWriters != null && !dnsWriters.isEmpty(),
"At least one DNS writer must be specified."
+ " VoidDnsWriter can be used if DNS writing isn't desired");
checkArgument(
numDnsPublishLocks > 0,
"Number of DNS publish locks must be positive. Use 1 for TLD-wide locks.");
}
/** A builder for constructing {@link Tld} objects, since they are immutable. */
public static class Builder extends Buildable.Builder<Tld> {
public Builder() {}
@@ -966,10 +1015,6 @@ public class Tld extends ImmutableObject implements Buildable, UnsafeSerializabl
public Builder setCreateBillingCostTransitions(
ImmutableSortedMap<Instant, Money> createCostsMap) {
checkArgumentNotNull(createCostsMap, "Create billing costs map cannot be null");
checkArgument(
createCostsMap.values().stream().allMatch(Money::isPositiveOrZero),
"Create billing cost cannot be negative");
getInstance().createBillingCostTransitions =
TimedTransitionProperty.fromValueMap(createCostsMap);
return this;
@@ -1012,17 +1057,12 @@ public class Tld extends ImmutableObject implements Buildable, UnsafeSerializabl
}
public Builder setRestoreBillingCost(Money amount) {
checkArgument(amount.isPositiveOrZero(), "restoreBillingCost cannot be negative");
getInstance().restoreBillingCost = amount;
return this;
}
public Builder setRenewBillingCostTransitions(
ImmutableSortedMap<Instant, Money> renewCostsMap) {
checkArgumentNotNull(renewCostsMap, "Renew billing costs map cannot be null");
checkArgument(
renewCostsMap.values().stream().allMatch(Money::isPositiveOrZero),
"Renew billing cost cannot be negative");
getInstance().renewBillingCostTransitions =
TimedTransitionProperty.fromValueMap(renewCostsMap);
return this;
@@ -1030,10 +1070,6 @@ public class Tld extends ImmutableObject implements Buildable, UnsafeSerializabl
/** Sets the EAP fee schedule for the TLD. */
public Builder setEapFeeSchedule(ImmutableSortedMap<Instant, Money> eapFeeSchedule) {
checkArgumentNotNull(eapFeeSchedule, "EAP fee schedule cannot be null");
checkArgument(
eapFeeSchedule.values().stream().allMatch(Money::isPositiveOrZero),
"EAP fee cannot be negative");
getInstance().eapFeeSchedule = TimedTransitionProperty.fromValueMap(eapFeeSchedule);
return this;
}
@@ -1051,14 +1087,11 @@ public class Tld extends ImmutableObject implements Buildable, UnsafeSerializabl
}
public Builder setServerStatusChangeBillingCost(Money amount) {
checkArgument(
amount.isPositiveOrZero(), "Server status change billing cost cannot be negative");
getInstance().serverStatusChangeBillingCost = amount;
return this;
}
public Builder setRegistryLockOrUnlockBillingCost(Money amount) {
checkArgument(amount.isPositiveOrZero(), "Registry lock/unlock cost cannot be negative");
getInstance().registryLockOrUnlockBillingCost = amount;
return this;
}
@@ -1120,58 +1153,10 @@ public class Tld extends ImmutableObject implements Buildable, UnsafeSerializabl
@Override
public Tld build() {
final Tld instance = getInstance();
// Pick up the name of the associated TLD from the instance object.
String tldName = instance.tldStr;
checkArgument(tldName != null, "No registry TLD specified");
// Check for canonical form by converting to an InternetDomainName and then back.
checkArgument(
InternetDomainName.isValid(tldName)
&& tldName.equals(InternetDomainName.from(tldName).toString()),
"Cannot create registry for TLD that is not a valid, canonical domain name");
// Check the validity of all TimedTransitionProperties to ensure that they have values for
// START_INSTANT. The setters above have already checked this for new values, but also check
// here to catch cases where we loaded an invalid TimedTransitionProperty from the database
// and cloned it into a new builder, to block re-building a Tld in an invalid state.
instance.tldStateTransitions.checkValidity();
instance.createBillingCostTransitions.checkValidity();
instance.renewBillingCostTransitions.checkValidity();
instance.eapFeeSchedule.checkValidity();
// All costs must be in the expected currency.
checkArgumentNotNull(instance.getCurrency(), "Currency must be set");
checkArgument(
instance.getRestoreBillingCost().getCurrencyUnit().equals(instance.currency),
"Restore cost must be in the TLD's currency");
checkArgument(
instance.getServerStatusChangeBillingCost().getCurrencyUnit().equals(instance.currency),
"Server status change cost must be in the TLD's currency");
checkArgument(
instance.getRegistryLockOrUnlockBillingCost().getCurrencyUnit().equals(instance.currency),
"Registry lock/unlock cost must be in the TLD's currency");
Predicate<Money> currencyCheck =
(Money money) -> money.getCurrencyUnit().equals(instance.currency);
checkArgument(
instance.getRenewBillingCostTransitions().values().stream().allMatch(currencyCheck),
"Renew cost must be in the TLD's currency");
checkArgument(
instance.getCreateBillingCostTransitions().values().stream().allMatch(currencyCheck),
"Create cost must be in the TLD's currency");
checkArgument(
instance.eapFeeSchedule.toValueMap().values().stream().allMatch(currencyCheck),
"All EAP fees must be in the TLD's currency");
checkArgumentNotNull(
instance.pricingEngineClassName, "All registries must have a configured pricing engine");
checkArgument(
instance.dnsWriters != null && !instance.dnsWriters.isEmpty(),
"At least one DNS writer must be specified."
+ " VoidDnsWriter can be used if DNS writing isn't desired");
// If not set explicitly, numDnsPublishLocks defaults to 1.
instance.setDefaultNumDnsPublishLocks();
checkArgument(
instance.numDnsPublishLocks > 0,
"Number of DNS publish locks must be positive. Use 1 for TLD-wide locks.");
instance.tldStr = tldName;
instance.tldUnicode = Idn.toUnicode(tldName);
getInstance().setDefaultNumDnsPublishLocks();
getInstance().validateState();
getInstance().tldUnicode = Idn.toUnicode(getInstance().tldStr);
return super.build();
}
}
core/src/main/java/google/registry/tools/ConfigureTldCommand.java
+2
View File
@@ -164,6 +164,8 @@ public class ConfigureTldCommand extends MutatingCommand {
if (Boolean.TRUE.equals(breakGlass)) {
newTld = newTld.asBuilder().setBreakglassMode(true).build();
}
// Enforce any restrictions, e.g. "no negative fees"
newTld.validateState();
stageEntityChange(oldTld, newTld);
}
core/src/test/java/google/registry/model/tld/TldTest.java
+26 -12
View File
@@ -257,7 +257,8 @@ public final class TldTest extends EntityTestCase {
.asBuilder()
.setCreateBillingCostTransitions(createCostTransitions)
.build());
assertThat(thrown.getMessage()).isEqualTo("Create billing cost cannot be negative");
assertThat(thrown.getMessage())
.isEqualTo("Some create cost(s) are negative or in the wrong currency");
}
@Test
@@ -614,8 +615,11 @@ public final class TldTest extends EntityTestCase {
Tld.get("tld")
.asBuilder()
.setRenewBillingCostTransitions(
ImmutableSortedMap.of(START_INSTANT, Money.of(USD, -42))));
assertThat(thrown).hasMessageThat().contains("billing cost cannot be negative");
ImmutableSortedMap.of(START_INSTANT, Money.of(USD, -42)))
.build());
assertThat(thrown)
.hasMessageThat()
.isEqualTo("Some renew cost(s) are negative or in the wrong currency");
}
@Test
@@ -623,8 +627,10 @@ public final class TldTest extends EntityTestCase {
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
() -> Tld.get("tld").asBuilder().setRestoreBillingCost(Money.of(USD, -42)));
assertThat(thrown).hasMessageThat().contains("restoreBillingCost cannot be negative");
() -> Tld.get("tld").asBuilder().setRestoreBillingCost(Money.of(USD, -42)).build());
assertThat(thrown)
.hasMessageThat()
.isEqualTo("Restore cost is negative or in the wrong currency");
}
@Test
@@ -652,8 +658,14 @@ public final class TldTest extends EntityTestCase {
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
() -> Tld.get("tld").asBuilder().setServerStatusChangeBillingCost(Money.of(USD, -42)));
assertThat(thrown).hasMessageThat().contains("billing cost cannot be negative");
() ->
Tld.get("tld")
.asBuilder()
.setServerStatusChangeBillingCost(Money.of(USD, -42))
.build());
assertThat(thrown)
.hasMessageThat()
.isEqualTo("Server status change cost is negative or in the wrong currency");
}
@Test
@@ -667,7 +679,9 @@ public final class TldTest extends EntityTestCase {
.setRenewBillingCostTransitions(
ImmutableSortedMap.of(START_INSTANT, Money.of(EUR, 42)))
.build());
assertThat(thrown).hasMessageThat().contains("cost must be in the TLD's currency");
assertThat(thrown)
.hasMessageThat()
.isEqualTo("Some renew cost(s) are negative or in the wrong currency");
}
@Test
@@ -681,7 +695,7 @@ public final class TldTest extends EntityTestCase {
.setCreateBillingCostTransitions(
ImmutableSortedMap.of(START_INSTANT, Money.of(EUR, 42)))
.build());
assertThat(thrown).hasMessageThat().contains("cost must be in the TLD's currency");
assertThat(thrown).hasMessageThat().contains("negative or in the wrong currency");
}
@Test
@@ -690,7 +704,7 @@ public final class TldTest extends EntityTestCase {
assertThrows(
IllegalArgumentException.class,
() -> Tld.get("tld").asBuilder().setRestoreBillingCost(Money.of(EUR, 42)).build());
assertThat(thrown).hasMessageThat().contains("cost must be in the TLD's currency");
assertThat(thrown).hasMessageThat().contains("negative or in the wrong currency");
}
@Test
@@ -703,7 +717,7 @@ public final class TldTest extends EntityTestCase {
.asBuilder()
.setServerStatusChangeBillingCost(Money.of(EUR, 42))
.build());
assertThat(thrown).hasMessageThat().contains("cost must be in the TLD's currency");
assertThat(thrown).hasMessageThat().contains("negative or in the wrong currency");
}
@Test
@@ -742,7 +756,7 @@ public final class TldTest extends EntityTestCase {
.asBuilder()
.setEapFeeSchedule(ImmutableSortedMap.of(START_INSTANT, Money.zero(EUR)))
.build());
assertThat(thrown).hasMessageThat().contains("All EAP fees must be in the TLD's currency");
assertThat(thrown).hasMessageThat().contains("negative or in the wrong currency");
}
@Test
core/src/test/java/google/registry/tools/ConfigureTldCommandTest.java
+10
View File
@@ -454,6 +454,16 @@ public class ConfigureTldCommandTest extends CommandTestCase<ConfigureTldCommand
+ " unit USD. Found [EUR] currency unit(s) in the renewBillingCostTransitionsMap");
}
@Test
void testFailure_negativeCreateCost() throws Exception {
File tldFile = tmpDir.resolve("negativecost.yaml").toFile();
Files.asCharSink(tldFile, UTF_8).write(loadFile(getClass(), "negativecost.yaml"));
IllegalArgumentException thrown =
assertThrows(IllegalArgumentException.class, () -> runCommandForced("--input=" + tldFile));
assertThat(thrown.getMessage())
.isEqualTo("Some create cost(s) are negative or in the wrong currency");
}
@Test
void testSuccess_emptyStringClearsDefaultPromoTokens() throws Exception {
Tld tld = createTld("tld");
core/src/test/resources/google/registry/tools/negativecost.yaml
+58
View File
@@ -0,0 +1,58 @@
addGracePeriodLength: "PT120H"
allowedFullyQualifiedHostNames: []
anchorTenantAddGracePeriodLength: "PT720H"
autoRenewGracePeriodLength: "PT1080H"
automaticTransferLength: "PT120H"
claimsPeriodEnd: "294247-01-10T04:00:54.775Z"
createBillingCostTransitions:
"1970-01-01T00:00:00.000Z":
currency: "USD"
amount: 8.00
"2020-01-01T00:00:00.000Z":
currency: "USD"
amount: -25.00
creationTime: "2022-09-01T00:00:00.000Z"
currency: "USD"
defaultPromoTokens: []
dnsAPlusAaaaTtl: "PT15M"
dnsDsTtl: null
dnsNsTtl: null
dnsPaused: false
dnsWriters:
- "VoidDnsWriter"
driveFolderId: "driveFolder"
eapFeeSchedule:
"1970-01-01T00:00:00.000Z":
currency: "USD"
amount: 0.00
escrowEnabled: false
idnTables: []
invoicingEnabled: false
lordnUsername: null
numDnsPublishLocks: 1
pendingDeleteLength: "PT120H"
premiumListName: "test"
pricingEngineClassName: "google.registry.model.pricing.StaticPremiumListPricingEngine"
redemptionGracePeriodLength: "PT720H"
registryLockOrUnlockBillingCost:
currency: "USD"
amount: 0.00
renewBillingCostTransitions:
"1970-01-01T00:00:00.000Z":
currency: "USD"
amount: 11.00
renewGracePeriodLength: "PT120H"
reservedListNames: []
restoreBillingCost:
currency: "USD"
amount: 17.00
roidSuffix: "TLD"
serverStatusChangeBillingCost:
currency: "USD"
amount: 19.00
tldStateTransitions:
"1970-01-01T00:00:00.000Z": "GENERAL_AVAILABILITY"
tldStr: "negativecost"
tldType: "REAL"
tldUnicode: "negativecost"
transferGracePeriodLength: "PT120H"
core/src/test/resources/google/registry/tools/nullablefieldsallnull.yaml
+1
View File
@@ -16,6 +16,7 @@ tldStateTransitions:
creationTime: "2022-09-01T00:00:00.000Z"
reservedListNames: null
premiumListName: null
pricingEngineClassName: "google.registry.model.pricing.StaticPremiumListPricingEngine"
escrowEnabled: false
dnsPaused: false
addGracePeriodLength: "PT720H"