[#123] Change DnsResourceRecord such that it can accept zero RDLENGTH.

CHANGES.md

@@ -5,6 +5,7 @@ Future
 * Add Packets class, a utility class for operating on packets.
 
 ### Bug Fixes ###
+* [Issues#123](https://github.com/kaitoy/pcap4j/issues/123): IllegalArgumentException occurs during dissecting a DNS packet including a resource record the RDLENGTH of which is zero.
 
 ### Other Changes ###
 

pcap4j-core/src/main/java/org/pcap4j/packet/DnsDomainName.java

@@ -1,3 +1,10 @@
+/*_##########################################################################
+  _##
+  _##  Copyright (C) 2016-2017  Pcap4J.org
+  _##
+  _##########################################################################
+*/
+
 package org.pcap4j.packet;
 
 import static org.pcap4j.util.ByteArrays.*;
@@ -41,6 +48,19 @@ public final class DnsDomainName implements Serializable {
 
   private static final Logger LOG = LoggerFactory.getLogger(DnsDomainName.class);
 
+  /**
+   * The root domain (zero)
+   */
+  public static final DnsDomainName ROOT_DOMAIN;
+
+  static {
+    try {
+      ROOT_DOMAIN = new DnsDomainName(new byte[] { 0 }, 0, 1);
+    } catch (IllegalRawDataException e) {
+      throw new AssertionError("Never get here.");
+    }
+  }
+
   private final List<String> labels;
   private final String name;
   private final Short pointer;
@@ -227,7 +247,7 @@ public Integer getPointerAsInt() {
   /**
    * @param headerRawData the raw data of the DNS header including this domain name.
    * @return decompressed name.
-   * @throws IllegalRawDataException if an eror occurred during decompression
+   * @throws IllegalRawDataException if an error occurred during decompression
    *                                 or circular reference is detected.
    */
   public String decompress(byte[] headerRawData) throws IllegalRawDataException {
@@ -315,6 +335,10 @@ public int length() {
 
   @Override
   public String toString() {
+    if (labels.size() == 0 && pointer == null) {
+      return "<ROOT>";
+    }
+
     if (pointer == null) {
       return name;
     }
@@ -337,6 +361,10 @@ public String toString() {
    * @return string representation of this object.
    */
   public String toString(byte[] headerRawData) {
+    if (labels.size() == 0 && pointer == null) {
+      return "<ROOT>";
+    }
+
     if (pointer == null) {
       return name;
     }

pcap4j-core/src/main/java/org/pcap4j/packet/DnsResourceRecord.java

@@ -1,6 +1,6 @@
 /*_##########################################################################
   _##
-  _##  Copyright (C) 2016  Pcap4J.org
+  _##  Copyright (C) 2016-2017  Pcap4J.org
   _##
   _##########################################################################
 */
@@ -132,14 +132,19 @@ private DnsResourceRecord(
       throw new IllegalRawDataException(sb.toString());
     }
 
-    this.rData = PacketFactories
-                   .getFactory(DnsRData.class, DnsResourceRecordType.class)
-                      .newInstance(
-                         rawData,
-                         offset + cursor,
-                         rdLen,
-                         dataType
-                       );
+    if (rdLen != 0) {
+      this.rData = PacketFactories
+                     .getFactory(DnsRData.class, DnsResourceRecordType.class)
+                     .newInstance(
+                       rawData,
+                       offset + cursor,
+                       rdLen,
+                       dataType
+                     );
+    }
+    else {
+      this.rData = null;
+    }
   }
 
   private DnsResourceRecord(Builder builder) {
@@ -148,14 +153,12 @@ private DnsResourceRecord(Builder builder) {
       || builder.name == null
       || builder.dataType == null
       || builder.dataClass == null
-      || builder.rData == null
     ) {
       StringBuilder sb = new StringBuilder();
       sb.append("builder").append(builder)
         .append(" builder.name: ").append(builder.name)
         .append(" builder.dataType: ").append(builder.dataType)
-        .append(" builder.dataClass: ").append(builder.dataClass)
-        .append(" builder.rData: ").append(builder.rData);
+        .append(" builder.dataClass: ").append(builder.dataClass);
       throw new NullPointerException(sb.toString());
     }
 
@@ -166,7 +169,7 @@ private DnsResourceRecord(Builder builder) {
     this.rData = builder.rData;
 
     if (builder.correctLengthAtBuild) {
-      int rdLen = rData.length();
+      int rdLen = rData == null ? 0 : rData.length();
       if ((rdLen & 0xFFFF0000) != 0) {
         throw new IllegalArgumentException(
                 "(rData.length() & 0xFFFF0000) must be zero. rData: " + rData
@@ -229,7 +232,7 @@ public int getRdLengthAsInt() {
   }
 
   /**
-   * @return rData
+   * @return rData. May be null.
    */
   public DnsRData getRData() {
     return rData;
@@ -275,12 +278,14 @@ public byte[] getRawData() {
       ByteArrays.toByteArray(rdLength), 0,
       data, cursor, SHORT_SIZE_IN_BYTES
     );
-    cursor += SHORT_SIZE_IN_BYTES;
-    byte[] rawRData = rData.getRawData();
-    System.arraycopy(
-      rawRData, 0,
-      data, cursor, rawRData.length
-    );
+    if (rData != null) {
+      cursor += SHORT_SIZE_IN_BYTES;
+      byte[] rawRData = rData.getRawData();
+      System.arraycopy(
+        rawRData, 0,
+        data, cursor, rawRData.length
+      );
+    }
 
     return data;
   }
@@ -289,7 +294,8 @@ public byte[] getRawData() {
    * @return length
    */
   public int length() {
-    return name.length() + SHORT_SIZE_IN_BYTES * 3 + INT_SIZE_IN_BYTES + rData.length();
+    int rDataLen = rData == null ? 0 : rData.length();
+    return name.length() + SHORT_SIZE_IN_BYTES * 3 + INT_SIZE_IN_BYTES + rDataLen;
   }
 
   @Override
@@ -330,60 +336,56 @@ private String convertToString(String indent, byte[] headerRawData) {
       .append(indent).append("TTL: ")
       .append(getTtlAsLong()).append(ls)
       .append(indent).append("RDLENGTH: ")
-      .append(getRdLengthAsInt()).append(ls)
-      .append(indent).append("RDATA:").append(ls)
-      .append(
-         headerRawData != null ? rData.toString(indent + "  ", headerRawData)
-                               : rData.toString(indent + "  ")
-       );
+      .append(getRdLengthAsInt()).append(ls);
+      if (rData != null) {
+        sb.append(indent).append("RDATA:").append(ls)
+          .append(
+            headerRawData != null ? rData.toString(indent + "  ", headerRawData)
+                                  : rData.toString(indent + "  ")
+          );
+      }
 
     return sb.toString();
   }
 
   @Override
   public int hashCode() {
-    final int prime = 31;
-    int result = 1;
-    result = prime * result + dataClass.hashCode();
-    result = prime * result + dataType.hashCode();
-    result = prime * result + name.hashCode();
-    result = prime * result + rdLength;
-    result = prime * result + ttl;
-    result = prime * result + rData.hashCode();
+    int result = name.hashCode();
+    result = 31 * result + dataType.hashCode();
+    result = 31 * result + dataClass.hashCode();
+    result = 31 * result + ttl;
+    result = 31 * result + (int) rdLength;
+    result = 31 * result + (rData != null ? rData.hashCode() : 0);
     return result;
   }
 
   @Override
-  public boolean equals(Object obj) {
-    if (this == obj) {
+  public boolean equals(Object o) {
+    if (this == o) {
       return true;
     }
-    if (obj == null) {
-      return false;
-    }
-    if (getClass() != obj.getClass()) {
+    if (o == null || getClass() != o.getClass()) {
       return false;
     }
-    DnsResourceRecord other = (DnsResourceRecord) obj;
-    if (!dataClass.equals(other.dataClass)) {
-      return false;
-    }
-    if (!dataType.equals(other.dataType)) {
+
+    DnsResourceRecord that = (DnsResourceRecord) o;
+
+    if (ttl != that.ttl) {
       return false;
     }
-    if (!name.equals(other.name)) {
+    if (rdLength != that.rdLength) {
       return false;
     }
-    if (rdLength != other.rdLength) {
+    if (!name.equals(that.name)) {
       return false;
     }
-    if (ttl != other.ttl) {
+    if (!dataType.equals(that.dataType)) {
       return false;
     }
-    if (!rData.equals(other.rData)) {
+    if (!dataClass.equals(that.dataClass)) {
       return false;
     }
-    return true;
+    return rData != null ? rData.equals(that.rData) : that.rData == null;
   }
 
   /**

pcap4j-packettest/src/test/java/org/pcap4j/packet/DnsPacketTest.java

@@ -67,7 +67,7 @@ public DnsPacketTest() throws Exception {
     this.qdCount = 1;
     this.anCount = 6;
     this.nsCount = 6;
-    this.arCount = 8;
+    this.arCount = 9;
     this.questions = new ArrayList<DnsQuestion>();
     this.answers = new ArrayList<DnsResourceRecord>();
     this.authorities = new ArrayList<DnsResourceRecord>();
@@ -543,24 +543,34 @@ public DnsPacketTest() throws Exception {
 
     DnsResourceRecord aRRStr
       = new DnsResourceRecord.Builder()
-      .name(
-        new DnsDomainName.Builder()
-          .labels(hogeDomain)
-          .build()
-      )
-      .dataType(DnsResourceRecordType.A)
-      .dataClass(DnsClass.ANY)
-      .ttl(123123)
-      .rData(
-        new DnsRDataA.Builder()
-          .address((Inet4Address) InetAddress.getByName("192.168.0.100"))
-          .addressPlainText(true)
-          .build()
-      )
-      .correctLengthAtBuild(true)
-      .build();
+          .name(
+            new DnsDomainName.Builder()
+              .labels(hogeDomain)
+              .build()
+          )
+          .dataType(DnsResourceRecordType.A)
+          .dataClass(DnsClass.ANY)
+          .ttl(123123)
+          .rData(
+            new DnsRDataA.Builder()
+              .address((Inet4Address) InetAddress.getByName("192.168.0.100"))
+              .addressPlainText(true)
+              .build()
+          )
+          .correctLengthAtBuild(true)
+          .build();
     additionalInfo.add(aRRStr);
 
+    DnsResourceRecord optRR
+      = new DnsResourceRecord.Builder()
+          .name(DnsDomainName.ROOT_DOMAIN)
+          .dataType(DnsResourceRecordType.OPT)
+          .dataClass(DnsClass.ANY)
+          .ttl(123123)
+          .correctLengthAtBuild(true)
+          .build();
+    additionalInfo.add(optRR);
+
     this.packet
       = new DnsPacket.Builder()
           .id(id)

pcap4j-packettest/src/test/resources/DnsPacketTest.log

@@ -1,4 +1,4 @@
-[DNS Header (1217 bytes)]
+[DNS Header (1228 bytes)]
   ID: 0x2fc9
   QR: response
   OPCODE: 2 (Status)
@@ -13,7 +13,7 @@
   QDCOUNT: 1
   ANCOUNT: 6
   NSCOUNT: 6
-  ARCOUNT: 8
+  ARCOUNT: 9
   Question:
     QNAME: hoge.co.jp
     QTYPE: 1 (A (Host address))
@@ -219,3 +219,9 @@
     RDATA:
       A RDATA:
         ADDRESS: 192.168.0.100 (text)
+  Additional:
+    NAME: <ROOT>
+    TYPE: 41 (OPT)
+    CLASS: 255 (ANY)
+    TTL: 123123
+    RDLENGTH: 0