Change semantics of DNSInput.saveActive() (#228)

nresare · web-flow · commit aa9e845bfc58 · 2021-10-22T23:52:35.000+02:00
Previously, DNSInput.saveActive() would return the active range without taking prior calls to setActive() into account. Add a test case that illustrates the failure that the user experienced, as well as a test case verifying DNSInput semantics when called recursively with ByteBuffers with limit and length set. Please see #225 for further details.
diff --git a/src/main/java/org/xbill/DNS/DNSInput.java b/src/main/java/org/xbill/DNS/DNSInput.java
@@ -79,7 +79,7 @@ public void clearActive() {
 
   /** Returns the position of the end of the current active region. */
   public int saveActive() {
-    return limit - offset;
+    return byteBuffer.limit() - offset;
   }
 
   /**
diff --git a/src/test/java/org/xbill/DNS/DNSInputTest.java b/src/test/java/org/xbill/DNS/DNSInputTest.java
@@ -39,6 +39,8 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.nio.ByteBuffer;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -84,6 +86,19 @@ void setUp() {
     }
   }
 
+  static class DNSInputByteBufferLimitOffsetTest extends DNSInputBase {
+    @BeforeEach
+    void setUp() throws IOException {
+      m_raw = new byte[] {0, 1, 2, 3, 4, 5, (byte) 255, (byte) 255, (byte) 255, (byte) 255};
+      // create a new byte array with a prefix and a suffix to be ignored
+      ByteArrayOutputStream out = new ByteArrayOutputStream();
+      out.write(42);
+      out.write(m_raw);
+      out.write(47);
+      m_di = new DNSInput(ByteBuffer.wrap(out.toByteArray(), 1, 10));
+    }
+  }
+
   @Test
   void initial_state() {
     assertEquals(0, m_di.current());
@@ -319,4 +334,28 @@ void readCountedSting() throws WireParseException {
     assertEquals(3, m_di.current());
     assertEquals(2, out[0]);
   }
+
+  @Test
+  void setActive_recursive() throws WireParseException {
+    int outer = m_di.saveActive();
+    m_di.setActive(3);
+
+    assertEquals(0x00, m_di.readU8());
+    assertEquals(2, m_di.remaining());
+
+    int inner = m_di.saveActive();
+
+    m_di.setActive(1);
+    assertArrayEquals(new byte[] {0x01}, m_di.readByteArray());
+
+    m_di.restoreActive(inner);
+
+    assertArrayEquals(new byte[] {0x02}, m_di.readByteArray());
+    assertEquals(0, m_di.remaining());
+
+    m_di.restoreActive(outer);
+
+    assertEquals(0x03, m_di.readU8());
+    assertEquals(6, m_di.remaining());
+  }
 }
diff --git a/src/test/java/org/xbill/DNS/OPTRecordTest.java b/src/test/java/org/xbill/DNS/OPTRecordTest.java
@@ -6,7 +6,10 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.io.IOException;
+import java.util.Collections;
 import org.junit.jupiter.api.Test;
+import org.xbill.DNS.utils.base16;
 
 class OPTRecordTest {
 
@@ -47,6 +50,24 @@ void rdataFromString() {
     assertTrue(thrown.getMessage().contains("no text format defined for OPT"));
   }
 
+  @Test
+  void rdataFromWire() throws IOException {
+    byte[] buf = base16.fromString("000029100000000000000C000A00084531D089BA80C6EB");
+    OPTRecord record = (OPTRecord) OPTRecord.fromWire(new DNSInput(buf), Section.ADDITIONAL);
+    assertEquals(
+        Collections.singletonList(new CookieOption(base16.fromString("4531D089BA80C6EB"))),
+        record.getOptions());
+  }
+
+  @Test
+  void rdataFromWire_nullPadded() throws IOException {
+    byte[] buf = base16.fromString("000029100000000000000C000A00084531D089BA80C6EB00");
+    OPTRecord record = (OPTRecord) OPTRecord.fromWire(new DNSInput(buf), Section.ADDITIONAL);
+    assertEquals(
+        Collections.singletonList(new CookieOption(base16.fromString("4531D089BA80C6EB"))),
+        record.getOptions());
+  }
+
   private void assertNotEqual(final OPTRecord optRecordOne, final OPTRecord optRecordTwo) {
     assertFalse(optRecordOne.equals(optRecordTwo));
     assertFalse(optRecordTwo.equals(optRecordOne));

Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,7 @@ public void clearActive() {`
`79`	`79`
`80`	`80`	`/** Returns the position of the end of the current active region. */`
`81`	`81`	`public int saveActive() {`
`82`		`- return limit - offset;`
	`82`	`+ return byteBuffer.limit() - offset;`
`83`	`83`	`}`
`84`	`84`
`85`	`85`	`/**`