diff --git a/java/google/registry/rdap/RdapDomainSearchAction.java b/java/google/registry/rdap/RdapDomainSearchAction.java index dde33bd62..394ec3b99 100644 --- a/java/google/registry/rdap/RdapDomainSearchAction.java +++ b/java/google/registry/rdap/RdapDomainSearchAction.java @@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; +import com.google.common.collect.Streams; import com.google.common.primitives.Booleans; import com.googlecode.objectify.Key; import com.googlecode.objectify.cmd.Query; @@ -44,7 +45,7 @@ import google.registry.util.FormattingLogger; import google.registry.util.Idn; import java.net.InetAddress; import java.util.ArrayList; -import java.util.LinkedHashSet; +import java.util.Comparator; import java.util.List; import java.util.Optional; 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 MAX_NAMESERVERS_IN_FIRST_STAGE = 1000; + public static final int MAX_NAMESERVERS_IN_FIRST_STAGE = 300; 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 // {@link queryItems}. // - // Only return the first 1000 nameservers. This could result in an incomplete result set if - // a search asks for something like "ns*", but we need to enforce a limit in order to avoid - // arbitrarily long-running queries. + // Only return the first MAX_NAMESERVERS_IN_FIRST_STAGE nameservers. This could result in an + // incomplete result set if a search asks for something like "ns*", but we need to enforce a + // limit in order to avoid arbitrarily long-running queries. Query query = queryItems( HostResource.class, @@ -379,9 +380,9 @@ public class RdapDomainSearchAction extends RdapActionBase { *

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. * 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 - * should be orders of magnitude more than there actually are. But it could result in us missing - * some domains. + * IP. To avoid this, fetch only the first {@link #MAX_NAMESERVERS_IN_FIRST_STAGE} nameservers. In + * all normal circumstances, this should be orders of magnitude more than there actually are. But + * it could result in us missing some domains. * *

The includeDeleted parameter does NOT cause deleted nameservers to be searched, only deleted * 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. // 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 - // domain), we must create a set of resulting {@link DomainResource} objects. But we use a - // LinkedHashSet to preserve the order in which we found the domains. - LinkedHashSet domains = new LinkedHashSet<>(); + // domain), we must create a set of resulting {@link DomainResource} objects. Use a sorted set, + // and fetch all domains, to make sure that we can return the first domains in alphabetical + // order. + ImmutableSortedSet.Builder domainSetBuilder = + ImmutableSortedSet.orderedBy( + Comparator.comparing(DomainResource::getFullyQualifiedDomainName)); int numHostKeysSearched = 0; for (List> chunk : Iterables.partition(hostKeys, 30)) { numHostKeysSearched += chunk.size(); @@ -425,22 +429,25 @@ public class RdapDomainSearchAction extends RdapActionBase { if (!shouldIncludeDeleted()) { query = query.filter("deletionTime >", now); } - for (DomainResource domain : query.limit(rdapResultSetMaxSize + 1)) { - if (!domains.contains(domain) && isAuthorized(domain, now)) { - if (domains.size() >= rdapResultSetMaxSize) { - return makeSearchResults( - ImmutableList.copyOf(domains), IncompletenessWarningType.TRUNCATED, now); - } - domains.add(domain); - } - } + Streams.stream(query) + .filter(domain -> isAuthorized(domain, now)) + .forEach(domain -> domainSetBuilder.add(domain)); + } + List domains = domainSetBuilder.build().asList(); + if (domains.size() > rdapResultSetMaxSize) { + return makeSearchResults( + domains.subList(0, rdapResultSetMaxSize), IncompletenessWarningType.TRUNCATED, now); + } else { + // 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( + domains, + (numHostKeysSearched >= MAX_NAMESERVERS_IN_FIRST_STAGE) + ? IncompletenessWarningType.MIGHT_BE_INCOMPLETE + : IncompletenessWarningType.NONE, + now); } - return makeSearchResults( - ImmutableList.copyOf(domains), - (numHostKeysSearched >= MAX_NAMESERVERS_IN_FIRST_STAGE) - ? IncompletenessWarningType.MIGHT_BE_INCOMPLETE - : IncompletenessWarningType.NONE, - now); } /** Output JSON for a list of domains, with no incompleteness warnings. */ diff --git a/javatests/google/registry/rdap/RdapDomainSearchActionTest.java b/javatests/google/registry/rdap/RdapDomainSearchActionTest.java index faf0be612..e9697c054 100644 --- a/javatests/google/registry/rdap/RdapDomainSearchActionTest.java +++ b/javatests/google/registry/rdap/RdapDomainSearchActionTest.java @@ -516,7 +516,7 @@ public class RdapDomainSearchActionTest { ImmutableSet.Builder> hostKeysBuilder = new ImmutableSet.Builder<>(); ImmutableSet.Builder subordinateHostnamesBuilder = new ImmutableSet.Builder<>(); 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); subordinateHostnamesBuilder.add(hostName); HostResource host = makeAndPersistHostResource( @@ -526,7 +526,7 @@ public class RdapDomainSearchActionTest { ImmutableSet> hostKeys = hostKeysBuilder.build(); // Create all the domains at once, then persist them in parallel, for increased efficiency. ImmutableList.Builder 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); DomainResource.Builder builder = makeDomainResource( @@ -624,6 +624,29 @@ public class RdapDomainSearchActionTest { 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) { assertThat(generateActualJson(requestType, fullName)) .isEqualTo(generateExpectedJson(errorMessage, "rdap_error_404.json")); @@ -997,11 +1020,11 @@ public class RdapDomainSearchActionTest { .isEqualTo(readMultiDomainFile( "rdap_incomplete_domain_result_set.json", "domain100.lol", - "A6-LOL", + "A7-LOL", "domain150.lol", - "D8-LOL", + "75-LOL", "domain200.lol", - "10A-LOL", + "43-LOL", "domainunused.lol", "unused-LOL")); assertThat(response.getStatus()).isEqualTo(200); @@ -1010,35 +1033,27 @@ public class RdapDomainSearchActionTest { @Test public void testDomainMatch_nontruncatedResultsSet() throws Exception { createManyDomainsAndHosts(4, 1, 2); - assertThat(generateActualJson(RequestType.NAME, "domain*.lol")) - .isEqualTo(readMultiDomainFile( - "rdap_nontruncated_domains.json", - "domain1.lol", - "43-LOL", - "domain2.lol", - "44-LOL", - "domain3.lol", - "45-LOL", - "domain4.lol", - "46-LOL")); - assertThat(response.getStatus()).isEqualTo(200); + runSuccessfulTestWithFourDomains( + RequestType.NAME, + "domain*.lol", + "46-LOL", + "45-LOL", + "44-LOL", + "43-LOL", + "rdap_nontruncated_domains.json"); } @Test public void testDomainMatch_truncatedResultsSet() throws Exception { createManyDomainsAndHosts(5, 1, 2); - assertThat(generateActualJson(RequestType.NAME, "domain*.lol")) - .isEqualTo(readMultiDomainFile( - "rdap_domains_four_truncated.json", - "domain1.lol", - "43-LOL", - "domain2.lol", - "44-LOL", - "domain3.lol", - "45-LOL", - "domain4.lol", - "46-LOL")); - assertThat(response.getStatus()).isEqualTo(200); + runSuccessfulTestWithFourDomains( + RequestType.NAME, + "domain*.lol", + "47-LOL", + "46-LOL", + "45-LOL", + "44-LOL", + "rdap_domains_four_truncated.json"); } @Test @@ -1046,18 +1061,14 @@ public class RdapDomainSearchActionTest { // 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. createManyDomainsAndHosts(9, 1, 2); - assertThat(generateActualJson(RequestType.NAME, "domain*.lol")) - .isEqualTo(readMultiDomainFile( - "rdap_domains_four_truncated.json", - "domain1.lol", - "43-LOL", - "domain2.lol", - "44-LOL", - "domain3.lol", - "45-LOL", - "domain4.lol", - "46-LOL")); - assertThat(response.getStatus()).isEqualTo(200); + runSuccessfulTestWithFourDomains( + RequestType.NAME, + "domain*.lol", + "4B-LOL", + "4A-LOL", + "49-LOL", + "48-LOL", + "rdap_domains_four_truncated.json"); } @Test @@ -1067,13 +1078,13 @@ public class RdapDomainSearchActionTest { .isEqualTo(readMultiDomainFile( "rdap_domains_four_truncated.json", "domain12.lol", - "4E-LOL", + "55-LOL", "domain18.lol", - "54-LOL", + "4F-LOL", "domain24.lol", - "5A-LOL", + "49-LOL", "domain30.lol", - "60-LOL")); + "43-LOL")); assertThat(response.getStatus()).isEqualTo(200); } @@ -1244,7 +1255,7 @@ public class RdapDomainSearchActionTest { 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 @Test public void testNameserverMatchDomainsInTestTld_notFound() throws Exception { @@ -1354,52 +1365,40 @@ public class RdapDomainSearchActionTest { @Test public void testNameserverMatch_nontruncatedResultsSet() throws Exception { createManyDomainsAndHosts(4, 1, 2); - assertThat(generateActualJson(RequestType.NS_LDH_NAME, "ns1.domain1.lol")) - .isEqualTo(readMultiDomainFile( - "rdap_nontruncated_domains.json", - "domain1.lol", - "43-LOL", - "domain2.lol", - "44-LOL", - "domain3.lol", - "45-LOL", - "domain4.lol", - "46-LOL")); - assertThat(response.getStatus()).isEqualTo(200); + runSuccessfulTestWithFourDomains( + RequestType.NS_LDH_NAME, + "ns1.domain1.lol", + "46-LOL", + "45-LOL", + "44-LOL", + "43-LOL", + "rdap_nontruncated_domains.json"); } @Test public void testNameserverMatch_truncatedResultsSet() throws Exception { createManyDomainsAndHosts(5, 1, 2); - assertThat(generateActualJson(RequestType.NS_LDH_NAME, "ns1.domain1.lol")) - .isEqualTo(readMultiDomainFile( - "rdap_domains_four_truncated.json", - "domain1.lol", - "43-LOL", - "domain2.lol", - "44-LOL", - "domain3.lol", - "45-LOL", - "domain4.lol", - "46-LOL")); - assertThat(response.getStatus()).isEqualTo(200); + runSuccessfulTestWithFourDomains( + RequestType.NS_LDH_NAME, + "ns1.domain1.lol", + "47-LOL", + "46-LOL", + "45-LOL", + "44-LOL", + "rdap_domains_four_truncated.json"); } @Test public void testNameserverMatch_reallyTruncatedResultsSet() throws Exception { createManyDomainsAndHosts(9, 1, 2); - assertThat(generateActualJson(RequestType.NS_LDH_NAME, "ns1.domain1.lol")) - .isEqualTo(readMultiDomainFile( - "rdap_domains_four_truncated.json", - "domain1.lol", - "43-LOL", - "domain2.lol", - "44-LOL", - "domain3.lol", - "45-LOL", - "domain4.lol", - "46-LOL")); - assertThat(response.getStatus()).isEqualTo(200); + runSuccessfulTestWithFourDomains( + RequestType.NS_LDH_NAME, + "ns1.domain1.lol", + "4B-LOL", + "4A-LOL", + "49-LOL", + "48-LOL", + "rdap_domains_four_truncated.json"); } @Test @@ -1412,13 +1411,13 @@ public class RdapDomainSearchActionTest { .isEqualTo(readMultiDomainFile( "rdap_nontruncated_domains.json", "domain1.lol", - "B7-LOL", + "BA-LOL", "domain2.lol", - "B8-LOL", - "domain3.lol", "B9-LOL", + "domain3.lol", + "B8-LOL", "domain4.lol", - "BA-LOL")); + "B7-LOL")); assertThat(response.getStatus()).isEqualTo(200); } @@ -1429,9 +1428,9 @@ public class RdapDomainSearchActionTest { .isEqualTo(readMultiDomainFile( "rdap_incomplete_domains.json", "domain1.lol", - "13C7-LOL", - "domain2.lol", "13C8-LOL", + "domain2.lol", + "13C7-LOL", "x", "x", "x", @@ -1473,7 +1472,7 @@ public class RdapDomainSearchActionTest { 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 @Test public void testAddressMatchDomainsInTestTld_notFound() throws Exception { @@ -1547,51 +1546,39 @@ public class RdapDomainSearchActionTest { @Test public void testAddressMatch_nontruncatedResultsSet() throws Exception { createManyDomainsAndHosts(4, 1, 2); - assertThat(generateActualJson(RequestType.NS_IP, "5.5.5.1")) - .isEqualTo(readMultiDomainFile( - "rdap_nontruncated_domains.json", - "domain1.lol", - "43-LOL", - "domain2.lol", - "44-LOL", - "domain3.lol", - "45-LOL", - "domain4.lol", - "46-LOL")); - assertThat(response.getStatus()).isEqualTo(200); + runSuccessfulTestWithFourDomains( + RequestType.NS_IP, + "5.5.5.1", + "46-LOL", + "45-LOL", + "44-LOL", + "43-LOL", + "rdap_nontruncated_domains.json"); } @Test public void testAddressMatch_truncatedResultsSet() throws Exception { createManyDomainsAndHosts(5, 1, 2); - assertThat(generateActualJson(RequestType.NS_IP, "5.5.5.1")) - .isEqualTo(readMultiDomainFile( - "rdap_domains_four_truncated.json", - "domain1.lol", - "43-LOL", - "domain2.lol", - "44-LOL", - "domain3.lol", - "45-LOL", - "domain4.lol", - "46-LOL")); - assertThat(response.getStatus()).isEqualTo(200); + runSuccessfulTestWithFourDomains( + RequestType.NS_IP, + "5.5.5.1", + "47-LOL", + "46-LOL", + "45-LOL", + "44-LOL", + "rdap_domains_four_truncated.json"); } @Test public void testAddressMatch_reallyTruncatedResultsSet() throws Exception { createManyDomainsAndHosts(9, 1, 2); - assertThat(generateActualJson(RequestType.NS_IP, "5.5.5.1")) - .isEqualTo(readMultiDomainFile( - "rdap_domains_four_truncated.json", - "domain1.lol", - "43-LOL", - "domain2.lol", - "44-LOL", - "domain3.lol", - "45-LOL", - "domain4.lol", - "46-LOL")); - assertThat(response.getStatus()).isEqualTo(200); + runSuccessfulTestWithFourDomains( + RequestType.NS_IP, + "5.5.5.1", + "4B-LOL", + "4A-LOL", + "49-LOL", + "48-LOL", + "rdap_domains_four_truncated.json"); } }