zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-05-13 16:07:15 +02:00
Code Issues Projects Releases Packages Wiki Activity

Correctly order RDAP domain searches by nameserver

Browse source 
Usually, the correct order happens automatically, because we are searching on either the key or a specific field like fullyQualifiedDomainName, and the results come back in that order. But when searching by nameserver, where we split the query into multiple "IN" chunks, we have to assemble the result set and order after the fact.

The tests didn't pick up the problem, because the domains and hosts were created in alphabetical order, so it happened to work anyway. The tests have now been changed to create things in reverse order, to test the reordering. Also, the previous arbitrary limit of 1000 nameservers in the intermediate query has been reduced to 300, because we now loop through all nameservers no matter what, rather than stopping when we collect enough domains, so there's more of a penalty for having way too many nameservers.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=173163121
This commit is contained in:
mountford 2017-10-23 13:36:00 -07:00 • committed by jianglai
parent 1790914058
commit 52fd9d8c4e
2 changed files with 145 additions and 151 deletions
Download patch file Download diff file Expand all files Collapse all files

49
java/google/registry/rdap/RdapDomainSearchAction.java
View file

@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.common.primitives.Booleans; import com.google.common.primitives.Booleans;
import com.googlecode.objectify.Key; import com.googlecode.objectify.Key;
import com.googlecode.objectify.cmd.Query; import com.googlecode.objectify.cmd.Query;
@ -44,7 +45,7 @@ import google.registry.util.FormattingLogger;
import google.registry.util.Idn; import google.registry.util.Idn;
import java.net.InetAddress; import java.net.InetAddress;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.LinkedHashSet; import java.util.Comparator;
import java.util.List; import java.util.List;
import java.util.Optional; import java.util.Optional;
import javax.inject.Inject; import javax.inject.Inject;
@ -71,7 +72,7 @@ public class RdapDomainSearchAction extends RdapActionBase {
public static final int RESULT_SET_SIZE_SCALING_FACTOR = 30; public static final int RESULT_SET_SIZE_SCALING_FACTOR = 30;
public static final int MAX_NAMESERVERS_IN_FIRST_STAGE = 1000; public static final int MAX_NAMESERVERS_IN_FIRST_STAGE = 300;
private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass();
@ -282,9 +283,9 @@ public class RdapDomainSearchAction extends RdapActionBase {
// must be present, to avoid querying every host in the system. This restriction is enforced by // must be present, to avoid querying every host in the system. This restriction is enforced by
// {@link queryItems}. // {@link queryItems}.
// //
// Only return the first 1000 nameservers. This could result in an incomplete result set if // Only return the first MAX_NAMESERVERS_IN_FIRST_STAGE nameservers. This could result in an
// a search asks for something like "ns*", but we need to enforce a limit in order to avoid // incomplete result set if a search asks for something like "ns*", but we need to enforce a
// arbitrarily long-running queries. // limit in order to avoid arbitrarily long-running queries.
Query<HostResource> query = Query<HostResource> query =
queryItems( queryItems(
HostResource.class, HostResource.class,
@ -379,9 +380,9 @@ public class RdapDomainSearchAction extends RdapActionBase {
* <p>In theory, we could have any number of hosts using the same IP address. To make sure we get * <p>In theory, we could have any number of hosts using the same IP address. To make sure we get
* all the associated domains, we have to retrieve all of them, and use them to look up domains. * all the associated domains, we have to retrieve all of them, and use them to look up domains.
* This could open us up to a kind of DoS attack if huge number of hosts are defined on a single * This could open us up to a kind of DoS attack if huge number of hosts are defined on a single
* IP. To avoid this, fetch only the first 1000 nameservers. In all normal circumstances, this * IP. To avoid this, fetch only the first {@link #MAX_NAMESERVERS_IN_FIRST_STAGE} nameservers. In
* should be orders of magnitude more than there actually are. But it could result in us missing * all normal circumstances, this should be orders of magnitude more than there actually are. But
* some domains. * it could result in us missing some domains.
* *
* <p>The includeDeleted parameter does NOT cause deleted nameservers to be searched, only deleted * <p>The includeDeleted parameter does NOT cause deleted nameservers to be searched, only deleted
* domains which used to be connected to an undeleted nameserver. * domains which used to be connected to an undeleted nameserver.
@ -413,9 +414,12 @@ public class RdapDomainSearchAction extends RdapActionBase {
// We must break the query up into chunks, because the in operator is limited to 30 subqueries. // We must break the query up into chunks, because the in operator is limited to 30 subqueries.
// Since it is possible for the same domain to show up more than once in our result list (if // Since it is possible for the same domain to show up more than once in our result list (if
// we do a wildcard nameserver search that returns multiple nameservers used by the same // we do a wildcard nameserver search that returns multiple nameservers used by the same
// domain), we must create a set of resulting {@link DomainResource} objects. But we use a // domain), we must create a set of resulting {@link DomainResource} objects. Use a sorted set,
// LinkedHashSet to preserve the order in which we found the domains. // and fetch all domains, to make sure that we can return the first domains in alphabetical
LinkedHashSet<DomainResource> domains = new LinkedHashSet<>(); // order.
ImmutableSortedSet.Builder<DomainResource> domainSetBuilder =
ImmutableSortedSet.orderedBy(
Comparator.comparing(DomainResource::getFullyQualifiedDomainName));
int numHostKeysSearched = 0; int numHostKeysSearched = 0;
for (List<Key<HostResource>> chunk : Iterables.partition(hostKeys, 30)) { for (List<Key<HostResource>> chunk : Iterables.partition(hostKeys, 30)) {
numHostKeysSearched += chunk.size(); numHostKeysSearched += chunk.size();
@ -425,23 +429,26 @@ public class RdapDomainSearchAction extends RdapActionBase {
if (!shouldIncludeDeleted()) { if (!shouldIncludeDeleted()) {
query = query.filter("deletionTime >", now); query = query.filter("deletionTime >", now);
} }
for (DomainResource domain : query.limit(rdapResultSetMaxSize + 1)) { Streams.stream(query)
if (!domains.contains(domain) && isAuthorized(domain, now)) { .filter(domain -> isAuthorized(domain, now))
if (domains.size() >= rdapResultSetMaxSize) { .forEach(domain -> domainSetBuilder.add(domain));
}
List<DomainResource> domains = domainSetBuilder.build().asList();
if (domains.size() > rdapResultSetMaxSize) {
return makeSearchResults( return makeSearchResults(
ImmutableList.copyOf(domains), IncompletenessWarningType.TRUNCATED, now); domains.subList(0, rdapResultSetMaxSize), IncompletenessWarningType.TRUNCATED, now);
} } else {
domains.add(domain); // If everything that we found will fit in the result, check whether there might have been
} // more results that got dropped because the first stage limit on number of nameservers. If
} // so, indicate the result might be incomplete.
}
return makeSearchResults( return makeSearchResults(
ImmutableList.copyOf(domains), domains,
(numHostKeysSearched >= MAX_NAMESERVERS_IN_FIRST_STAGE) (numHostKeysSearched >= MAX_NAMESERVERS_IN_FIRST_STAGE)
? IncompletenessWarningType.MIGHT_BE_INCOMPLETE ? IncompletenessWarningType.MIGHT_BE_INCOMPLETE
: IncompletenessWarningType.NONE, : IncompletenessWarningType.NONE,
now); now);
} }
}
/** Output JSON for a list of domains, with no incompleteness warnings. */ /** Output JSON for a list of domains, with no incompleteness warnings. */
private RdapSearchResults makeSearchResults(List<DomainResource> domains, DateTime now) { private RdapSearchResults makeSearchResults(List<DomainResource> domains, DateTime now) {

225
javatests/google/registry/rdap/RdapDomainSearchActionTest.java
View file

@ -516,7 +516,7 @@ public class RdapDomainSearchActionTest {
ImmutableSet.Builder<Key<HostResource>> hostKeysBuilder = new ImmutableSet.Builder<>(); ImmutableSet.Builder<Key<HostResource>> hostKeysBuilder = new ImmutableSet.Builder<>();
ImmutableSet.Builder<String> subordinateHostnamesBuilder = new ImmutableSet.Builder<>(); ImmutableSet.Builder<String> subordinateHostnamesBuilder = new ImmutableSet.Builder<>();
String mainDomainName = String.format("domain%d.lol", numTotalDomainsPerActiveDomain); String mainDomainName = String.format("domain%d.lol", numTotalDomainsPerActiveDomain);
for (int i = 1; i <= numHosts; i++) { for (int i = numHosts; i >= 1; i--) {
String hostName = String.format("ns%d.%s", i, mainDomainName); String hostName = String.format("ns%d.%s", i, mainDomainName);
subordinateHostnamesBuilder.add(hostName); subordinateHostnamesBuilder.add(hostName);
HostResource host = makeAndPersistHostResource( HostResource host = makeAndPersistHostResource(
@ -526,7 +526,7 @@ public class RdapDomainSearchActionTest {
ImmutableSet<Key<HostResource>> hostKeys = hostKeysBuilder.build(); ImmutableSet<Key<HostResource>> hostKeys = hostKeysBuilder.build();
// Create all the domains at once, then persist them in parallel, for increased efficiency. // Create all the domains at once, then persist them in parallel, for increased efficiency.
ImmutableList.Builder<DomainResource> domainsBuilder = new ImmutableList.Builder<>(); ImmutableList.Builder<DomainResource> domainsBuilder = new ImmutableList.Builder<>();
for (int i = 1; i <= numActiveDomains * numTotalDomainsPerActiveDomain; i++) { for (int i = numActiveDomains * numTotalDomainsPerActiveDomain; i >= 1; i--) {
String domainName = String.format("domain%d.lol", i); String domainName = String.format("domain%d.lol", i);
DomainResource.Builder builder = DomainResource.Builder builder =
makeDomainResource( makeDomainResource(
@ -624,6 +624,29 @@ public class RdapDomainSearchActionTest {
assertThat(response.getStatus()).isEqualTo(200); assertThat(response.getStatus()).isEqualTo(200);
} }
private void runSuccessfulTestWithFourDomains(
RequestType requestType,
String queryString,
String domainRoid1,
String domainRoid2,
String domainRoid3,
String domainRoid4,
String fileName) {
assertThat(generateActualJson(requestType, queryString))
.isEqualTo(
readMultiDomainFile(
fileName,
"domain1.lol",
domainRoid1,
"domain2.lol",
domainRoid2,
"domain3.lol",
domainRoid3,
"domain4.lol",
domainRoid4));
assertThat(response.getStatus()).isEqualTo(200);
}
private void runNotFoundTest(RequestType requestType, String fullName, String errorMessage) { private void runNotFoundTest(RequestType requestType, String fullName, String errorMessage) {
assertThat(generateActualJson(requestType, fullName)) assertThat(generateActualJson(requestType, fullName))
.isEqualTo(generateExpectedJson(errorMessage, "rdap_error_404.json")); .isEqualTo(generateExpectedJson(errorMessage, "rdap_error_404.json"));
@ -997,11 +1020,11 @@ public class RdapDomainSearchActionTest {
.isEqualTo(readMultiDomainFile( .isEqualTo(readMultiDomainFile(
"rdap_incomplete_domain_result_set.json", "rdap_incomplete_domain_result_set.json",
"domain100.lol", "domain100.lol",
"A6-LOL", "A7-LOL",
"domain150.lol", "domain150.lol",
"D8-LOL", "75-LOL",
"domain200.lol", "domain200.lol",
"10A-LOL", "43-LOL",
"domainunused.lol", "domainunused.lol",
"unused-LOL")); "unused-LOL"));
assertThat(response.getStatus()).isEqualTo(200); assertThat(response.getStatus()).isEqualTo(200);
@ -1010,35 +1033,27 @@ public class RdapDomainSearchActionTest {
@Test @Test
public void testDomainMatch_nontruncatedResultsSet() throws Exception { public void testDomainMatch_nontruncatedResultsSet() throws Exception {
createManyDomainsAndHosts(4, 1, 2); createManyDomainsAndHosts(4, 1, 2);
assertThat(generateActualJson(RequestType.NAME, "domain*.lol")) runSuccessfulTestWithFourDomains(
.isEqualTo(readMultiDomainFile( RequestType.NAME,
"rdap_nontruncated_domains.json", "domain*.lol",
"domain1.lol", "46-LOL",
"43-LOL",
"domain2.lol",
"44-LOL",
"domain3.lol",
"45-LOL", "45-LOL",
"domain4.lol", "44-LOL",
"46-LOL")); "43-LOL",
assertThat(response.getStatus()).isEqualTo(200); "rdap_nontruncated_domains.json");
} }
@Test @Test
public void testDomainMatch_truncatedResultsSet() throws Exception { public void testDomainMatch_truncatedResultsSet() throws Exception {
createManyDomainsAndHosts(5, 1, 2); createManyDomainsAndHosts(5, 1, 2);
assertThat(generateActualJson(RequestType.NAME, "domain*.lol")) runSuccessfulTestWithFourDomains(
.isEqualTo(readMultiDomainFile( RequestType.NAME,
"rdap_domains_four_truncated.json", "domain*.lol",
"domain1.lol", "47-LOL",
"43-LOL", "46-LOL",
"domain2.lol",
"44-LOL",
"domain3.lol",
"45-LOL", "45-LOL",
"domain4.lol", "44-LOL",
"46-LOL")); "rdap_domains_four_truncated.json");
assertThat(response.getStatus()).isEqualTo(200);
} }
@Test @Test
@ -1046,18 +1061,14 @@ public class RdapDomainSearchActionTest {
// Don't use 10 or more domains for this test, because domain10.lol will come before // Don't use 10 or more domains for this test, because domain10.lol will come before
// domain2.lol, and you'll get the wrong domains in the result set. // domain2.lol, and you'll get the wrong domains in the result set.
createManyDomainsAndHosts(9, 1, 2); createManyDomainsAndHosts(9, 1, 2);
assertThat(generateActualJson(RequestType.NAME, "domain*.lol")) runSuccessfulTestWithFourDomains(
.isEqualTo(readMultiDomainFile( RequestType.NAME,
"rdap_domains_four_truncated.json", "domain*.lol",
"domain1.lol", "4B-LOL",
"43-LOL", "4A-LOL",
"domain2.lol", "49-LOL",
"44-LOL", "48-LOL",
"domain3.lol", "rdap_domains_four_truncated.json");
"45-LOL",
"domain4.lol",
"46-LOL"));
assertThat(response.getStatus()).isEqualTo(200);
} }
@Test @Test
@ -1067,13 +1078,13 @@ public class RdapDomainSearchActionTest {
.isEqualTo(readMultiDomainFile( .isEqualTo(readMultiDomainFile(
"rdap_domains_four_truncated.json", "rdap_domains_four_truncated.json",
"domain12.lol", "domain12.lol",
"4E-LOL", "55-LOL",
"domain18.lol", "domain18.lol",
"54-LOL", "4F-LOL",
"domain24.lol", "domain24.lol",
"5A-LOL", "49-LOL",
"domain30.lol", "domain30.lol",
"60-LOL")); "43-LOL"));
assertThat(response.getStatus()).isEqualTo(200); assertThat(response.getStatus()).isEqualTo(200);
} }
@ -1244,7 +1255,7 @@ public class RdapDomainSearchActionTest {
runNotFoundTest(RequestType.NS_LDH_NAME, "ns1.missing.com", "No matching nameservers found"); runNotFoundTest(RequestType.NS_LDH_NAME, "ns1.missing.com", "No matching nameservers found");
} }
// todo (b/27378695): reenable or delete this test // TODO(b/27378695): reenable or delete this test
@Ignore @Ignore
@Test @Test
public void testNameserverMatchDomainsInTestTld_notFound() throws Exception { public void testNameserverMatchDomainsInTestTld_notFound() throws Exception {
@ -1354,52 +1365,40 @@ public class RdapDomainSearchActionTest {
@Test @Test
public void testNameserverMatch_nontruncatedResultsSet() throws Exception { public void testNameserverMatch_nontruncatedResultsSet() throws Exception {
createManyDomainsAndHosts(4, 1, 2); createManyDomainsAndHosts(4, 1, 2);
assertThat(generateActualJson(RequestType.NS_LDH_NAME, "ns1.domain1.lol")) runSuccessfulTestWithFourDomains(
.isEqualTo(readMultiDomainFile( RequestType.NS_LDH_NAME,
"rdap_nontruncated_domains.json", "ns1.domain1.lol",
"domain1.lol", "46-LOL",
"43-LOL",
"domain2.lol",
"44-LOL",
"domain3.lol",
"45-LOL", "45-LOL",
"domain4.lol", "44-LOL",
"46-LOL")); "43-LOL",
assertThat(response.getStatus()).isEqualTo(200); "rdap_nontruncated_domains.json");
} }
@Test @Test
public void testNameserverMatch_truncatedResultsSet() throws Exception { public void testNameserverMatch_truncatedResultsSet() throws Exception {
createManyDomainsAndHosts(5, 1, 2); createManyDomainsAndHosts(5, 1, 2);
assertThat(generateActualJson(RequestType.NS_LDH_NAME, "ns1.domain1.lol")) runSuccessfulTestWithFourDomains(
.isEqualTo(readMultiDomainFile( RequestType.NS_LDH_NAME,
"rdap_domains_four_truncated.json", "ns1.domain1.lol",
"domain1.lol", "47-LOL",
"43-LOL", "46-LOL",
"domain2.lol",
"44-LOL",
"domain3.lol",
"45-LOL", "45-LOL",
"domain4.lol", "44-LOL",
"46-LOL")); "rdap_domains_four_truncated.json");
assertThat(response.getStatus()).isEqualTo(200);
} }
@Test @Test
public void testNameserverMatch_reallyTruncatedResultsSet() throws Exception { public void testNameserverMatch_reallyTruncatedResultsSet() throws Exception {
createManyDomainsAndHosts(9, 1, 2); createManyDomainsAndHosts(9, 1, 2);
assertThat(generateActualJson(RequestType.NS_LDH_NAME, "ns1.domain1.lol")) runSuccessfulTestWithFourDomains(
.isEqualTo(readMultiDomainFile( RequestType.NS_LDH_NAME,
"rdap_domains_four_truncated.json", "ns1.domain1.lol",
"domain1.lol", "4B-LOL",
"43-LOL", "4A-LOL",
"domain2.lol", "49-LOL",
"44-LOL", "48-LOL",
"domain3.lol", "rdap_domains_four_truncated.json");
"45-LOL",
"domain4.lol",
"46-LOL"));
assertThat(response.getStatus()).isEqualTo(200);
} }
@Test @Test
@ -1412,13 +1411,13 @@ public class RdapDomainSearchActionTest {
.isEqualTo(readMultiDomainFile( .isEqualTo(readMultiDomainFile(
"rdap_nontruncated_domains.json", "rdap_nontruncated_domains.json",
"domain1.lol", "domain1.lol",
"B7-LOL", "BA-LOL",
"domain2.lol", "domain2.lol",
"B8-LOL",
"domain3.lol",
"B9-LOL", "B9-LOL",
"domain3.lol",
"B8-LOL",
"domain4.lol", "domain4.lol",
"BA-LOL")); "B7-LOL"));
assertThat(response.getStatus()).isEqualTo(200); assertThat(response.getStatus()).isEqualTo(200);
} }
@ -1429,9 +1428,9 @@ public class RdapDomainSearchActionTest {
.isEqualTo(readMultiDomainFile( .isEqualTo(readMultiDomainFile(
"rdap_incomplete_domains.json", "rdap_incomplete_domains.json",
"domain1.lol", "domain1.lol",
"13C7-LOL",
"domain2.lol",
"13C8-LOL", "13C8-LOL",
"domain2.lol",
"13C7-LOL",
"x", "x",
"x", "x",
"x", "x",
@ -1473,7 +1472,7 @@ public class RdapDomainSearchActionTest {
runNotFoundTest(RequestType.NS_IP, "127.0.0.1", "No domains found"); runNotFoundTest(RequestType.NS_IP, "127.0.0.1", "No domains found");
} }
// todo (b/27378695): reenable or delete this test // TODO(b/27378695): reenable or delete this test
@Ignore @Ignore
@Test @Test
public void testAddressMatchDomainsInTestTld_notFound() throws Exception { public void testAddressMatchDomainsInTestTld_notFound() throws Exception {
@ -1547,51 +1546,39 @@ public class RdapDomainSearchActionTest {
@Test @Test
public void testAddressMatch_nontruncatedResultsSet() throws Exception { public void testAddressMatch_nontruncatedResultsSet() throws Exception {
createManyDomainsAndHosts(4, 1, 2); createManyDomainsAndHosts(4, 1, 2);
assertThat(generateActualJson(RequestType.NS_IP, "5.5.5.1")) runSuccessfulTestWithFourDomains(
.isEqualTo(readMultiDomainFile( RequestType.NS_IP,
"rdap_nontruncated_domains.json", "5.5.5.1",
"domain1.lol", "46-LOL",
"43-LOL",
"domain2.lol",
"44-LOL",
"domain3.lol",
"45-LOL", "45-LOL",
"domain4.lol", "44-LOL",
"46-LOL")); "43-LOL",
assertThat(response.getStatus()).isEqualTo(200); "rdap_nontruncated_domains.json");
} }
@Test @Test
public void testAddressMatch_truncatedResultsSet() throws Exception { public void testAddressMatch_truncatedResultsSet() throws Exception {
createManyDomainsAndHosts(5, 1, 2); createManyDomainsAndHosts(5, 1, 2);
assertThat(generateActualJson(RequestType.NS_IP, "5.5.5.1")) runSuccessfulTestWithFourDomains(
.isEqualTo(readMultiDomainFile( RequestType.NS_IP,
"rdap_domains_four_truncated.json", "5.5.5.1",
"domain1.lol", "47-LOL",
"43-LOL", "46-LOL",
"domain2.lol",
"44-LOL",
"domain3.lol",
"45-LOL", "45-LOL",
"domain4.lol", "44-LOL",
"46-LOL")); "rdap_domains_four_truncated.json");
assertThat(response.getStatus()).isEqualTo(200);
} }
@Test @Test
public void testAddressMatch_reallyTruncatedResultsSet() throws Exception { public void testAddressMatch_reallyTruncatedResultsSet() throws Exception {
createManyDomainsAndHosts(9, 1, 2); createManyDomainsAndHosts(9, 1, 2);
assertThat(generateActualJson(RequestType.NS_IP, "5.5.5.1")) runSuccessfulTestWithFourDomains(
.isEqualTo(readMultiDomainFile( RequestType.NS_IP,
"rdap_domains_four_truncated.json", "5.5.5.1",
"domain1.lol", "4B-LOL",
"43-LOL", "4A-LOL",
"domain2.lol", "49-LOL",
"44-LOL", "48-LOL",
"domain3.lol", "rdap_domains_four_truncated.json");
"45-LOL",
"domain4.lol",
"46-LOL"));
assertThat(response.getStatus()).isEqualTo(200);
} }
} }