Return proper RDAP error messages when invalid IP addresses are specified

We were relying on Dagger to validate the IP address, but that resulted in 500 errors when the IP address was not valid, which is undesirable. Instead, accept the parameters as strings, then convert them to IP addresses and throw a proper error when conversion fails.

Also fixes an improperly specified test.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=173172516
This commit is contained in:
mountford 2017-10-23 14:40:05 -07:00 committed by jianglai
parent 52fd9d8c4e
commit 4267fa7e48
6 changed files with 39 additions and 38 deletions

View file

@ -26,6 +26,7 @@ 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.net.InetAddresses;
import com.google.common.primitives.Booleans;
import com.googlecode.objectify.Key;
import com.googlecode.objectify.cmd.Query;
@ -79,7 +80,7 @@ public class RdapDomainSearchAction extends RdapActionBase {
@Inject Clock clock;
@Inject @Parameter("name") Optional<String> nameParam;
@Inject @Parameter("nsLdhName") Optional<String> nsLdhNameParam;
@Inject @Parameter("nsIp") Optional<InetAddress> nsIpParam;
@Inject @Parameter("nsIp") Optional<String> nsIpParam;
@Inject RdapDomainSearchAction() {}
@Override
@ -132,7 +133,13 @@ public class RdapDomainSearchAction extends RdapActionBase {
RdapSearchPattern.create(nsLdhNameParam.get(), true), now);
} else {
// syntax: /rdap/domains?nsIp=1.2.3.4
results = searchByNameserverIp(nsIpParam.get(), now);
InetAddress inetAddress;
try {
inetAddress = InetAddresses.forString(nsIpParam.get());
} catch (IllegalArgumentException e) {
throw new BadRequestException("Invalid value of nsIp parameter");
}
results = searchByNameserverIp(inetAddress, now);
}
if (results.jsonList().isEmpty()) {
throw new NotFoundException("No domains found");

View file

@ -18,7 +18,6 @@ import dagger.Module;
import dagger.Provides;
import google.registry.request.Parameter;
import google.registry.request.RequestParameters;
import java.net.InetAddress;
import java.util.Optional;
import javax.servlet.http.HttpServletRequest;
@ -40,14 +39,14 @@ public final class RdapModule {
@Provides
@Parameter("nsIp")
static Optional<InetAddress> provideNsIp(HttpServletRequest req) {
return RequestParameters.extractOptionalInetAddressParameter(req, "nsIp");
static Optional<String> provideNsIp(HttpServletRequest req) {
return RequestParameters.extractOptionalParameter(req, "nsIp");
}
@Provides
@Parameter("ip")
static Optional<InetAddress> provideIp(HttpServletRequest req) {
return RequestParameters.extractOptionalInetAddressParameter(req, "ip");
static Optional<String> provideIp(HttpServletRequest req) {
return RequestParameters.extractOptionalParameter(req, "ip");
}
@Provides

View file

@ -22,6 +22,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.net.InetAddresses;
import com.google.common.primitives.Booleans;
import com.googlecode.objectify.cmd.Query;
import google.registry.model.domain.DomainResource;
@ -66,7 +67,7 @@ public class RdapNameserverSearchAction extends RdapActionBase {
@Inject Clock clock;
@Inject @Parameter("name") Optional<String> nameParam;
@Inject @Parameter("ip") Optional<InetAddress> ipParam;
@Inject @Parameter("ip") Optional<String> ipParam;
@Inject RdapNameserverSearchAction() {}
@Override
@ -107,7 +108,13 @@ public class RdapNameserverSearchAction extends RdapActionBase {
results = searchByName(RdapSearchPattern.create(Idn.toASCII(nameParam.get()), true), now);
} else {
// syntax: /rdap/nameservers?ip=1.2.3.4
results = searchByIp(ipParam.get(), now);
InetAddress inetAddress;
try {
inetAddress = InetAddresses.forString(ipParam.get());
} catch (IllegalArgumentException e) {
throw new BadRequestException("Invalid value of ip parameter");
}
results = searchByIp(inetAddress, now);
}
if (results.jsonList().isEmpty()) {
throw new NotFoundException("No nameservers found");

View file

@ -20,9 +20,7 @@ import static com.google.common.base.Strings.nullToEmpty;
import com.google.common.base.Ascii;
import com.google.common.collect.ImmutableSet;
import com.google.common.net.InetAddresses;
import google.registry.request.HttpException.BadRequestException;
import java.net.InetAddress;
import java.util.Optional;
import javax.annotation.Nullable;
import javax.servlet.http.HttpServletRequest;
@ -215,26 +213,6 @@ public final class RequestParameters {
return datesBuilder.build();
}
/**
* Returns first request parameter associated with {@code name} parsed as an optional
* {@link InetAddress} (which might be IPv6).
*
* @throws BadRequestException if request parameter named {@code name} is present but could not
* be parsed as an {@link InetAddress}
*/
public static Optional<InetAddress> extractOptionalInetAddressParameter(
HttpServletRequest req, String name) {
Optional<String> paramVal = extractOptionalParameter(req, name);
if (!paramVal.isPresent()) {
return Optional.empty();
}
try {
return Optional.of(InetAddresses.forString(paramVal.get()));
} catch (IllegalArgumentException e) {
throw new BadRequestException("Not an IPv4 or IPv6 address: " + name);
}
}
private static boolean equalsFalse(@Nullable String value) {
return nullToEmpty(value).equalsIgnoreCase("false");
}

View file

@ -126,7 +126,7 @@ public class RdapDomainSearchActionTest {
case NS_IP:
action.nameParam = Optional.empty();
action.nsLdhNameParam = Optional.empty();
action.nsIpParam = Optional.of(InetAddresses.forString(paramValue));
action.nsIpParam = Optional.of(paramValue);
break;
default:
action.nameParam = Optional.empty();
@ -1438,6 +1438,12 @@ public class RdapDomainSearchActionTest {
assertThat(response.getStatus()).isEqualTo(200);
}
@Test
public void testAddressMatchV4Address_invalidAddress() throws Exception {
generateActualJson(RequestType.NS_IP, "1.2.3.4.5.6.7.8.9");
assertThat(response.getStatus()).isEqualTo(400);
}
@Test
public void testAddressMatchV4Address_foundMultiple() throws Exception {
assertThat(generateActualJson(RequestType.NS_IP, "1.2.3.4"))

View file

@ -33,7 +33,6 @@ import com.google.appengine.api.users.User;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.net.InetAddresses;
import com.googlecode.objectify.Key;
import google.registry.model.domain.DomainResource;
import google.registry.model.host.HostResource;
@ -78,7 +77,6 @@ public class RdapNameserverSearchActionTest {
private DomainResource domainCatLol;
private HostResource hostNs1CatLol;
private HostResource hostNs2CatLol;
private HostResource hostNs1Cat2Lol;
private Object generateActualJsonWithName(String name) {
action.nameParam = Optional.of(name);
@ -87,7 +85,7 @@ public class RdapNameserverSearchActionTest {
}
private Object generateActualJsonWithIp(String ipString) {
action.ipParam = Optional.of(InetAddresses.forString(ipString));
action.ipParam = Optional.of(ipString);
action.run();
return JSONValue.parse(response.getPayload());
}
@ -104,7 +102,7 @@ public class RdapNameserverSearchActionTest {
"ns1.cat.lol", "1.2.3.4", clock.nowUtc().minusYears(1));
hostNs2CatLol = makeAndPersistHostResource(
"ns2.cat.lol", "bad:f00d:cafe::15:beef", clock.nowUtc().minusYears(1));
hostNs1Cat2Lol = makeAndPersistHostResource(
makeAndPersistHostResource(
"ns1.cat2.lol", "1.2.3.3", "bad:f00d:cafe::15:beef", clock.nowUtc().minusYears(1));
makeAndPersistHostResource("ns1.cat.external", null, null, clock.nowUtc().minusYears(1));
@ -487,8 +485,8 @@ public class RdapNameserverSearchActionTest {
@Test
public void testNameMatchDeletedHost_foundTheOtherHost() throws Exception {
persistResource(
hostNs1Cat2Lol.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build());
assertThat(generateActualJsonWithIp("bad:f00d:cafe::15:beef"))
hostNs1CatLol.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build());
assertThat(generateActualJsonWithName("ns*.cat.lol"))
.isEqualTo(
generateExpectedJsonForNameserver(
"ns2.cat.lol",
@ -586,6 +584,12 @@ public class RdapNameserverSearchActionTest {
assertThat(response.getStatus()).isEqualTo(404);
}
@Test
public void testAddressMatch_invalidAddress() throws Exception {
generateActualJsonWithIp("It is to laugh");
assertThat(response.getStatus()).isEqualTo(400);
}
@Test
public void testAddressMatchV4Address_found() throws Exception {
assertThat(generateActualJsonWithIp("1.2.3.4"))