diff --git a/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsMessageTransport.java b/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsMessageTransport.java deleted file mode 100644 index 99e4347d056..00000000000 --- a/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsMessageTransport.java +++ /dev/null @@ -1,134 +0,0 @@ -// Copyright 2017 The Nomulus Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package google.registry.dns.writer.dnsupdate; - -import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Verify.verify; - -import com.google.common.annotations.VisibleForTesting; -import com.google.common.primitives.Ints; -import google.registry.config.RegistryConfig.Config; -import jakarta.inject.Inject; -import java.io.DataInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.net.InetAddress; -import java.net.Socket; -import java.nio.ByteBuffer; -import java.time.Duration; -import javax.net.SocketFactory; -import org.xbill.DNS.Message; -import org.xbill.DNS.Opcode; - -/** - * A transport for DNS messages. Sends/receives DNS messages over TCP using old-style {@link Socket} - * s and the message framing defined in RFC 1035. - * - *
TODO(b/463732345): now that we're no longer on AppEngine, see if we can use the dnsjava - * library's {@link org.xbill.DNS.SimpleResolver} class instead of this (that requires {@link - * java.nio.channels.SocketChannel} which is not supported on AppEngine). - */ -public class DnsMessageTransport { - - /** - * Size of message length field for DNS TCP transport. - * - * @see RFC 1035 - */ - static final int MESSAGE_LENGTH_FIELD_BYTES = 2; - private static final int MESSAGE_MAXIMUM_LENGTH = (1 << (MESSAGE_LENGTH_FIELD_BYTES * 8)) - 1; - - /** - * The standard DNS port number. - * - * @see RFC 1035 - */ - @VisibleForTesting static final int DNS_PORT = 53; - - private final SocketFactory factory; - private final String updateHost; - private final int updateTimeout; - - /** - * Class constructor. - * - * @param factory a factory for TCP sockets - * @param updateHost host name of the DNS server - * @param updateTimeout update I/O timeout - */ - @Inject - public DnsMessageTransport( - SocketFactory factory, - @Config("dnsUpdateHost") String updateHost, - @Config("dnsUpdateTimeout") Duration updateTimeout) { - this.factory = factory; - this.updateHost = updateHost; - this.updateTimeout = Ints.checkedCast(updateTimeout.toMillis()); - } - - /** - * Sends a DNS "query" message (most likely an UPDATE) and returns the response. The response is - * checked for matching ID and opcode. - * - * @param query a message to send - * @return the response received from the server - * @throws IOException if the Socket input/output streams throws one - * @throws IllegalArgumentException if the query is too large to be sent (> 65535 bytes) - */ - public Message send(Message query) throws IOException { - try (Socket socket = factory.createSocket(InetAddress.getByName(updateHost), DNS_PORT)) { - socket.setSoTimeout(updateTimeout); - writeMessage(socket.getOutputStream(), query); - Message response = readMessage(socket.getInputStream()); - checkValidResponse(query, response); - return response; - } - } - - private void checkValidResponse(Message query, Message response) { - verify( - response.getHeader().getID() == query.getHeader().getID(), - "response ID %s does not match query ID %s", - response.getHeader().getID(), - query.getHeader().getID()); - verify( - response.getHeader().getOpcode() == query.getHeader().getOpcode(), - "response opcode '%s' does not match query opcode '%s'", - Opcode.string(response.getHeader().getOpcode()), - Opcode.string(query.getHeader().getOpcode())); - } - - private void writeMessage(OutputStream outputStream, Message message) throws IOException { - byte[] messageData = message.toWire(); - checkArgument( - messageData.length <= MESSAGE_MAXIMUM_LENGTH, - "DNS request message larger than maximum of %s: %s", - MESSAGE_MAXIMUM_LENGTH, - messageData.length); - ByteBuffer buffer = ByteBuffer.allocate(messageData.length + MESSAGE_LENGTH_FIELD_BYTES); - buffer.putShort((short) messageData.length); - buffer.put(messageData); - outputStream.write(buffer.array()); - } - - private Message readMessage(InputStream inputStream) throws IOException { - DataInputStream stream = new DataInputStream(inputStream); - int length = stream.readUnsignedShort(); - byte[] messageData = new byte[length]; - stream.readFully(messageData); - return new Message(messageData); - } -} diff --git a/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriter.java b/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriter.java index 0c1b1414764..0778d958c2e 100644 --- a/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriter.java +++ b/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriter.java @@ -50,6 +50,7 @@ import org.xbill.DNS.Name; import org.xbill.DNS.RRset; import org.xbill.DNS.Rcode; +import org.xbill.DNS.Resolver; import org.xbill.DNS.TextParseException; import org.xbill.DNS.Type; import org.xbill.DNS.Update; @@ -58,7 +59,7 @@ * A DnsWriter that implements the DNS UPDATE protocol as specified in RFC 2136. Publishes changes in the domain-registry * to a (capable) external DNS server, sometimes called a "hidden master". DNS UPDATE messages are - * sent via a supplied "transport" class. + * sent via a supplied {@link Resolver}. * *
On call to {@link #commit()}, a single UPDATE message is created containing the records
* required to "synchronize" the DNS with the current (at the time of processing) state of the
@@ -89,7 +90,7 @@ public class DnsUpdateWriter extends BaseDnsWriter {
private final Duration dnsDefaultATtl;
private final Duration dnsDefaultNsTtl;
private final Duration dnsDefaultDsTtl;
- private final DnsMessageTransport transport;
+ private final Resolver resolver;
private final Clock clock;
private final Update update;
private final String zoneName;
@@ -100,7 +101,7 @@ public class DnsUpdateWriter extends BaseDnsWriter {
* @param dnsDefaultATtl TTL used for any created resource records
* @param dnsDefaultNsTtl TTL used for any created nameserver records
* @param dnsDefaultDsTtl TTL used for any created DS records
- * @param transport the transport used to send/receive the UPDATE messages
+ * @param resolver the resolver used to send/receive the UPDATE messages
* @param clock a source of time
*/
@Inject
@@ -109,14 +110,14 @@ public DnsUpdateWriter(
@Config("dnsDefaultATtl") Duration dnsDefaultATtl,
@Config("dnsDefaultNsTtl") Duration dnsDefaultNsTtl,
@Config("dnsDefaultDsTtl") Duration dnsDefaultDsTtl,
- DnsMessageTransport transport,
+ Resolver resolver,
Clock clock) {
this.zoneName = zoneName;
this.update = new Update(toAbsoluteName(zoneName));
this.dnsDefaultATtl = dnsDefaultATtl;
this.dnsDefaultNsTtl = dnsDefaultNsTtl;
this.dnsDefaultDsTtl = dnsDefaultDsTtl;
- this.transport = transport;
+ this.resolver = resolver;
this.clock = clock;
}
@@ -175,7 +176,7 @@ public void publishHost(String hostName) {
@Override
protected void commitUnchecked() {
try {
- Message response = transport.send(update);
+ Message response = resolver.send(update);
verify(
response.getRcode() == Rcode.NOERROR,
"DNS server failed domain update for '%s' rcode: %s",
diff --git a/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterModule.java b/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterModule.java
index 7fcca5b8e9e..9610b974337 100644
--- a/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterModule.java
+++ b/core/src/main/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterModule.java
@@ -20,17 +20,30 @@
import dagger.multibindings.IntoMap;
import dagger.multibindings.IntoSet;
import dagger.multibindings.StringKey;
+import google.registry.config.RegistryConfig.Config;
import google.registry.dns.writer.DnsWriter;
import jakarta.inject.Named;
-import javax.net.SocketFactory;
+import java.net.UnknownHostException;
+import java.time.Duration;
+import org.xbill.DNS.Resolver;
+import org.xbill.DNS.SimpleResolver;
/** Dagger module that provides a DnsUpdateWriter. */
@Module
public abstract class DnsUpdateWriterModule {
@Provides
- static SocketFactory provideSocketFactory() {
- return SocketFactory.getDefault();
+ static Resolver provideResolver(
+ @Config("dnsUpdateHost") String updateHost,
+ @Config("dnsUpdateTimeout") Duration updateTimeout) {
+ try {
+ SimpleResolver resolver = new SimpleResolver(updateHost);
+ resolver.setTCP(true);
+ resolver.setTimeout(updateTimeout);
+ return resolver;
+ } catch (UnknownHostException e) {
+ throw new RuntimeException(e);
+ }
}
@Binds
diff --git a/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsMessageTransportTest.java b/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsMessageTransportTest.java
deleted file mode 100644
index 78bf6f440c1..00000000000
--- a/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsMessageTransportTest.java
+++ /dev/null
@@ -1,193 +0,0 @@
-// Copyright 2017 The Nomulus Authors. All Rights Reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package google.registry.dns.writer.dnsupdate;
-
-import static com.google.common.io.BaseEncoding.base16;
-import static com.google.common.truth.Truth.assertThat;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
-import com.google.common.base.VerifyException;
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.EOFException;
-import java.io.InputStream;
-import java.net.InetAddress;
-import java.net.Socket;
-import java.net.SocketTimeoutException;
-import java.nio.ByteBuffer;
-import java.time.Duration;
-import java.util.Arrays;
-import javax.net.SocketFactory;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
-import org.junit.jupiter.api.Test;
-import org.xbill.DNS.ARecord;
-import org.xbill.DNS.DClass;
-import org.xbill.DNS.Flags;
-import org.xbill.DNS.Message;
-import org.xbill.DNS.Name;
-import org.xbill.DNS.Opcode;
-import org.xbill.DNS.Rcode;
-import org.xbill.DNS.Record;
-import org.xbill.DNS.Type;
-import org.xbill.DNS.Update;
-
-/** Unit tests for {@link DnsMessageTransport}. */
-class DnsMessageTransportTest {
-
- private static final String UPDATE_HOST = "127.0.0.1";
-
- private final SocketFactory mockFactory = mock(SocketFactory.class);
- private final Socket mockSocket = mock(Socket.class);
-
- private Message simpleQuery;
- private Message expectedResponse;
- private DnsMessageTransport resolver;
-
- @BeforeEach
- @SuppressWarnings("AddressSelection")
- void beforeEach() throws Exception {
- simpleQuery =
- Message.newQuery(Record.newRecord(Name.fromString("example.com."), Type.A, DClass.IN));
- expectedResponse = responseMessageWithCode(simpleQuery, Rcode.NOERROR);
- when(mockFactory.createSocket(InetAddress.getByName(UPDATE_HOST), DnsMessageTransport.DNS_PORT))
- .thenReturn(mockSocket);
- resolver = new DnsMessageTransport(mockFactory, UPDATE_HOST, Duration.ZERO);
- }
-
- @Test
- void testSentMessageHasCorrectLengthAndContent() throws Exception {
- ByteArrayInputStream inputStream =
- new ByteArrayInputStream(messageToBytesWithLength(expectedResponse));
- when(mockSocket.getInputStream()).thenReturn(inputStream);
- ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
- when(mockSocket.getOutputStream()).thenReturn(outputStream);
-
- resolver.send(simpleQuery);
-
- ByteBuffer sentMessage = ByteBuffer.wrap(outputStream.toByteArray());
- int messageLength = sentMessage.getShort();
- byte[] messageData = new byte[messageLength];
- sentMessage.get(messageData);
- assertThat(messageLength).isEqualTo(simpleQuery.toWire().length);
- assertThat(base16().encode(messageData)).isEqualTo(base16().encode(simpleQuery.toWire()));
- }
-
- @Test
- void testReceivedMessageWithLengthHasCorrectContent() throws Exception {
- ByteArrayInputStream inputStream =
- new ByteArrayInputStream(messageToBytesWithLength(expectedResponse));
- when(mockSocket.getInputStream()).thenReturn(inputStream);
- when(mockSocket.getOutputStream()).thenReturn(new ByteArrayOutputStream());
-
- Message actualResponse = resolver.send(simpleQuery);
-
- assertThat(base16().encode(actualResponse.toWire()))
- .isEqualTo(base16().encode(expectedResponse.toWire()));
- }
-
- @Test
- void testEofReceivingResponse() throws Exception {
- byte[] messageBytes = messageToBytesWithLength(expectedResponse);
- ByteArrayInputStream inputStream =
- new ByteArrayInputStream(Arrays.copyOf(messageBytes, messageBytes.length - 1));
- when(mockSocket.getInputStream()).thenReturn(inputStream);
- when(mockSocket.getOutputStream()).thenReturn(new ByteArrayOutputStream());
- assertThrows(EOFException.class, () -> resolver.send(new Message()));
- }
-
- @Test
- @Disabled("This test hangs and causes OOM when using Java 21")
- // This test should never have passed. The socket is mocked and calling setSoTimeout() has no
- // effect whatsoever. We should consider removing it.
- void testTimeoutReceivingResponse() throws Exception {
- InputStream mockInputStream = mock(InputStream.class);
- when(mockInputStream.read()).thenThrow(new SocketTimeoutException("testing"));
- when(mockSocket.getInputStream()).thenReturn(mockInputStream);
- when(mockSocket.getOutputStream()).thenReturn(new ByteArrayOutputStream());
-
- Duration testTimeout = Duration.ofSeconds(1);
- DnsMessageTransport resolver = new DnsMessageTransport(mockFactory, UPDATE_HOST, testTimeout);
- Message expectedQuery = new Message();
- assertThrows(SocketTimeoutException.class, () -> resolver.send(expectedQuery));
- verify(mockSocket).setSoTimeout((int) testTimeout.toMillis());
- }
-
- @Test
- void testSentMessageTooLongThrowsException() throws Exception {
- Update oversize = new Update(Name.fromString("tld", Name.root));
- for (int i = 0; i < 2000; i++) {
- oversize.add(
- ARecord.newRecord(
- Name.fromString("test-extremely-long-name-" + i + ".tld", Name.root),
- Type.A,
- DClass.IN));
- }
- ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
- when(mockSocket.getOutputStream()).thenReturn(outputStream);
- IllegalArgumentException thrown =
- assertThrows(IllegalArgumentException.class, () -> resolver.send(oversize));
- assertThat(thrown).hasMessageThat().contains("message larger than maximum");
- }
-
- @Test
- void testResponseIdMismatchThrowsException() throws Exception {
- expectedResponse.getHeader().setID(1 + simpleQuery.getHeader().getID());
- when(mockSocket.getInputStream())
- .thenReturn(new ByteArrayInputStream(messageToBytesWithLength(expectedResponse)));
- when(mockSocket.getOutputStream()).thenReturn(new ByteArrayOutputStream());
- VerifyException thrown = assertThrows(VerifyException.class, () -> resolver.send(simpleQuery));
- assertThat(thrown)
- .hasMessageThat()
- .contains(
- "response ID "
- + expectedResponse.getHeader().getID()
- + " does not match query ID "
- + simpleQuery.getHeader().getID());
- }
-
- @Test
- void testResponseOpcodeMismatchThrowsException() throws Exception {
- simpleQuery.getHeader().setOpcode(Opcode.QUERY);
- expectedResponse.getHeader().setOpcode(Opcode.STATUS);
- when(mockSocket.getInputStream())
- .thenReturn(new ByteArrayInputStream(messageToBytesWithLength(expectedResponse)));
- when(mockSocket.getOutputStream()).thenReturn(new ByteArrayOutputStream());
- VerifyException thrown = assertThrows(VerifyException.class, () -> resolver.send(simpleQuery));
- assertThat(thrown)
- .hasMessageThat()
- .contains("response opcode 'STATUS' does not match query opcode 'QUERY'");
- }
-
- private Message responseMessageWithCode(Message query, int responseCode) {
- Message message = new Message(query.getHeader().getID());
- message.getHeader().setOpcode(query.getHeader().getOpcode());
- message.getHeader().setFlag(Flags.QR);
- message.getHeader().setRcode(responseCode);
- return message;
- }
-
- private byte[] messageToBytesWithLength(Message message) {
- byte[] bytes = message.toWire();
- ByteBuffer buffer =
- ByteBuffer.allocate(bytes.length + DnsMessageTransport.MESSAGE_LENGTH_FIELD_BYTES);
- buffer.putShort((short) bytes.length);
- buffer.put(bytes);
- return buffer.array();
- }
-}
diff --git a/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java b/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java
index 2cdace40075..6dbea9cdb73 100644
--- a/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java
+++ b/core/src/test/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java
@@ -18,6 +18,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static google.registry.testing.DatabaseHelper.createTld;
+import static google.registry.testing.DatabaseHelper.newDomain;
import static google.registry.testing.DatabaseHelper.newHost;
import static google.registry.testing.DatabaseHelper.persistActiveDomain;
import static google.registry.testing.DatabaseHelper.persistActiveHost;
@@ -43,13 +44,20 @@
import google.registry.model.tld.Tld;
import google.registry.persistence.transaction.JpaTestExtensions;
import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension;
-import google.registry.testing.DatabaseHelper;
import google.registry.testing.FakeClock;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.ServerSocket;
+import java.net.Socket;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -66,12 +74,16 @@
import org.xbill.DNS.RRset;
import org.xbill.DNS.Rcode;
import org.xbill.DNS.Record;
+import org.xbill.DNS.Resolver;
import org.xbill.DNS.Section;
+import org.xbill.DNS.SimpleResolver;
import org.xbill.DNS.Type;
import org.xbill.DNS.Update;
+import org.xbill.DNS.WireParseException;
/** Unit tests for {@link DnsUpdateWriter}. */
@ExtendWith(MockitoExtension.class)
+@MockitoSettings(strictness = Strictness.LENIENT)
public class DnsUpdateWriterTest {
@RegisterExtension
@@ -81,7 +93,7 @@ public class DnsUpdateWriterTest {
// Base16 encoded sha1 digest. Must have 40 chars (20 bytes after decoding)
private static final String BASE16_SHA1_DIGEST = "0123".repeat(10);
- @Mock private DnsMessageTransport mockResolver;
+ @Mock private Resolver mockResolver;
@Captor private ArgumentCaptor